Skip to content

Commit

Permalink
Improving UX by hiding editing buttons for readers of a project (#3682)
Browse files Browse the repository at this point in the history
  • Loading branch information
VitorVieiraZ authored Jan 23, 2025
1 parent f671c96 commit 6115084
Show file tree
Hide file tree
Showing 18 changed files with 318 additions and 7 deletions.
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: 11 additions & 1 deletion 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 "merginprojectmetadata.h"

/**
* \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,10 @@ class ActiveProject: public QObject
LayersProxyModel &mRecordingLayerPM;
LocalProjectsManager &mLocalProjectsManager;
InputMapSettings *mMapSettings = nullptr;

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
80 changes: 80 additions & 0 deletions app/test/testcoreutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,83 @@ void TestCoreUtils::testNameAbbr()
QCOMPARE( CoreUtils::nameAbbr( name, email ), test.second );
}
}

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

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

QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "value", 456 ) );

// verify
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" ) );
}
// valid JSON with string value
{
QFile file( testFilePath );
QVERIFY( file.open( QIODevice::WriteOnly ) );
file.write( R"({"name": "test", "status": "active"})" );
file.close();

QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "status", "inactive" ) );

// verify replacement
QVERIFY( file.open( QIODevice::ReadOnly ) );
QJsonDocument doc = QJsonDocument::fromJson( file.readAll() );
file.close();
QVERIFY( doc.isObject() );
QJsonObject obj = doc.object();
QCOMPARE( obj["status"].toString(), QString( "inactive" ) );
QCOMPARE( obj["name"].toString(), QString( "test" ) );
}

// 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" ) );
}

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

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

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

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
37 changes: 36 additions & 1 deletion app/test/testmerginapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2943,6 +2943,42 @@ void TestMerginApi::testParseVersion()
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 fullProjectName = MerginApi::getFullProjectName( mWorkspaceName, projectName );

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

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

// Verify role was updated in cache
cachedRole = mApi->getCachedProjectRole( fullProjectName );
QCOMPARE( cachedRole, QString( "reader" ) );

// Role in server wasn't updated and stills "owner" => let's reload it from server and see if it updates in cached
QSignalSpy spy( mApi, &MerginApi::projectRoleUpdated );
mApi->reloadProjectRole( fullProjectName );
QVERIFY( spy.wait() );
cachedRole = mApi->getCachedProjectRole( fullProjectName );
QCOMPARE( cachedRole, QString( "owner" ) );

// Clean up
deleteRemoteProjectNow( mApi, mWorkspaceName, projectName );
}

void TestMerginApi::testDownloadWithNetworkError()
{
// Store original manager
Expand Down Expand Up @@ -3081,4 +3117,3 @@ void TestMerginApi::testDownloadWithNetworkErrorRecovery()
mApi->setNetworkManager( originalManager );
delete failingManager;
}

1 change: 1 addition & 0 deletions app/test/testmerginapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,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 );

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

0 comments on commit 6115084

Please sign in to comment.