Keep attributes when adding 1-N related features with a multi-edition attribute form

[Djedouas]

Description

The bug is described in this PR, which has been merged, and reverted recently.

Therefore, the bug is back again.

In this PR I want to discuss the right way to fix the bug (and propose an approach), and decide if we should fix it, or if we should re-think the behavior because the UX might be wrong.

The bug

Context

We have a layer A (say buildings - Point) with a 1-N relation with layer B (say pictures - No geometry, one field for file path).

Each building can have multiple pictures.

Intent

The user wants to add the same picture to many (say 4) buildings.

UX

It's possible with QGIS, and we see in the code that the intent is to create 4 identical picture features and reference them to each building.

Bug

Only one (the first) of the new picture features have the correct attributes:

Cause of the bug

The first picture feature is added with QgsVectorLayerTools::addFeatureV2() in QgsGuiVectorLayerTool implementation, which delegates the feature addition to QgsFeatureAction::addFeature(), which delegates the feature addition to QgsVectorLayerUtils::createFeature() and the attribute form QgsAttributeForm::featureSaved, etc.

In this process, the attributes are set asynchronously through signals.

It means that when the user writes the file path in his new picture feature attribute, the other 3 picture features have already been added (without attributes, copied from the bare first picture feature).

1. The first picture feature is added (AddFeatureResult::Pending)
2. The 2nd, 3rd, 4th feature are added
3. The user finishes writing the attribute and clicks OK in the attribute form
4. The first picture feature is updated with the attributes

Should it be even possible?

This kind of use-case might raise the question: "Maybe I need an N-M relation? One picture can be related to more than one building, and one building can be related to more than one picture."

So should QGIS really give the user the possibility to multi-edit a building to add a child feature on a 1-N relation?

Fix the bug

I first fixed the bug by making the add-a-child-popup modal, but it created a regression.

Ways to fix:

1. wait for the attributes to be filled-in and the form to be validated (first reverted PR, and other ways I tried to no avail to catch the attributes as early as possible)
2. add the first feature with the attribute form, and the other features without showing the form but using the previous attributes (this PR, but not completely satisfied with this way)
3. other ideas welcome

ping @nyalldawson @nirvn @Guts

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 7010edc)

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[Guts]

Hi there,

The end customer is concerned that the initial anomaly may return to the LTR. Can we move forward on Jacky's proposal @nyalldawson @nirvn?

[Guts]

Anyone else more available can have a look?

@3nids (since you commented here #57474 (comment)) maybe?

[nirvn]

I'll try to have a look today

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[Guts]

Hello Mathieu,

Did you make it?

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.