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

Consequent use of database settings in Export / Dataset Manager #936

Merged
merged 8 commits into from
Jun 21, 2024

Conversation

signedav
Copy link
Member

@signedav signedav commented Jun 21, 2024

Dataset Manager

Because you barely want to edit datasets on another datasource than the one you are currently using, Model Baker detects now automatically the source of your project (according to the selected layer). You cannot edit the datasource in the Dataset Manager anymore. We removed it because it leaded to confusion.

And what if my current layer is not a valid layer (e.g. a group or a WMS)?

Then it goes through all the layers and takes the first valid one.

And what if I have no project opened at all?

Then it takes the last used connection (used anywhere - not only in Dataset Manager)

Screenshot from 2024-06-21 13-04-34

Without an opened project

Screenshot from 2024-06-21 13-05-16

Embedded in the Wizard (takes the setting from the import)

Screenshot from 2024-06-21 12-59-45

Detecting a data source but without basket handling

Screenshot from 2024-06-21 13-07-25

Some thoughts I made here

I like that with Model Baker the QGIS Window is still responsive. But we had to decide, what functions in Model Baker Window (Wizard or Dataset Manager) should be triggered on interactions in the QGIS Window. In Validaton it's clear, there you change the db-settings by changing the layer. For the Dataset Manager we get now the db-settings from the current layer as well, but I avoided that it triggers a layer change (because this could lead to faults and confusions). In case you opened with a wrongly activated layer, you have to change the db-settings by selecting the correct one and close and open the Dataset Manager again.

Export

On export it does the same. It checks for layers in the current project and takes their data source. If no project or valid layer present, it takes the last used connection.

And on import / generate

After some thoughts I decided to adapt the behavior here. Although on schema import you usually create a new schema / gpkg at the moment it just takes the last used connection, what is not better than take the one from the active project. And additionally I assume, that mostly the other parameters (like the db / host etc. or on GeoPackage the directory) are the same like the one of the opened project.

Screencast.from.21.06.2024.14.44.33.webm

And on Generate you have no project opened usually. But better have it everywhere in the same way...

Resolves #847

Copy link
Collaborator

@gacarrillor gacarrillor left a comment

Choose a reason for hiding this comment

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

Nice move!

In terms of UX, I think it would be handy for the users to have a message box warning them of a db-settings change when selecting another layer in QGIS layer tree. For instance, "Would you like to change your connection settings to the newly selected layer (database: MyDB, schema: MySchema)? Yes/No". This would make them aware of the impact of QGIS interactions while having MB dialogs/dock widgets opened.

After they had seen this message box a couple of times, they will get used to potential db-settings changes, so the message box could offer a checkbox to avoid being shown in the future (e.g., "Don't show me this dialog anymore.")

Don't know if it's too much, though,

QgisModelBaker/gui/dataset_manager.py Show resolved Hide resolved
QgisModelBaker/gui/dataset_manager.py Outdated Show resolved Hide resolved
@signedav
Copy link
Member Author

In terms of UX, I think it would be handy for the users to have a message box warning them of a db-settings change when selecting another layer in QGIS layer tree. For instance, "Would you like to change your connection settings to the newly selected layer (database: MyDB, schema: MySchema)? Yes/No". This would make them aware of the impact of QGIS interactions while having MB dialogs/dock widgets opened.

After they had seen this message box a couple of times, they will get used to potential db-settings changes, so the message box could offer a checkbox to avoid being shown in the future (e.g., "Don't show me this dialog anymore.")

Don't know if it's too much, though,

Well I first had it like this - that the source gets changed when changing the layer - but then I decided against it. Now it's like this, that when the Dataset Manager opens, it takes the current layer and does not change anymore (on any changes in the layer-tree). My thoughts where:

  • User opens the Dataset Manager and sees the recognized source. Mostly they don't care at all where it comes from, but it is - mostly - the correct one. I see the use case where people want to click through the layer-tree and change the datasource accordingly as a quite rare one.
  • Mostly they have only one data source anyway in the project. But in case there are multiple and they click around in the layer tree, it could suddenly recognize another source and the user does not get it and might break something accidentally. There it would need your warning-popup or - as it is now - not a change of the datasource at all.

@gacarrillor
Copy link
Collaborator

In terms of UX, I think it would be handy for the users to have a message box warning them of a db-settings change when selecting another layer in QGIS layer tree. For instance, "Would you like to change your connection settings to the newly selected layer (database: MyDB, schema: MySchema)? Yes/No". This would make them aware of the impact of QGIS interactions while having MB dialogs/dock widgets opened.
After they had seen this message box a couple of times, they will get used to potential db-settings changes, so the message box could offer a checkbox to avoid being shown in the future (e.g., "Don't show me this dialog anymore.")
Don't know if it's too much, though,

Well I first had it like this - that the source gets changed when changing the layer - but then I decided against it. Now it's like this, that when the Dataset Manager opens, it takes the current layer and does not change anymore (on any changes in the layer-tree). My thoughts where:

* User opens the Dataset Manager and sees the recognized source. Mostly they don't care at all where it comes from, but it is - mostly - the correct one. I see the use case where people want to click through the layer-tree and change the datasource accordingly as a quite rare one.

* Mostly they have only one data source anyway in the project. But in case there are multiple and they click around in the layer tree, it could suddenly recognize another source and the user does not get it and might break something accidentally. There it would need your warning-popup or - as it is now - not a change of the datasource at all.

Yeap, for the Dataset Manager I see it's really clear now with the messages you've added. That gives a lot of context for users.
The comment was more general, pointing to any immediate change in an open MD dialog/dockwidget after a QGIS interaction (i.e., active layer changed). If I understand well, with this PR, an immediate db-settings change will only happen in the Validator's dock widget, since for other dialogs the automatic db-settings change will be blocked (thus, no need for a message box).

For the validation, I guess we can live with the current situation, and only in case you consider it'd be confusing for users, use a warning message box.

@signedav
Copy link
Member Author

If I understand well, with this PR, an immediate db-settings change will only happen in the Validator's dock widget, since for other dialogs the automatic db-settings change will be blocked

Yes.

For the validation, I guess we can live with the current situation, and only in case you consider it'd be confusing for users, use a warning message box.

Agree. I think there it's not a big issue but good to keep an eye on that. Thanks for your thoughts.

@signedav signedav merged commit edc7406 into master Jun 21, 2024
7 checks passed
@signedav signedav deleted the db-settings branch June 21, 2024 20:12
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.

Database Settings in Export / Dataset Manager / Validator
2 participants