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 style constraint save when strength is NotSet #58986

Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Oct 7, 2024

Fix #58431

I feel that the underlying issue remains (which is that setting the strength to NotSet removes it from the field but not from the layer and that the default strength is Hard) but since it was clearly made in purpose 8 years ago I am not keen to change it lightly.

@elpaso elpaso added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Oct 7, 2024
@github-actions github-actions bot added this to the 3.40.0 milestone Oct 7, 2024
@elpaso
Copy link
Contributor Author

elpaso commented Oct 7, 2024

@nyalldawson this fixes the issue #58431 but not the root cause and I'd like an opinion about the possibility to change
https://github.com/qgis/QGIS/blob/master/src/core/qgsfieldconstraints.cpp#L32
(see also: https://github.com/qgis/QGIS/blob/master/src/core/qgsfieldconstraints.cpp#L40) to be consistent with QgsVectorLayer::setFieldConstraint which doesn't remove the constraint if strength is NotSet.

The current behavior is not consistent.

@elpaso elpaso force-pushed the bugfix-gh58431-field-constraint-strength branch from cb175d5 to df947ef Compare October 7, 2024 12:31
Copy link

github-actions bot commented Oct 7, 2024

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 34531f7)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 34531f7)

@nyalldawson
Copy link
Collaborator

@elpaso

default strength is Hard

It's too long ago for me to remember exactly, but I suspect this was done in order to preserve API (ie before the introducing of soft constraints, all constraints where implicitly "hard" but we weren't explicitly tracking that).

I'm happy for you to tweak this logic as needed -- there's good test coverage there, so I'm confident we'll be alerted if you break anything in the process..

Fix qgis#58431

I feel that the underlying issue remains (which is that setting
the strength to NotSet removes it from the field but not
from the layer and that the default strength is Hard) but
since it was clearly made in purpose 8 years ago I am
not keen to change it lightly.
@elpaso elpaso force-pushed the bugfix-gh58431-field-constraint-strength branch from b0e9d50 to e67e777 Compare October 8, 2024 13:34
@nyalldawson nyalldawson merged commit 75c3ad0 into qgis:master Oct 11, 2024
30 checks passed
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!
Projects
None yet
2 participants