-
Notifications
You must be signed in to change notification settings - Fork 56
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
#82765 [10-10EZR]: Add/update a form submission's veteranDateOfBirth
if it's missing or blank
#16699
Conversation
lib/form1010_ezr/service.rb
Outdated
def post_fill_veteran_date_of_birth(parsed_form) | ||
return if parsed_form['veteranDateOfBirth'].present? | ||
|
||
user_dob = @user.birth_date |
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.
Curious if you want to log or send a metric to DD when this post-fill happens? That way we'll know how often it's taking place and for who
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.
That would be cool! Did you have something in mind for tracking the "who" in these scenarios? I'm fairly certain logging PHI is frowned upon unless it's for something important. Also, regarding logging, do you mean like a regular rails log or something more robust? The problem with rails logs is I think we'd probably run into an issue of tracking WHEN this occurs. What do you think?
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.
non-blocking comments/questions
lib/form1010_ezr/service.rb
Outdated
return if parsed_form['veteranDateOfBirth'].present? | ||
|
||
user_dob = @user.birth_date | ||
parsed_form['veteranDateOfBirth'] = user_dob if user_dob.present? |
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.
Almost feel like you don't need the if
since parsed_form['veteranDateOfBirth']
is nil
already when this line executes; you would just be setting it to nil
again, which it already is. One less conditional
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.
Agreed! It has been removed. Thanks Derrick!
…fairs/vets-api into 82765_1010_ezr_postfill_veteran_dob
…fairs/vets-api into 82765_1010_ezr_postfill_veteran_dob
Summary
veteranDateOfBirth
in the form params if it's missing or blank. We'll utilize thecurrent_user
's session by setting the form value to thebirth_date
value. This is to hopefully resolve the schema validation error related toveteranDateOfBirth
we've been seeing on and off.Related issue(s)
Testing done
What areas of the site does it impact?
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?