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

B9 Validation #254

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

B9 Validation #254

wants to merge 9 commits into from

Conversation

Oddvocado
Copy link
Member

@Oddvocado Oddvocado commented Jan 23, 2025

Fixes #252
Fixes #255
Fixes #256

Corrected the properties "MOMODE,"MOMOPARK" and "MOMOALS" to be required if "DECCLMOT" has a value of 1. "COURSE" and "FRSTCHG" have also been updated to be required one "DECCLIN" has a value of 1 per documentation here --> https://github.com/naccdata/uniform-data-set/blob/main/forms/uds/b9/form_b9_ivp_error_checks_mc.csv

@Oddvocado Oddvocado marked this pull request as ready for review January 23, 2025 17:24
@ashleybot ashleybot self-requested a review January 23, 2025 18:02
@ashleybot
Copy link
Member

ashleybot commented Jan 23, 2025

@Oddvocado Nice catch on the correct RequiredIf for COURSE and FRSTCHG. I'm also noticing that COURSE has a wrong value for Not Applicable. Can you change the value of "Not applicable" to 8 in the FRSTCHGListItems list?

Screenshot 2025-01-23 at 1 06 53 PM

@Oddvocado
Copy link
Member Author

@ashleybot good catch! I went ahead and changed the radio button value from 6 to 8. Ready for another review!

@Oddvocado
Copy link
Member Author

Adding issue #255 to this PR as well

@Oddvocado
Copy link
Member Author

Oddvocado commented Jan 23, 2025

@ashleybot Took care of issue #255 with this commit -->1cf5619

Ready for another review!

I take that back. Still need to address some additional validation

@Oddvocado
Copy link
Member Author

Added issues #256 to this PR as well! Addressed in commit --> 002f70b

@Oddvocado
Copy link
Member Author

Oddvocado commented Jan 24, 2025

@ashleybot B9 could use additional enabling/disabling in the UI. For examples 12l should only be enabled when any question 12h-12k has a value of 1. This pattern shows up a couple additional times in the form. As of now PSYCHAGE only has model validation. Since these cards are high priority I was thinking we can create a new card for the UI? Let me know your thoughts!

image

As of now this PR is ready for another review. Should fix issues #252 #255 & #256

Copy link
Member

@ashleybot ashleybot left a comment

Choose a reason for hiding this comment

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

@Oddvocado Looks good! Good to bump to 4.6.15. I added a couple of commits:

  • [New item] I noticed PERCHAGE wasn't following the correct rules for validation - fixed
  • TIL adding a value tag will cause some odd behaviors with rendering. So, I removed the value attribute on the new PSYCHAGE input field and it renders properly.

Screenshot of incorrect rendering (the PSYCHAGE value was rendered to the hidden field rather than the number input, so it appeared as though it didn't save)

Screenshot 2025-01-24 at 2 06 07 PM

Fixed by removing value attribute

Screenshot 2025-01-24 at 2 10 19 PM

@Oddvocado Thanks for your attention to detail on this form. Let's go through it as a group on Monday and add some cards to the backlog together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants