Skip to content

Conversation

@gabriel-bolbotina
Copy link
Contributor

Ready for review.
Added Auth sync class that handles the importation of authentication database configuration file.

Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

The exercise with the new AuthSync class is now feeling a bit too much - it's basically just calling the import method 🙃 I suggest we get rid of it altogether. In addition to that, the PR:

  • is missing autotests
  • missed project reload in case the auth file is changed
  • missed to reset auth entries before loading the auth db

Good job with the ossl linking fix.

ProjectDiff diff = transaction.diff;
int newVersion = syncSuccessful ? transaction.version : -1;

// TODO check if the xml configuration has been changed
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO?

Copy link
Contributor Author

@gabriel-bolbotina gabriel-bolbotina Dec 12, 2025

Choose a reason for hiding this comment

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

I tried to only modify the xml configuration file from QGIS. Upon syncing, it was already detected by that function, which emitted a force reload of the project. So I don't think we should specifically check for the cfg modification at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check this video.

@tomasMizera tomasMizera added the FROZEN 🥶 do not merge before upcoming release label Dec 15, 2025
@gabriel-bolbotina gabriel-bolbotina force-pushed the feature/qgis-auth-db-sync branch from 8c4755a to 2281cca Compare December 15, 2025 13:32
@gabriel-bolbotina gabriel-bolbotina self-assigned this Dec 16, 2025
@Withalion Withalion linked an issue Dec 17, 2025 that may be closed by this pull request
@Withalion Withalion removed the FROZEN 🥶 do not merge before upcoming release label Dec 18, 2025
@Withalion
Copy link
Contributor

Some ideas for testing:

  • test opening project without qgis auth db exported and private layer (opening the project should succeed, but layer should be available)
  • test opening project with qgis auth db ( opening the project should succeed and layer should be available)
  • test synchronizing project should trigger reload of auth db

@github-actions
Copy link

github-actions bot commented Dec 18, 2025

Pull Request Test Coverage Report for Build 21581403561

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 174 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.2%) to 59.7%

Files with Coverage Reduction New Missed Lines %
mm/app/attributes/attributecontroller.cpp 1 76.83%
mm/core/merginapi.cpp 3 75.29%
mm/app/activeproject.cpp 75 70.83%
mm/app/main.cpp 95 36.34%
Totals Coverage Status
Change from base Build 21428698414: 0.2%
Covered Lines: 8638
Relevant Lines: 14469

💛 - Coveralls

@Withalion
Copy link
Contributor

Windows build is failing because of the QCA_OSSL_PLUGIN lookup

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
QCA_OSSL_PLUGIN_LIB
    linked by target "MerginMaps" in directory D:/a/mobile/mobile/mm/app

@github-actions
Copy link

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📬 Mergin Maps 62251 dmg Expires: 22/04/2026 #6225
linux Build 📭 Build not yet complete or failed.
win64 Build 📬 Mergin Maps 54181 win64 Expires: 22/04/2026 #5418
Android Build 📬 Mergin Maps 753551 APK [arm64-v8a] Expires: 22/04/2026 #7535
Android Build 📬 Mergin Maps 753511 APK [armeabi-v7a] Expires: 22/04/2026 #7535
iOS Build 📬 Build number: 26.01.847611 #8476

@github-actions
Copy link

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📬 Mergin Maps 62261 dmg Expires: 23/04/2026 #6226
linux Build 📭 Build not yet complete or failed.
win64 Build 📬 Mergin Maps 54191 win64 Expires: 23/04/2026 #5419
Android Build 📬 Mergin Maps 753611 APK [armeabi-v7a] Expires: 23/04/2026 #7536
Android Build 📬 Mergin Maps 753651 APK [arm64-v8a] Expires: 23/04/2026 #7536
iOS Build 📬 Build number: 26.01.847711 #8477

Copy link
Contributor

@Withalion Withalion left a comment

Choose a reason for hiding this comment

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

The loading of QGIS auth DB didn't work on Windows. Same project worked on Android.

@gabriel-bolbotina gabriel-bolbotina force-pushed the feature/qgis-auth-db-sync branch from 9697596 to fe5119e Compare January 29, 2026 07:34
@github-actions
Copy link

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📬 Mergin Maps 62391 dmg Expires: 29/04/2026 #6239
linux Build 📭 Build not yet complete or failed.
win64 Build 📬 Mergin Maps 54321 win64 Expires: 29/04/2026 #5432
Android Build 📬 Mergin Maps 754951 APK [arm64-v8a] Expires: 29/04/2026 #7549
Android Build 📬 Mergin Maps 754911 APK [armeabi-v7a] Expires: 29/04/2026 #7549
iOS Build 📬 Build number: 26.01.849011 #8490

Upgraded qca version for windows functionality
Removed unnecessary lines of code in main
@gabriel-bolbotina
Copy link
Contributor Author

I upgraded the QCA version, and the authentication worked on Windows as well.

@github-actions
Copy link

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📭 Build not yet complete or failed.
linux Build 📬 Mergin Maps 62621 x86_64 Expires: 30/04/2026 #6262
win64 Build 📬 Mergin Maps 54441 win64 Expires: 30/04/2026 #5444
Android Build 📭 Build not yet complete or failed.
iOS Build 📭 Build not yet complete or failed.

Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Looks good. Please adjust the unit test so that it includes at least one happy-case scenario though - success import of auth data, but not directly via auth manager API (like now), rather through activeProject.load() so it tests your new code.

QString projectName = QStringLiteral( "auth-test.qgz" );
QString authFile = QDir( projectDir ).filePath( AUTH_CONFIG_FILENAME );

QgsApplication::initQgis();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to explicitly initQgis here?

@Withalion
Copy link
Contributor

Off topic: We should try to build the app with cmake 4.0+ now, last time QCA was the thing not letting us build it

@gabriel-bolbotina
Copy link
Contributor Author

Off topic: We should try to build the app with cmake 4.0+ now, last time QCA was the thing not letting us build it

I think you could create a ticket for it to be in the backlog 😉

@github-actions
Copy link

github-actions bot commented Feb 2, 2026

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📭 Build not yet complete or failed.
linux Build 📬 Mergin Maps 62671 x86_64 Expires: 03/05/2026 #6267
win64 Build 📬 Mergin Maps 54491 win64 Expires: 03/05/2026 #5449
Android Build 📭 Build not yet complete or failed.
iOS Build 📬 Build number: 26.02.850711 #8507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support to import QGIS auth DB from file

4 participants