-
Notifications
You must be signed in to change notification settings - Fork 43
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
PIMS-2000: Make non-optional checkboxes mandatory #2640
Conversation
🚀 Deployment Information The React APP Image has been built with the tag: |
Code Climate has analyzed commit 52b753b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 93.1%. View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, this is a good fix.
Before we approve and merge, I wouldn't mind getting Josh's opinion on whether he wants to be restricted on all these fields.
For example, if we look at the list to get into Approved for ERP:
It feels like you wouldn't likely check off both Appraisal fields or both Consultation fields at the same time, but you can't save the project if they aren't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contract is conditional is currently required, but that should be an optional taskUpdated the migration to handle this.Noticing that the required field error message will stay between statuses, even if the new field is not required. I wonder if there's a way to force it to reset the error state.Solved this one. Change pushed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the two changes noted in the last comment.
Everything appears to be working as expected.
Can't progress to a new project status without these required fields now.
Co-authored-by: dbarkowsky <[email protected]> Co-authored-by: Dylan Barkowsky <[email protected]>
🎯 Summary
PIMS-2000:
🔰 Checklist