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

Fix #51543 and make big forms more fluid by reducing calls to updateFieldDependencies #51836

Merged

Conversation

domi4484
Copy link
Contributor

@domi4484 domi4484 commented Feb 13, 2023

As of the current implementation when creating a new feature the attribute form behaves as "Always apply default values" checkbox was activated for all default values. After this pr the checkbox will be respected even when creating new features.

  1. Update default values only during setting the feature (or always if "Always apply default values" checkbox is checked)

  2. updateFieldDependencies only when resetting the form. This should make working with big forms more fluid as it spares a lot of loops trough all fields. Small downside for an edge case: if the dependencies are changed (for example default value expression on children layer) when a form is open, that form will not reflect the changes until reopened.

@github-actions github-actions bot added this to the 3.30.0 milestone Feb 13, 2023
@ponceta
Copy link

ponceta commented Feb 15, 2023

@domi4484 will this be backported to QGIS 3.28? This is a prio1 on our side, otherwise there's a no go on QGIS 3.28.

@andreasneumann
Copy link
Member

@domi4484 will this be backported to QGIS 3.28? This is a prio1 on our side, otherwise there's a no go on QGIS 3.28.

I agree - it would be really important to get such form improvements into 3.28.

But it will require lots of testing by users. Form code is a bit fragile ..

@domi4484
Copy link
Contributor Author

But it will require lots of testing by users. Form code is a bit fragile ..

This would be very welcome and almost mandatory for such kind of changes.
But I still have to look at the failing tests before

@domi4484
Copy link
Contributor Author

domi4484 commented Mar 1, 2023

@ponceta @andreasneumann from my point of view this pr is ready. If you could do some additional manual tests it would be very appreciated!

@andreasneumann
Copy link
Member

@domi4484 - this is still in a separate PR and not yet in master - right? I can do some tests tonight.

@domi4484
Copy link
Contributor Author

domi4484 commented Mar 1, 2023

@andreasneumann yes it is not merged yet

@andreasneumann
Copy link
Member

@domi4484 - I tested this PR.

I can confirm that the forms feel a lot "snappier" when the forms are big and have a lot of default values with "Apply on update". Good Work.

I can't tell for sure that there are no side effects but so far I haven't found any problems.

It would be great if another person that has complex forms could test as well. Otherwise I am definitely interested to see this land in master and backported to the LTR version.

@andreasneumann
Copy link
Member

@ponceta - do you have a chance to test this? I would really like to see this land in master, and then after quarantine in an upcoming 3.28 release.

For us, it makes a HUGE difference in our complex forms. Before it was very slow - now instant.

@ponceta
Copy link

ponceta commented Mar 2, 2023

I never build a custom QGIS with a PR yet but I do agree with Andreas, this is a great improvement.

@signedav
Copy link
Contributor

signedav commented Mar 6, 2023

@domi4484 Can you create an LTR PR for this?

@domi4484
Copy link
Contributor Author

domi4484 commented Mar 6, 2023

@signedav can you add the label for automatic backport? I don't have permissions for that

@ponceta
Copy link

ponceta commented Mar 20, 2023

Thank you a lot, that's a huge thing for us, tried today on QGIS 3.28.4-2 and it's working as expected!

@andreasneumann
Copy link
Member

@ponceta - thank you for the feedback. No troubles or side-effects were observed?

For me it is the same - quite a good improvement!

@ponceta
Copy link

ponceta commented Mar 20, 2023

Not yet at least! ;)

@spacerat1
Copy link

Hi,
is there a possibility to bring back the behaviour of #51543 (maybe as option 'Update fields only on creating new features') ?
I found the 'bug' very convenient. I have many forms where the user gets suggestions depending on most common use cases. For example: the user creates a "cable" -> the field for 'status' is set to 'new', but he can change it to 'existing'. (Just to iilustrate, there are many other fields)
With the last update, this field is not updating anymore.
So now i need to fill all fields either manually, or check the 'Always apply default values', but run the risk of losing all my 'manual changed data' if i refresh parts of the attribute table.

So it would be very convenient for me, to have the behaviour of 'Always apply default values' while creating new features, but let the data untouched while refreshing the attribute table.
(just like the 'bug' worked, but as an option)

@peterantkowiak
Copy link

Hi!
I have a similar issue as @spacerat1. I have developed drill-down forms for quick field work where the most probable default value is inserted into a field by running an expression on inputs further up in the form.
For this to work, I want the default value to be applied only once when creating the feature so that it is possible to record values differing from default and not delete all my data everytime I edit the entry afterwards.
This behavior worked beautifully with QGIS 3.28 / QField 2.6 until some update in March or April. With QGIS 3.34.1 / QField 3.0.7, expressions for defaults don't work at all unless I check the "apply defaults on update" box, which in turn resets all my recorded data to default when going back and editing a feature.

So I agree, an "apply defaults only when inserting feature" option would be highly appreciated.

@signedav
Copy link
Contributor

@peterankowiak and @spacerat1 I'm not sure if I understand your requirement. What you wish is the "Apply default value on update" behavior (live update in the form) on creating features, but not on changing values on existing features?

There is a clear bug #54446 (maybe introduced with this PR but not sure) that in the attribute table the values are updated with "Apply default value on update" even when no value changed (no update is performed). Maybe you would already have the needed behavior when this is fixed?

@peterantkowiak
Copy link

Thanks for your reply @signedav ! I will try to explain using an example.

I use QField to map birds as point features. The form has the following fields:

  • species code
  • breeding status code (e.g. territorial male, feeding pair, migratory flock etc.)
  • sex
  • number of individuals
  • observation time
  • observation duration
  • optional comment (e.g. details on plummage)
  • some optional fields for behavioral details only recorded sometimes
  • a bunch of other metadata that are editable, but usually filled in automatically, e.g. a session key, linking the observation to a weather record, type of inventory and observer details

As I need to be quick in the field (like 3 seconds per entry), I usually only enter species and breeding status code when creating a record, and pre-fill the rest of the form with defaults, based on expressions applied to those two entries. E.g. if I select "territorial male" -> sex = male, number = 1, observation time = now() and duration = 1, as this is the most likely combination. If other values occur (e.g. multiple individuals, observation in the past), I change them manually.
Assume I have saved the feature, keep adding other records, but after 15 mins I notice that the bird is still there. I want to update its record and set observation duration to 15 minutes. With the current behavior (irrespective of #54446 afaik), all fields are re-set to default, even if only a single field is changed, so I practically have to re-enter the whole observation, whose details I might already have forgotten. The alternative is to not use default values at all, which takes to much time.

For such use cases, where somewhat complex features need to be updated frequently, it would be ideal to have an option to apply defaults on feature creation only, but not again when updating a feature. As I mentioned in my previous post, this worked for me with older QGIS / QField versions, but stopped working at some point in spring 2023 (Was it possibly unintended behavior? Could it somehow be related to #51543?).

@signedav
Copy link
Contributor

You have to note that this part of the implementation is in the GUI - means QGIS and QField have independent implementations.

I think yes, what you require is to be able to apply this functionality on create feature only. Maybe you could make a feature request issue for that option.

@peterantkowiak
Copy link

peterantkowiak commented Dec 15, 2023

Thank you @signedav ! So if I understand you correctly, it would have to be a feature request for both QField and QGIS?

As I recall, it used to work (for above use case, in both QField and QGIS) when entering an expression in the defaults field but leaving the "apply defaults on update" box unchecked. In the recent versions, I observe the following behavior:

QGIS 3.34:
If "apply on update"-box is unchecked, defaults are applied once at feature creation, but not updated while filling the form, so no drill-down-form functionality (expressions like "now()" will work, but expressions based on values entered in other fields won't).
If "apply on update"-box is checked, defaults are updated throughout feature creation, but when modifying a single field of a feature later, all fields with the box checked are reset to default values.

QField 3.0:
Expression-based defaults only work if the "apply on update"-box is checked. In this case, the form behaves the same as in QGIS, so that defaults are applied to all fields with the box checked, when a feature is updated.

There might actually be a workaround using virtual fields:

  • Adding a virtual field which simply mirrors the secondary / dependent field that is pre-filled via expression.
  • Adding a condition to the default value expression so that it calculates default value based on current form entries only if the virtual field is empty and otherwise inserts the value saved in the virtual field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants