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

[Feature] New management of the N to M relations #4024

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

mind84
Copy link
Contributor

@mind84 mind84 commented Dec 15, 2023

This PR changes the way n-m relationships are handled.

Precondition
Two layers are explicitly related in an "n" to "m" relationship, i.e. in the QGIS project properties (Relations tab) both layers are related to a third "join layer" and in the Lizmap configuration (Attribute table tab) this join layer is defined as "pivot".

Changes on current behavior

Attribute table:

  • when the user opens the attribute table for the "n" layer, then the "m" layer attribute table is show as "child" instead of the pivot table. The related features shown are filtered based on the association between "n" and "m" layer in the the pivot table.
  • In the main toolbar of the "n" layer attribute table, the button to add records to the pivot no longer appears, and is replaced with an "add a new m feature" button. This allows the user to create a new "m" feature and directly associate it to the selected feature of the "m" layer. This means that after the new "n" feature is added, Lizmap automatically inserts the association record on the pivot table.
  • in the lizmap-feature-toolbar of the "n" layer, the button Create feature now shows only the layers that are 1:n related with the "n" layer.
  • in the lizmap-feature-toolbar of the "n" layer, the button Delete will also delete the records on all pivots involved in an "n" to "m" relation with the current "n" feature. A confirmation dialog is displayed, containing the list of all "m" layer associated with the "n" layer, in order to make the user fully aware of the changes that he is making. The deletion of record is done in a database transaction: the records in the pivot are deleted, then the "n" layer feature is deleted. If any step goes wrong, the transaction is rolled back and no changes will happen. The order of execution ensures that, if a foreign key relation was defined in the database, it will not be violated.
  • in the lizmap-feature-toolbar of the "m" layer, the buttons Create feature and Delete are hidden in order to avoid confusion for the user.
  • in the lizmap-feature-toolbar of the "m" layer, the button Unlink child deletes the corresponding pivot record.

The behavior to link existing features is not changed, thus the user should select the records to link, and then click on the button Link selected features.

Popup:

  • when a popup is displayed for "n" layer, the list of pivot feature associated with the record is replaced with the list of "m" layer features associated (the behaviour is the same one described in the attribute table)
  • the user can unlink an "m" record with an action on the lizmap-feature-toolbar (the behaviour is the same one described in the attribute table)
  • the user can delete the "n" record (the behaviour is the same one described in the attribute table)

Edition form:

  • when the edition form is opened from the attribute table ("add a new m feature" button), a message on the form informs the user that the new "m" feature will be linked to the "n" layer.
    This also happens if the user adds a new child "m" feature from an "n" feature while editing (i.e. editing a child from edition form).
  • Submit a form: if a pivot record is added during the "m" feature creation (previous case), the success/failure of the insertion is displayed along the same dialog message used to notify the user on the result of the "m" record insertion/update. The "m" record insert/update operation is independent from the corresponding pivot record insert action, so if the pivot insertion fails for some reason the "m" record is inserted/updated anyway, if no errors occurs.
  • Reopen form vs Create new feature options: when the "create new feature" option is selected, the new form that opens after the first submit still has the "link to pivot table behavior". On the other hand, if "Reopen form" is selected, this behavior is suppressed in order to avoid any duplication on the pivot table.
    In both cases, after submitting the form, if the "link to pivot table" is enabled, a notification message appears on the top of the form along with the main record insert/update message, informing the user that the new feature has been linked (or not) to the "n" layer.

Limitations/concerns:

  • since the insert/delete operation on pivot is automatically performed, the primary key of the pivot table must be serial
  • to delete records in a transaction I had to create a connection object first and pass it along the various delete methods. I performed several tests on my local environment, but I still don't know if this could lead to issues.
  • to show the "m" layer record I had to get the pivot records first to construct the wfs filter for the "m" layer. This could be avoided if in the wfs filter of the "m" layer I could specify something like filter = "id" IN (SELECT m_id from pivot WHERE n_id = 'foo'). Unfortunately this seems unsupported, isn't it?

TODO

Add the Link selected features option in the Popup

Funded by Faunalia

@github-actions github-actions bot added this to the 3.8.0 milestone Dec 15, 2023
@github-actions github-actions bot added editing form tests unit tests and docker configuration for tests labels Dec 15, 2023
@mind84 mind84 force-pushed the n_to_m_relations branch 3 times, most recently from 505e3a0 to 396d2eb Compare December 22, 2023 13:29
@Gustry Gustry added the sponsored development This development has been funded label Dec 22, 2023
@mind84 mind84 force-pushed the n_to_m_relations branch 2 times, most recently from 03ce578 to db49c9d Compare December 22, 2023 15:29
@mind84 mind84 force-pushed the n_to_m_relations branch 2 times, most recently from 5277449 to a96a97c Compare January 8, 2024 09:06
@mind84 mind84 force-pushed the n_to_m_relations branch 2 times, most recently from 547ff22 to 0b2493a Compare January 24, 2024 13:09
@Gustry Gustry added the run end2end If the PR must run end2end tests or not label Jan 24, 2024
@Gustry
Copy link
Member

Gustry commented Jan 24, 2024

The cypress error is coming from #4055 you can skip it

@mdouchin
Copy link
Collaborator

Thanks a lot for this proposal. It is huge, we think it will be easier to test against several projects if we merge it asap.

A question regarding your sentence

in the lizmap-feature-toolbar of the "m" layer, the buttons Create feature and Delete are hidden in order to avoid confusion for the user.

Did you meant in the lizmap-feature-toolbar of the "pivot" layer, ?

  • If so, ok for me, it is indeed logical
  • if not, you are supposing there is a priority between n and m layers, with n > m ? I think it depends on the data. For example, for bus stops / pivot / bus lines, the user should be able to create and delete objects of the bus AND the line layer.

@Gustry
Copy link
Member

Gustry commented Feb 13, 2024

@mind84 Maybe you have notice, we have started to autogenerate the documention, for JavaScript in #4185 which is temporary available on https://docs.3liz.org/lizmap-web-client/ and also for PHP in #4200 (we will organize permanent URL a little bit later, it's just a work in progress for now).

Can you, for just newly added function in JS/PHP add the corresponding docstring ? For Javascript, this kind of helper https://developer.wordpress.org/coding-standards/inline-documentation-standards/javascript/ or the PR #4185

We will make this good practice step by step on existing function.

I guess you need to rebase.
Thanks

@mind84
Copy link
Contributor Author

mind84 commented Feb 14, 2024

Hi @mdouchin,

Thanks a lot for this proposal. It is huge, we think it will be easier to test against several projects if we merge it asap.

In this regard, I think it's a sort of "breaking change" and I am aware that the tests are not trivial at all. If there is anything I can do to support you in this process let me know.

A question regarding your sentence

Take for example the test project of this PR (n_to_m_relation.qgs):

  1. open attribute table for the Natural areas (n layer) and select a record
  2. in the Birds children tab (m layer) you can see that on these records the button Delete and Create feature are hidden.
    Selection_013

If you want to add a new bird as "child", you can add it from the corresponding button on the n layer table.

Screenshot from 2024-02-14 08-38-34

This choice is mainly based on the fact that the user is editing the n layer. I think that by adding the "child" feature (m layer) from the "parent" table, the user is more aware of what he is doing and I think this is also more intuitive for him/her.

Same for Delete button, the presence of both Unlink and Delete buttons may lead to confusion IMHO, so for these reason I've removed the Delete for the m layer. If an user wants to delete a bird, I think this must be done from the bird attribute table (i.e. from the currently n layer and not from the m layer).

About the pivot table, user cannot act on it anymore since it is hidden.

Thanks

@mdouchin
Copy link
Collaborator

Hi @mind84 thanks for your answers. I agree with you this will help the users.

Copy link
Collaborator

@mdouchin mdouchin left a comment

Choose a reason for hiding this comment

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

Thanks a lot !

@rldhont rldhont merged commit afe007b into 3liz:master Feb 20, 2024
12 checks passed
@mind84 mind84 deleted the n_to_m_relations branch February 20, 2024 16:02
@mind84
Copy link
Contributor Author

mind84 commented Feb 20, 2024

Thanks a lot !

Thanks to you guys for taking the time to review this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editing form enhancement run end2end If the PR must run end2end tests or not sponsored development This development has been funded tests unit tests and docker configuration for tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants