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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Djedouas
Copy link
Member

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 github-actions bot added this to the 3.40.0 milestone Jul 23, 2024
Copy link

🪟 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)

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.

None yet

1 participant