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.

Sign up for GitHub

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

Jump to bottom

Add workaround for checkboxes to ValueConflictValidator #1058

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Add workaround for checkboxes to ValueConflictValidator #1058

merged 2 commits into from
Jul 18, 2024

Conversation

jochenklar
Copy link
Member

@jochenklar jochenklar commented Jul 15, 2024 •
edited
Loading

I investigated #903 further and I think I found a case when "normal" usage can lead to states which cannot be saved anymore:

  • A user saves a checkbox with collection_index=0
  • Later, an option is added to the option set before the first option
  • Now, the value above value will have the wrong collection_index
  • When the user then tries to save the new option with collection_index=0 the conflict validator will trigger.

This PR tries to loosen the conflict validator for checkboxes:

  • When creating the validator checks if the value comes from a checkbox, and then adds the option for the value to the query which checks if there is already a value in the database.
  • Since validators for the value do not know about the question, the front end needs to submit widget_type just like it submits value_type and units, but widget_type does not need to be saved (or be part of the model).

@jochenklar jochenklar requested a review from MyPyDavid July 15, 2024 13:55
@jochenklar jochenklar self-assigned this Jul 15, 2024
@jochenklar jochenklar added this to the RDMO 2.2.0 milestone Jul 15, 2024
@jochenklar
Copy link
Member Author

The bug is actually pretty easy to reproduce (in dev-2.2.0).

  • Answer any question with checkboxes and check any option, but the first.
  • Go to management and just change the order of options in this optionset used before.
  • Try to check a checkbox above the one selected before. This will trigger the validator.

@MyPyDavid
Copy link
Member

does this text need to be adapted for the different types of conflict "newer version of this value" or "existing value"?

rdmo/rdmo/projects/templates/projects/project_questions_value_errors.html

Line 7 in 2406492

This field could not be saved, since somebody else did so while you were editing.

@jochenklar
Copy link
Member Author

No, I don't think so, this is the main case for the validator.

MyPyDavid
MyPyDavid approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@MyPyDavid MyPyDavid left a comment

Choose a reason for hiding this comment

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

alright, thanks for the fix of the checkboxes and the nice tests!

@jochenklar jochenklar merged commit e2feb6c into dev-2.2.0 Jul 18, 2024
17 checks passed
@jochenklar jochenklar deleted the fix_conflict_validator branch July 18, 2024 10:08
@MyPyDavid MyPyDavid mentioned this pull request Aug 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MyPyDavid MyPyDavid MyPyDavid approved these changes

Assignees

@jochenklar jochenklar

Labels
None yet
Projects
None yet
Milestone
RDMO 2.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jochenklar @MyPyDavid