BE | General Main | Check behavior of Preferred Name (non-ascii issue)

mtcA6 commented 10 months ago

background

we had a call with VA Profile and MHV and discussed preferred name field. it really should be preferred first name field and spaces are not permitted. we should make sure our validation rules are matching what MPI by way of VA Profile will accept AND we should update the helper text to be more specific about those rules.

tom: I'm thinking we should open a ticket to make sure that the http://VA.gov|VA.gov validations are in line with MPI/VAPro validations so we can display a more meaningful error message to the user.

Slack Message

Tasks

[x] ~Add validation rule that prevents spaces.~
[x] Va profile swagger docs will be updated on 2/13 to reflect allowing empty spaces.
[x] Update validation to allow spaces

Acceptance criteria

The following validation rules are in place:

[x] ~Max character length of 25~
[x] ~Must only contain alpha, -, acute, grave, diaeresis, circumflex, tilde~
[x] ~No spaces~

Reconfirmed by Josh L. on 1/17 new criteria:

minLength: 1
maxLength: 25
Allowed Characters: (case insensitive)
alpha, -, ' (apostrophe), acute, grave, diaresis, cirumflex, tilde, space.
[ ] Follow up and resolve the Preferred Name production issue

tpharrison commented 9 months ago

@andaleliz @adamwhitlock1 The preferred name validation rules have been updated. They're a bit more strict than before (see the acceptance criteria). Let me know if you have any questions.

mtcA6 commented 9 months ago

@tpharrison ooooo missed this, Monday morning dentist really slowed me down here! Do I need to make a ticket for FE stuffs here? I know we have the design ticket do we need to wait on the design side before you can start to work on things @adamwhitlock1? If the answer is no then I'll create the next ticket and work on getting that slated

@Samara-Strauss do we need to reach out to Carnetta and MHV w/the rules?

Samara-Strauss commented 9 months ago

@mtcA6 I realized we never heard back from Danny Reed with final confirmation on the rules, so I reached back out and am hoping he can confirm once and for all what MPI accepts. That should be enough to get Carnetta a final answer on what's possible and what we support.

tpharrison commented 9 months ago

@mtcA6 At this point, our preferred name validation rules match VA Profile's validation rules.

Samara-Strauss commented 8 months ago

Update: In a MVH/VA Profile meeting on 1.17.24, we received confirmation that spaces are accepted by MPI. So, we need to update our logic accordingly.

Per @mtcA6 on Slack we hope to make this update in the sprint starting Jan 31.

tpharrison commented 8 months ago

Started a thread around preferred name validation rules.

tpharrison commented 8 months ago

@mtcA6 Barbie Flowers confirmed that a space is allowed in a preferred name. The VA Profile swagger docs will be updated on 2/13 to reflect this validation rule.

cc/ @Samara-Strauss

tpharrison commented 7 months ago

@mtcA6 The PR to allow a space has been deployed to staging and prod and I noticed that VA Pro's documentation has been updated for preferred name to include a space...

Must only contain alpha, -, acute, grave, diaeresis, circumflex, tilde, or space(' ')

Since the FE validation rules haven't been updated yet, I can't test through VA.gov.

I noticed that apostrophe is listed in the original ticket, but it isn't listed in the VA pro documentation. I am going to verify the VA Pro validation by calling their API directly in QA and testing it with different characters.

mtcA6 commented 7 months ago

Thanks tom keep me posted, i think we should go back w/any diffs we find

tpharrison commented 7 months ago

@mtcA6 I ran some update tests directly against the VA Profile API. Here are the results:

<!DOCTYPE html> Character	Example	Response
blank (' ')	mr robot	OK
dash (-)	mr-robot	PRFN102: The following characters returned error PRFN102: must only contain alpha, -, acute, grave, diaresis, cirumflex, tilde (case insensitive), or space (' ')
apostrophe (')	mr'robot	PRFN102: The following characters returned error PRFN102: must only contain alpha, -, acute, grave, diaresis, cirumflex, tilde (case insensitive), or space (' ')
acute	mistér	MVI203: MVI returned acknowledgement error code PNUPDATEV0002 with error detail: Rejected due to the business rules
grave	mistàr	MVI203: MVI returned acknowledgement error code PNUPDATEV0002 with error detail: Rejected due to the business rules
diaeresis	mistër	MVI203: MVI returned acknowledgement error code PNUPDATEV0002 with error detail: Rejected due to the business rules
circumflex	mistâ	MVI203: MVI returned acknowledgement error code PNUPDATEV0002 with error detail: Rejected due to the business rules
tilde	mistã	MVI203: MVI returned acknowledgement error code PNUPDATEV0002 with error detail: Rejected due to the business rules

mtcA6 commented 7 months ago

ok switching to teams will include you in a message to (i think the person's name is Josh, going to start w Josh on VA Profile team and go from there) - [edit] confirmed it's josh i just scrolled up and younger me took care of today me and the name is in the body of the ticket.

tpharrison commented 7 months ago

Ok - The contact to reach out to is Josh Lindsey.

BTW - The tests were run against VA Profile's QA environment.

tpharrison commented 7 months ago

After our VA Profile meeting, I'm going to reach out to the mobile team to let them know about this change.

tpharrison commented 7 months ago

@mtcA6 Do you want me to close this ticket and open another to review any VA Profile changes?

mtcA6 commented 7 months ago

@tpharrison Honestly, I'm not sure. Maybe we should move this to blocked? Maybe we use this as a use case for definition of ready convo

Did VA Profile commit to changes? They just said they were going to research, I guess that means the list above is accurate and they expect to support those options. I'm hesitant to close this because I really want that chart from the comments above, for all of the things they supposedly accept, to have "ok" in the response column.

This is probably my fault for breaking the work down too much pre-emptively. This is a case where in my past life I'd have written a single user story and jira would have done a lot of legwork for us with the way it organizes tickets and tasks:

As a user I want the ability to update my preferred name with the following formatting criteria:

minLength: 1
maxLength: 25
Allowed Characters: (case insensitive)
alpha, -,
' (apostrophe), acute, grave, diaresis, cirumflex, tilde, space.Does our BE accept those alternatives listed above?

Then I would have expected FE/BE/Design to coordinate by adding their sub tickets/tasks to that user story (that I wouldn't have juggled or looked at):

BE - needing to update validation rules
FE - needing a story to update validation rules
Design - providing updated designs/content

I would have pulled this single story into the sprint and expected the team to ensure it was completed by the end of sprint without my need to juggle 3+tickets.

Maybe this is what we should do in the first sprint of March, because honestly it seems like VA Profile is going to have to implement things before we can make our changes.

I know that's a really unsatisfying answer, I think we leave this open and look at it in retro.

cc: @ACParker89

tpharrison commented 7 months ago

@mtcA6 I actually think this is on me. I should have added a task to verify VA Profile validation rules in QA before adding and deploying them. I just assumed that things were working when VA profile confirmed that the validation rules were working and the Swagger docs were updated earlier this week.

The extra validation rules are in effect at the Vets API level, but the FE validation rules haven't changed (they're more strict), so it shouldn't cause any behavior changes or issues in prod. I'm will reach out to the VA mobile team just to make sure this doesn't cause any issues for them (I don't think it will).

I think moving it to blocked is fine (along with Adam's related FE ticket). cc/ @adamwhitlock1 @ACParker89

mtcA6 commented 4 months ago

@ACParker89 this is the other blocked ticket, VA Profile didn't have an update yesterday, there's work they have to do to support non-ascii characters for this to function correctly.

I keep notes in the team canvas for our two weekly calls. I try to check every two weeks when the VA Profile call happens, sometimes I forget. They're supposed to update us on where this sits.

tpharrison commented 4 months ago

@mtcA6 @ACParker89 I tested these in staging with the same results. None of these special characters are allowed.

mtcA6 commented 3 months ago

checked on the 6/13 VA Profile communications call, still no updates on this.

mtcA6 commented 2 months ago

@tpharrison latest feedback from the backend team, the test user you used had data in VISTA and there is some business rule that prevents users w/data in vista from updating their preferred name (something like that)

so, question, can you run the script on a user where preferred name is NULL and see if the tests fail?

tpharrison commented 2 months ago

@mtcA6 I'll do it today.

department-of-veterans-affairs / va.gov-team