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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
19 changes: 19 additions & 0 deletions app/activeproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ 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;
qDebug() << "ROLE 1 : " << role;
setProjectRole( role );

updateMapTheme();
updateRecordingLayers();
updateActiveLayer();
Expand Down Expand Up @@ -553,3 +557,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();
}
}
23 changes: 23 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"

/**
* \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,23 @@ 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 );

/**
* Calls Mergin API to update current project’s role
*/
void updateUserRoleInActiveProject();

/**
* Update current project’s role
*/
void onProjectRoleUpdated( const QString &projectFullName, const QString &role );

signals:
void qgsProjectChanged();
void localProjectChanged( LocalProject project );
Expand Down Expand Up @@ -145,6 +164,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 +203,12 @@ class ActiveProject: public QObject
LayersProxyModel &mRecordingLayerPM;
LocalProjectsManager &mLocalProjectsManager;
InputMapSettings *mMapSettings = nullptr;
MerginApi *mMerginApi = 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
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 );

// 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
8 changes: 8 additions & 0 deletions core/coreutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,3 +335,11 @@ 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";
}
5 changes: 5 additions & 0 deletions core/coreutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ class CoreUtils
*/
static QString bytesToHumanSize( double bytes );

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

private:
static QString sLogFile;
static int CHECKSUM_CHUNK_SIZE;
Expand Down
102 changes: 102 additions & 0 deletions core/merginapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3449,6 +3449,45 @@ bool MerginApi::writeData( const QByteArray &data, const QString &path )
return true;
}

bool MerginApi::updateCachedProjectRole( const QString &projectFullName, const QString &newRole )
{
LocalProject project = mLocalProjects.projectFromMerginName( projectFullName );
if ( !project.isValid() )
{
return false;
}

QString metadataPath = project.projectDir + "/" + sMetadataFile;

QFile file( metadataPath );
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["role"] = newRole;
doc.setObject( obj );

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

bool success = ( file.write( doc.toJson() ) != -1 );
file.close();
Comment on lines +3462 to +3487
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's turn this into a CoreUtils::replaceValueInJson method that can then be easily tested. Ideally pass in the name of the key, key value and path to json + autotests


return success;
}

void MerginApi::createPathIfNotExists( const QString &filePath )
{
Expand Down Expand Up @@ -3945,3 +3984,66 @@ DownloadQueueItem::DownloadQueueItem( const QString &fp, qint64 s, int v, qint64
tempFileName = CoreUtils::uuidWithoutBraces( QUuid::createUuid() );
}

void MerginApi::reloadProjectRole( const QString &projectFullName )
{
if ( projectFullName.isEmpty() )
{
return;
}

QNetworkReply *reply = getProjectInfo( projectFullName );
if ( !reply )
return;

reply->request().setAttribute( static_cast<QNetworkRequest::Attribute>( AttrProjectFullName ), projectFullName );
connect( reply, &QNetworkReply::finished, this, &MerginApi::reloadProjectRoleReplyFinished );
}

void MerginApi::reloadProjectRoleReplyFinished()
{
QNetworkReply *r = qobject_cast<QNetworkReply *>( sender() );
Q_ASSERT( r );

QString projectFullName = r->request().attribute( static_cast<QNetworkRequest::Attribute>( AttrProjectFullName ) ).toString();
QString cachedRole = MerginApi::getCachedProjectRole( projectFullName );

if ( r->error() == QNetworkReply::NoError )
{
QByteArray data = r->readAll();
MerginProjectMetadata serverProject = MerginProjectMetadata::fromJson( data );
QString role = serverProject.role;

if ( role != cachedRole )
{
if ( updateCachedProjectRole( projectFullName, role ) )
{
emit projectRoleUpdated( projectFullName, role );
}
else
{
CoreUtils::log( "metadata", QString( "Failed to update cached role for project %1" ).arg( projectFullName ) );
}
}
}
else
{
emit projectRoleUpdated( projectFullName, cachedRole );
}

r->deleteLater();
}

QString MerginApi::getCachedProjectRole( const QString &projectFullName ) const
{
if ( projectFullName.isEmpty() )
return QString();

QString projectDir = mLocalProjects.projectFromMerginName( projectFullName ).projectDir;

if ( projectDir.isEmpty() )
return QString();

MerginProjectMetadata cachedProjectMetadata = MerginProjectMetadata::fromCachedJson( projectDir + "/" + sMetadataFile );

return cachedProjectMetadata.role;
}
14 changes: 13 additions & 1 deletion core/merginapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,11 @@ class MerginApi: public QObject
*/
bool apiSupportsWorkspaces();

/**
* Reloads project metadata role by fetching latest information from server.
*/
Q_INVOKABLE void reloadProjectRole( const QString &projectFullName );

signals:
void apiSupportsSubscriptionsChanged();
void supportsSelectiveSyncChanged();
Expand Down Expand Up @@ -650,6 +655,7 @@ class MerginApi: public QObject
void apiSupportsWorkspacesChanged();

void serverWasUpgraded();
void projectRoleUpdated( const QString &projectFullName, const QString &role );

private slots:
void listProjectsReplyFinished( QString requestId );
Expand Down Expand Up @@ -789,6 +795,12 @@ class MerginApi: public QObject

bool projectFileHasBeenUpdated( const ProjectDiff &diff );

void reloadProjectRoleReplyFinished();

bool updateCachedProjectRole( const QString &projectFullName, const QString &newRole );

QString getCachedProjectRole( const QString &projectFullName ) const;

QNetworkAccessManager mManager;
QString mApiRoot;
LocalProjectsManager &mLocalProjects;
Expand All @@ -804,7 +816,7 @@ class MerginApi: public QObject
AttrProjectFullName = QNetworkRequest::User,
AttrTempFileName = QNetworkRequest::User + 1,
AttrWorkspaceName = QNetworkRequest::User + 2,
AttrAcceptFlag = QNetworkRequest::User + 3,
AttrAcceptFlag = QNetworkRequest::User + 3
};

Transactions mTransactionalStatus; //projectFullname -> transactionStatus
Expand Down
Loading
Loading