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

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
281 stars 198 forks source link

[bug] Certificate of Eligibility | VA loan number field should be a text input field #45026

Closed joshkimux closed 2 years ago

joshkimux commented 2 years ago

Value Statement

As a user I want to enter the correct VA loan number on my Certificate of Eligibility application So that I can submit my application without errors that may result in an undesired outcome.


Background Context

Issue Summary from Josh No validation exists for the VA loan number field which can afford situations like this where a Veteran can submit a negative number. Context from @scottymeyers: > the LGY service didn’t provide us with any validation information. It’s just “string”, “boolean”, etc. We’ve tried pressing for min/max lengths and stuff, but they didn’t provide any additional information to us. We may be interested in seeing if there's any baseline validation we can try like preventing negative numbers. @jsokohl are you familiar with any other rules we could place here from the research you conducted? https://user-images.githubusercontent.com/14154792/181587366-067e3c79-5940-42fb-b210-431485528ada.mov ![image](https://user-images.githubusercontent.com/14154792/181586577-fec031ad-d6a7-4fd1-ba0b-96316ebc668f.png)

Updated acceptance criteria

Designs and Build Notes

Tasks


How to configure this issue

saderagsdale commented 2 years ago

Update AC @saderagsdale

saderagsdale commented 2 years ago

@joshkimux @jsokohl Is the AC appropriate for this issue? Also, do we need a design snapshot for the fix we discussed?

joshkimux commented 2 years ago

@saderagsdale sorry for the late reply, this looks wonderful! Some notes:

The form field type is the number input (VA design system) I believe, which is desired (@jsokohl to confirm).

I don't think we need a design snapshot, but we probably need a list of error messages. @jsokohl would love your wisdom here.

I found this word doc on Veteran Loan Identification Numbers (VIN), but am unsure if this is the same thing we're addressing here.

The VA LIN is comprised of the following 12 digits with three hyphens to separate the four categories of the 12 digits. For example: XX-XX-X-XXXXXXX

If this is the case, we may be interested in providing hint text too.

jsokohl commented 2 years ago

We should allow for dashes (hyphens, ok). Ideally the app will just strip those items out if not needed. I strongly suggest not putting hint text in, either -- let's let the software comply with the user, rather than the other way around.

joshkimux commented 2 years ago

Awesome! A couple more clarifying questions (just in case):

jerekshoe commented 2 years ago

So I looked, and for SSN, we accept input with or without dashes. We remove the dashes (this includes ones at the beginning of the text input) when we store the value, so the form submission won't include dashes. We can probably do something similar for VA loan number.

jsokohl commented 2 years ago

We should allow for dashes (hyphens, ok). Ideally the app will just strip those items out if not needed.

Rethinking this ticket--IMO the field shouldn't be restricted to a number field with concepts of negativity or positivity. It's merely an identifier. To that end, I now think we should call this Not a Bug. I"m happy with adding the hint text if you think it's advisable, @joshkimux -- but, again, we're forcing users to comply with the implementation model, and that's generally a Bad Thing. Sometimes it's unavoidable, but especially for this field, where (as far as I've been able to determine) the loan number can vary in length, perhaps we don't need to mention the length.

Let's do use the same approach as @jerekshoe mentioned--very cool!

joshkimux commented 2 years ago

@jsokohl I've updated the acceptance criteria and the name of the ticket to reflect these changes 🥳 , feel free to update the title/description to what you believe to be most appropriate.

we're forcing users to comply with the implementation model, and that's generally a Bad Thing

I agree that focusing users to comply with an implementation model is a bad thing 💯

Still curious to get your thoughts on:

Apologize if my framing seemed like something I was advising; I meant to frame this as a question as you're our content lead 🙇

Edit: @saderagsdale , would Matt have any answers to these questions by any chance in regards to rules out of our control (that we might be able to help the user validate)?

jsokohl commented 2 years ago

@joshkimux sorry to be late in responding. Some answers to your bullets:

Hope that helps!

Mottie commented 2 years ago

I found this LGY document through a Google search that explicitly describes the VA loan ID number (LIN)

The VA LIN is comprised of the following 12 digits with three hyphens to separate the four categories of the 12 digits, for example XX-XX-X-XXXXXXX

Note: I had to open the word document on a Windows computer to get the formatting to work right. Here is the full text screenshot in way too much detail:

Full LIN details ![Overly detailed description of the VA loan ID number describing what each digit represents](https://user-images.githubusercontent.com/136959/185145918-ac3cc2ba-ff21-4148-baff-0c9eacc32fc0.png)
jsokohl commented 2 years ago

Here's a screenshot from a WebLGY document that shows the numbers as samples: LIN example

This comes from a streamline refinance PDF.

I would suggest that we allow for numbers with or without hyphens or spaces, and that we do check for the number of numbers. If the numbers don't match the 12 digits, then we show an error message (something like "The loan number should be 12 numbers.").

I do know that my loan identifier was I think eight numbers. Maybe there's a difference between what LGY uses and what a Veteran sees? At this point, I say we go with what we know. I would suggest a change to the AC.

joshkimux commented 2 years ago

calling this item the VA loan number is better than a VA LIN, simply because, over the years, it's a field (and an identifier) that has changed. I reviewed my loan number (and had to descend into musty repositories) and found it to be called a Loan account number.

@jsokohl Agree 💯 I don't recall reading anyone suggesting for it to be called a LIN (which would likely violate our content standards)

I can say with a fair degree of certainty that Veterans have no idea where there loan number is nor where to find it. It's on a piece of paper in their basement in a box, probably.

@saderagsdale this feels like something we might want to put a pin in for the future (if we can't look into it now); if there's research to verify this, we may need to explore if there are any ways to help Veterans find this number (if any methods exist through the VA or otherwise)

I do know that my loan identifier was I think eight numbers. Maybe there's a difference between what LGY uses and what a Veteran sees? At this point, I say we go with what we know. I would suggest a change to the AC.

Could we track this question as a known unknown? I'll write that into the AC. @jsokohl don't hesitate to update the main issue in the future! Feel free to make updates to AC as you see fit 🙏

Mottie commented 2 years ago

I'm working on this now, and I was planning on changing the input into a text input and only allow numbers & dashes. There won't be any other restrictions like number of digits. Sound good?

joshkimux commented 2 years ago

That sounds great as an intermediary solution to me @Mottie 🥳

jsokohl commented 2 years ago

Can we also allow spaces? People sometimes substitute spaces for dashes in strings.

Where did we end up with hint text? Yay or nay? Something like "A loan number is 12 numbers."

joshkimux commented 2 years ago

@jsokohl I updated the AC; hint text depends on if we're certain that the loan number is 12 numbers as it seemed like there were still outstanding questions around the consistency of the length

jsokohl commented 2 years ago

@joshkimux Gotcha!

Mottie commented 2 years ago

The fix to make the loan number field into a text input and allow dashes (but not at the beginning) and spaces has been merged. It should be available for testing in staging soon

joshkimux commented 2 years ago

https://user-images.githubusercontent.com/14154792/185693958-e1f9e8a5-7e5d-487e-8d73-d64d9b5160f2.mov

@Mottie It looks like error will still be triggered when you start the field with a 1, not sure what's causing it though :(

joshkimux commented 2 years ago

Bumping this, we got a response from the content team here

We heard back from our stakeholder — he reached out to 2 SMEs, and I'm pasting their responses below. Key takeaways from my reading:

LIN and loan identifier are the same, but loan account number is different and more tied to the lender loan numbers are always 12 digits (numbers only) Let us know if you need more info here!

First response: “I would say Loan Account number would be more of a reference to the lender’s loan number which is variable. It could help us track down a loan nonetheless but the term “loan identification number” or LIN or the loan identifier I would expect to mean the same thing.

VA Loan numbers always take the format: xx-xx-x-xxxxxxx the first two sets of digits are jurisdictional locators (ex: 40-40 means New Mexico) the middle digit tells us what kind of loan it is (direct, guaranteed, refunded, partial claim) and the last seven digits are the identifier.”

Second response: “The VA loan number from VA (sometimes shortened to LIN) is in this format xx-xx-x-xxxxxxx. No letters, just numbers and is the LIN number in VA records. At a time before computers, we would sometimes see the last seven digits and the state afterwards when the number was hand written instead of the full 12 digits. (xxxxxxx NM for Heather’s New Mexico example) Also, while generally loans in the past 20 plus years have had the same two digits begin a loan number (40-40-x-xxxxxxx), some LINs previously had the same number of digits; however, when there was more than one VA office in a state (CA, TX, or NY for example) the LIN will have a different XX and XX to begin the loan number and define the office location. The state would be the first two numbers and is consistent with what is used now. I don’t remember an office code for the second set of numbers, but they are rare now as most of these loans would be paid in full.”

saderagsdale commented 2 years ago

@jsokohl @Mottie hi both. What are next steps for this? Looks like Josh got back with some feedback from content peeps.

jsokohl commented 2 years ago

Thanks to @joshkimux for sussing this out.

We know that we can in effect ignore spaces and dashes. We can say, with some confidence, that all loan numbers (for this data field) are 12 numbers. My recommendation? We let them enter anything they want to. If we want to count for 12 numbers and ignore those spaces and dashes in the field, then we can give them the error message, "A loan number contains 12 numbers." In addition, and to Josh's initial great recco, add the hint text such as, "This number has 12 numbers." That would be my recommendation.

saderagsdale commented 2 years ago

@jsokohl Can you ping me when you've had a chance to finalize this?

jsokohl commented 2 years ago

@saderagsdale Sure.

  1. Add hint text after the label and before the field.
  2. Hint text is "This number is 12 numbers long."
  3. If the user enters more than or fewer than 12 numbers, display the message, "Please check that your VA loan number is 12 numbers long."

That's my recommendation.

joshkimux commented 2 years ago

Looks good! Thanks so much @jsokohl and @saderagsdale 🥳

Mottie commented 2 years ago

Finished up work on this, it ~should be~ is available in staging to test ~later today~.

joshkimux commented 2 years ago
Screen Shot 2022-09-19 at 10 58 17 AM Screen Shot 2022-09-19 at 10 58 27 AM

Looks great @Mottie !! 🥳 The only thing here that might be of interest is someone could submit the same loan number twice. Highly unlikely (I hope)?

joshkimux commented 2 years ago

If that's out of scope for this or too specific, we can close this out; I'm not sure what the minimum level is required from a QA perspective to launch for cases like these

saderagsdale commented 2 years ago

@joshkimux good catch! It wasn't part of this ticket but a worthy consideration. @Mottie do you agree that this is good validation to put in place?

Mottie commented 2 years ago

Yeah, I think we should check that the numbers are unique.

jsokohl commented 2 years ago

If we do that, then we need to be clear that one of the numbers might be wrong--we don't know if the first one is errant, or the second one, right?

"This number matches one you've already entered."

Something like that. In a multiloan situation (similar to my mockup), the latest one the user is entering might match one that was entered eight years ago.

saderagsdale commented 2 years ago

@Mottie should we ticket this separately?

Mottie commented 2 years ago

naw, we can work off of this one.

saderagsdale commented 2 years ago

@Mottie cool beans. I think that's the last step for this before closing it. Let me know if you agree.

Mottie commented 2 years ago

VA loan number unique validation is now in place in staging. It will detect duplicates and will ignore dashes & spaces.

joshkimux commented 2 years ago

Thanks so much @Mottie , let's close this out 🥳