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 checklist with reselection from old values #4057

Merged
merged 2 commits into from
Jan 11, 2022
Merged

Conversation

pxpm
Copy link
Contributor

@pxpm pxpm commented Jan 6, 2022

WHY

BEFORE - What was wrong? What was happening before this PR?

#4056 - i just wrote it the :|

AFTER - What is happening after this PR?

checkboxes get reselected.

HOW

How did you achieve that, in technical terms?

the JS was only registering the change event for the checkboxes and it was the only place where the checking process would run (after a change).

I refactored the JS a bit so we could still run the code on change, but also on field initialization we check if any of the checkboxes are checked, it they are we trigger we manually trigger the "auto-checking" for them.

Is it a breaking change or non-breaking change?

Non breaking, probably it could go into 4.1 too ?

How can we test the before & after?

Before: Go to user create in demo, select superadmin and notice the permissions get automatically selected. If you don't fill any more fields you will get a validation error, and will notice that the role was still selected but all permissions are gone.

In this PR, the permissions are re-selected.

@tabacitu tabacitu merged commit 458d5bb into 4.2 Jan 11, 2022
@tabacitu tabacitu deleted the fix-checklist-old branch January 11, 2022 05:27
@tabacitu tabacitu mentioned this pull request Jan 11, 2022
@tabacitu
Copy link
Member

Tested, was happening, now it's not. In the process, discovered ANOTHER bug in checklist_dependency, that I think we introduced in 4.2. Because it happens both on the 4.2 branch and this PR: when you clear everything in roles and permission, that doesn't get saved.

Added bug about it here - #4071

@tabacitu tabacitu mentioned this pull request Feb 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] checklist dependency disabled checkbox values dont persist
2 participants