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

Preserve attribute type in dialog update #46172

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented Nov 23, 2021

If an attribute is set to "apply on update" and has a complex type (anything that's not losslessly convertible to string and back), QGIS will up to now replace it with an empty value in the attribute form.

This patch simply removes the string conversion.

Fixes #46158

@m-kuhn m-kuhn added Bug Either a bug report, or a bug fix. Let's hope for the latter! backport release-3_22 labels Nov 23, 2021
@github-actions github-actions bot added this to the 3.24.0 milestone Nov 23, 2021
@m-kuhn m-kuhn force-pushed the preserve_attribute_type_in_dialog_update branch from 9b03249 to ef56bf1 Compare November 23, 2021 06:56
@m-kuhn
Copy link
Member Author

m-kuhn commented Nov 23, 2021

@signedav this has been introduced in #31167
was there a requirement for this string conversion which I didn't see?

Copy link
Contributor

@nirvn nirvn left a comment

Choose a reason for hiding this comment

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

Good catch.

@nirvn
Copy link
Contributor

nirvn commented Nov 23, 2021

@m-kuhn , a test covering that would be good.

@m-kuhn
Copy link
Member Author

m-kuhn commented Nov 23, 2021

@m-kuhn , a test covering that would be good.

The simpler the patch, the likelier this request 😆

@m-kuhn m-kuhn added the Requires Tests! Waiting on the submitter to add unit tests before eligible for merging label Nov 23, 2021
@signedav
Copy link
Contributor

It is quite possible that it was unintentional.

@m-kuhn m-kuhn force-pushed the preserve_attribute_type_in_dialog_update branch from ef56bf1 to fecf05d Compare November 23, 2021 21:46
eww->setValue( value );
mCurrentFormFeature.setAttribute( eww->field().name(), value );
Copy link
Member Author

Choose a reason for hiding this comment

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

For reference, this is another unreported issue, live default values were not stored into the current form feature and hence taken into account by any related functionality.

@m-kuhn m-kuhn force-pushed the preserve_attribute_type_in_dialog_update branch from fecf05d to c1b8722 Compare November 23, 2021 21:49
@m-kuhn m-kuhn merged commit 112754f into qgis:master Nov 24, 2021
@m-kuhn m-kuhn deleted the preserve_attribute_type_in_dialog_update branch November 24, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Requires Tests! Waiting on the submitter to add unit tests before eligible for merging
Projects
None yet
3 participants