Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving UX by hiding editing buttons for readers of a project #3682

Merged
merged 23 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
36b2f39
wip - reader and guests cant edit
VitorVieiraZ Nov 18, 2024
8ecf262
in progress
VitorVieiraZ Nov 21, 2024
b3ded65
in progress
VitorVieiraZ Nov 22, 2024
5da5eca
updating project role on opening a project from active project and ch…
VitorVieiraZ Nov 27, 2024
7a01cc6
updating role every time we open a project + visibility condition on …
VitorVieiraZ Nov 28, 2024
eabfc85
in progress
VitorVieiraZ Dec 9, 2024
6596705
ensuring user role is cached/updated each time we open a project
VitorVieiraZ Dec 10, 2024
8a92bff
cleaning code
VitorVieiraZ Dec 10, 2024
193574d
merging master to branch
VitorVieiraZ Dec 10, 2024
493bb18
Merge branch 'master' into enhancementUiUx/readerCantEdit
VitorVieiraZ Jan 10, 2025
4a6deda
post review changes
VitorVieiraZ Jan 13, 2025
e0f23b2
tests and removing truncate
VitorVieiraZ Jan 13, 2025
9cd5999
decoupling mergin api from activeProject and rebuilding connections
VitorVieiraZ Jan 15, 2025
713b347
final adjustments
VitorVieiraZ Jan 18, 2025
3b94135
Merge branch 'master' into enhancementUiUx/readerCantEdit
VitorVieiraZ Jan 18, 2025
48d244f
replaceValueInJson tests
VitorVieiraZ Jan 20, 2025
585412e
post review updates
VitorVieiraZ Jan 21, 2025
709b3fc
merging master
VitorVieiraZ Jan 21, 2025
97888cb
code layout adjustment
VitorVieiraZ Jan 21, 2025
8e4a046
small fix
VitorVieiraZ Jan 21, 2025
0b28d3b
include merginprojectmetadata in activeproject
VitorVieiraZ Jan 21, 2025
49b2a6a
updating tests
VitorVieiraZ Jan 22, 2025
721c762
fixing tests
VitorVieiraZ Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions app/activeproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ bool ActiveProject::forceLoad( const QString &filePath, bool force )
CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Could not find project in local projects: " ) + filePath );
}

QString role = MerginProjectMetadata::fromCachedJson( CoreUtils::getProjectMetadataPath( mLocalProject.projectDir ) ).role;
setProjectRole( role );

updateMapTheme();
updateRecordingLayers();
updateActiveLayer();
Expand Down Expand Up @@ -553,3 +556,18 @@ bool ActiveProject::positionTrackingSupported() const

return mQgsProject->readBoolEntry( QStringLiteral( "Mergin" ), QStringLiteral( "PositionTracking/Enabled" ), false );
}

QString ActiveProject::projectRole() const
{
return mProjectRole;
}

void ActiveProject::setProjectRole( const QString &role )
{
if ( mProjectRole != role )
{
mProjectRole = role;

emit projectRoleChanged();
}
}
12 changes: 12 additions & 0 deletions app/activeproject.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "localprojectsmanager.h"
#include "autosynccontroller.h"
#include "inputmapsettings.h"
#include "merginapi.h"
VitorVieiraZ marked this conversation as resolved.
Show resolved Hide resolved

/**
* \brief The ActiveProject class can load a QGIS project and holds its data.
Expand All @@ -33,6 +34,7 @@ class ActiveProject: public QObject
Q_PROPERTY( QgsProject *qgsProject READ qgsProject NOTIFY qgsProjectChanged ) // QgsProject instance of active project, never changes
Q_PROPERTY( AutosyncController *autosyncController READ autosyncController NOTIFY autosyncControllerChanged )
Q_PROPERTY( InputMapSettings *mapSettings READ mapSettings WRITE setMapSettings NOTIFY mapSettingsChanged )
Q_PROPERTY( QString projectRole READ projectRole WRITE setProjectRole NOTIFY projectRoleChanged )

Q_PROPERTY( QString mapTheme READ mapTheme WRITE setMapTheme NOTIFY mapThemeChanged )
Q_PROPERTY( bool positionTrackingSupported READ positionTrackingSupported NOTIFY positionTrackingSupportedChanged )
Expand Down Expand Up @@ -118,6 +120,12 @@ class ActiveProject: public QObject

bool positionTrackingSupported() const;

/**
* Returns role/permission level of current user for this project
*/
Q_INVOKABLE QString projectRole() const;
void setProjectRole( const QString &role );

signals:
void qgsProjectChanged();
void localProjectChanged( LocalProject project );
Expand Down Expand Up @@ -145,6 +153,8 @@ class ActiveProject: public QObject
// Emited when the app (UI) should show tracking because there is a running tracking service
void startPositionTracking();

void projectRoleChanged();

public slots:
// Reloads project if current project path matches given path (its the same project)
bool reloadProject( QString projectDir );
Expand Down Expand Up @@ -182,10 +192,12 @@ class ActiveProject: public QObject
LayersProxyModel &mRecordingLayerPM;
LocalProjectsManager &mLocalProjectsManager;
InputMapSettings *mMapSettings = nullptr;
MerginApi *mMerginApi = nullptr;
VitorVieiraZ marked this conversation as resolved.
Show resolved Hide resolved

std::unique_ptr<AutosyncController> mAutosyncController;

QString mProjectLoadingLog;
QString mProjectRole;

/**
* Reloads project.
Expand Down
22 changes: 22 additions & 0 deletions app/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ int main( int argc, char *argv[] )

ActiveLayer al;
ActiveProject activeProject( as, al, recordingLpm, localProjectsManager );

std::unique_ptr<VariablesManager> vm( new VariablesManager( ma.get() ) );
vm->registerInputExpressionFunctions();

Expand Down Expand Up @@ -569,6 +570,27 @@ int main( int argc, char *argv[] )
syncManager.syncProject( project, SyncOptions::Authorized, SyncOptions::Retry );
} );

QObject::connect( &activeProject, &ActiveProject::projectReloaded, &lambdaContext, [merginApi = ma.get(), &activeProject]()
{
merginApi->reloadProjectRole( activeProject.projectFullName() );
} );

QObject::connect( ma.get(), &MerginApi::authChanged, &lambdaContext, [merginApi = ma.get(), &activeProject]()
{
if ( activeProject.isProjectLoaded() )
{
merginApi->reloadProjectRole( activeProject.projectFullName() );
}
} );

QObject::connect( ma.get(), &MerginApi::projectRoleUpdated, &activeProject, [&activeProject]( const QString & projectFullName, const QString & role )
{
if ( projectFullName == activeProject.projectFullName() )
{
activeProject.setProjectRole( role );
}
} );

QObject::connect( ma.get(), &MerginApi::notifyInfo, &lambdaContext, [&notificationModel]( const QString & message )
{
notificationModel.addInfo( message );
Expand Down
4 changes: 2 additions & 2 deletions app/qml/form/MMFormPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ Page {

footer: MMComponents.MMToolbar {

visible: !root.layerIsReadOnly
visible: !root.layerIsReadOnly && __activeProject.projectRole !== "reader"

ObjectModel {
id: readStateButtons
Expand Down Expand Up @@ -231,7 +231,7 @@ Page {
id: editGeometry
text: qsTr( "Edit geometry" )
iconSource: __style.editIcon
visible: root.layerIsSpatial
visible: root.layerIsSpatial && __activeProject.projectRole !== "reader"
onClicked: root.editGeometryRequested( root.controller.featureLayerPair )
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/qml/form/MMPreviewDrawer.qml
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ Item {
property bool isHTMLType: root.controller.type === MM.AttributePreviewController.HTML
property bool isEmptyType: root.controller.type === MM.AttributePreviewController.Empty

property bool showEditButton: !root.layerIsReadOnly
property bool showEditButton: !root.layerIsReadOnly && __activeProject.projectRole !== "reader"
property bool showStakeoutButton: __inputUtils.isPointLayerFeature( controller.featureLayerPair )
property bool showButtons: showEditButton || showStakeoutButton

Expand Down
1 change: 1 addition & 0 deletions app/qml/form/components/MMFeaturesListPageDrawer.qml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Drawer {
}

text: qsTr( "Add feature" )
visible: __activeProject.projectRole !== "reader"

onClicked: root.buttonClicked()
}
Expand Down
2 changes: 1 addition & 1 deletion app/qml/layers/MMFeaturesListPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ MMComponents.MMPage {
anchors.bottom: parent.bottom
anchors.bottomMargin: root.hasToolbar ? __style.margin20 : ( __style.safeAreaBottom + __style.margin8 )

visible: __inputUtils.isNoGeometryLayer( root.selectedLayer ) && !root.layerIsReadOnly
visible: __inputUtils.isNoGeometryLayer( root.selectedLayer ) && !root.layerIsReadOnly && __activeProject.projectRole !== "reader"

text: qsTr("Add feature")

Expand Down
1 change: 1 addition & 0 deletions app/qml/main.qml
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ ApplicationWindow {
MMToolbarButton {
text: qsTr("Add")
iconSource: __style.addIcon
visible: __activeProject.projectRole !== "reader"
onClicked: {
if ( __recordingLayersModel.rowCount() > 0 ) {
stateManager.state = "map"
Expand Down
63 changes: 63 additions & 0 deletions app/test/testcoreutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,66 @@ void TestCoreUtils::testNameAbbr()
QCOMPARE( CoreUtils::nameAbbr( name, email ), test.second );
}
}

void TestCoreUtils::testReplaceValueInJson()
{
// Create a temporary test file
QString testFilePath = QDir::tempPath() + "/test_replace_value.json";

// Test case 1: Basic replacement in valid JSON
{
QFile file( testFilePath );
QVERIFY( file.open( QIODevice::WriteOnly ) );
file.write( R"({"name": "test", "value": 123})" );
file.close();

QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "value", 456 ) );
VitorVieiraZ marked this conversation as resolved.
Show resolved Hide resolved

// Verify the change
QVERIFY( file.open( QIODevice::ReadOnly ) );
QJsonDocument doc = QJsonDocument::fromJson( file.readAll() );
file.close();
QVERIFY( doc.isObject() );
QJsonObject obj = doc.object();
QCOMPARE( obj["value"].toInt(), 456 );
QCOMPARE( obj["name"].toString(), QString( "test" ) );
}

// Test case 2: Add new key-value pair
{
QFile file( testFilePath );
QVERIFY( file.open( QIODevice::WriteOnly ) );
file.write( R"({"name": "test"})" );
file.close();

QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "newKey", "newValue" ) );

// Verify the addition
QVERIFY( file.open( QIODevice::ReadOnly ) );
QJsonDocument doc = QJsonDocument::fromJson( file.readAll() );
file.close();
QVERIFY( doc.isObject() );
QJsonObject obj = doc.object();
QCOMPARE( obj["newKey"].toString(), QString( "newValue" ) );
QCOMPARE( obj["name"].toString(), QString( "test" ) );
}

// Test case 3: Invalid JSON file
{
QFile file( testFilePath );
QVERIFY( file.open( QIODevice::WriteOnly ) );
file.write( "invalid json content" );
file.close();

QVERIFY( !CoreUtils::replaceValueInJson( testFilePath, "key", "value" ) );
}

// Test case 4: Non-existent file
{
QString nonExistentPath = QDir::tempPath() + "/non_existent.json";
QVERIFY( !CoreUtils::replaceValueInJson( nonExistentPath, "key", "value" ) );
}

// Clean up
QFile::remove( testFilePath );
}
1 change: 1 addition & 0 deletions app/test/testcoreutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class TestCoreUtils : public QObject
void testHasProjectFileExtension();
void testNameValidation();
void testNameAbbr();
void testReplaceValueInJson();
};

#endif // TESTCOREUTILS_H
28 changes: 28 additions & 0 deletions app/test/testmerginapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2942,3 +2942,31 @@ void TestMerginApi::testParseVersion()
QCOMPARE( major, 2024 );
QCOMPARE( minor, 4 );
}

void TestMerginApi::testUpdateProjectMetadataRole()
{
QString projectName = "testUpdateProjectMetadataRole";

createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" );
downloadRemoteProject( mApi, mWorkspaceName, projectName );

LocalProject projectInfo = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName );
QVERIFY( projectInfo.isValid() );
QString projectDir = projectInfo.projectDir;

// Test 1: Initial role should be 'owner'
QString cachedRole = mApi->getCachedProjectRole( MerginApi::getFullProjectName( mWorkspaceName, projectName ) );
QCOMPARE( cachedRole, QString( "owner" ) );

// Test 2: Update cached role to 'reader'
QString newRole = "reader";
bool updateSuccess = mApi->updateCachedProjectRole( MerginApi::getFullProjectName( mWorkspaceName, projectName ), newRole );
QVERIFY( updateSuccess );

// Verify role was updated in cache
cachedRole = mApi->getCachedProjectRole( MerginApi::getFullProjectName( mWorkspaceName, projectName ) );
QCOMPARE( cachedRole, newRole );
VitorVieiraZ marked this conversation as resolved.
Show resolved Hide resolved

VitorVieiraZ marked this conversation as resolved.
Show resolved Hide resolved
// Clean up
deleteRemoteProjectNow( mApi, mWorkspaceName, projectName );
}
1 change: 1 addition & 0 deletions app/test/testmerginapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class TestMerginApi: public QObject
void testSynchronizationViaManager();
void testAutosync();
void testAutosyncFailure();
void testUpdateProjectMetadataRole();

void testRegisterAndDelete();
void testCreateWorkspace();
Expand Down
42 changes: 42 additions & 0 deletions core/coreutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include <QCryptographicHash>
#include <QRegularExpression>
#include <QStorageInfo>
#include <QJsonDocument>
#include <QJsonObject>

#include "qcoreapplication.h"

Expand Down Expand Up @@ -335,3 +337,43 @@ QString CoreUtils::bytesToHumanSize( double bytes )
return QString::number( bytes / 1024.0 / 1024.0 / 1024.0 / 1024.0, 'f', precision ) + " TB";
}
}

QString CoreUtils::getProjectMetadataPath( QString projectDir )
{
if ( projectDir.isEmpty() )
return QString();

return projectDir + "/.mergin/mergin.json";
}

bool CoreUtils::replaceValueInJson( const QString &filePath, const QString &key, const QJsonValue &value )
{
QFile file( filePath );
if ( !file.open( QIODevice::ReadOnly ) )
{
return false;
}

QByteArray data = file.readAll();
file.close();

QJsonDocument doc = QJsonDocument::fromJson( data );
if ( !doc.isObject() )
{
return false;
}

QJsonObject obj = doc.object();
obj[key] = value;
doc.setObject( obj );

if ( !file.open( QIODevice::WriteOnly ) )
{
return false;
}

bool success = ( file.write( doc.toJson() ) != -1 );
file.close();

return success;
}
10 changes: 10 additions & 0 deletions core/coreutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ class CoreUtils
*/
static QString bytesToHumanSize( double bytes );

/**
* Returns path to project metadata file for a given project directory
*/
static QString getProjectMetadataPath( QString projectDir );

/**
* Updates a value in a JSON file at the specified top-level key
*/
static bool replaceValueInJson( const QString &filePath, const QString &key, const QJsonValue &value );
tomasMizera marked this conversation as resolved.
Show resolved Hide resolved

private:
static QString sLogFile;
static int CHECKSUM_CHUNK_SIZE;
Expand Down
Loading
Loading