Repository navigation
Conversation
There's currently quite a few places where SSN inputs have spaces in them. There's no reason for not properly normalizing them in one place.
`message` property is deprecated in favour of `error`
ca509e8 to
9fe925b
Compare
b116531 to
08fd17a
Compare
…rimming.0 with tag feature-ssn-schemas-with-input-trimming
…nput-trimming.0 with tag feature-ssn-schemas-with-input-trimming
This reverts commit 96ee776.
01ebe59 to
3a607a2
Compare
3a607a2 to
5799883
Compare
ba859bf to
4a0e0f5
Compare
|
|
||
| export const zodSsn = z.string().refine(isSsnValid, { message: 'Invalid ssn' }).toUpperCase() | ||
| export const ssnSchema = z | ||
| .string({ error: 'SSN must be valid' }) |
There was a problem hiding this comment.
Tässä on nyt sellainen että tää piilottaa varsinaisen virheen, eli esim. sen että kenttä puuttuu. Sieltä tulee vain että "SSN must be valid" sen sijaan että se sanoisi esim. "expected string, received undefined". Samaten tässä tulee erikoinen virheilmo jos esim. joku syöttää sinne jonkun väärän tyypin - se vain sanoo että "SSN must be valid" itse virheviestissä. Sieltä virheen syövereistä löytyy se tyyppivirhe, mutta mun käsityksen mukaan tätä ei kannattaisi olla.
Ts.:
| .string({ error: 'SSN must be valid' }) | |
| .string() |
on informatiivisempi.
| export const ssnSchemaWithAgeChecks = z | ||
| .string({ error: 'SSN must be valid' }) | ||
| .trim() | ||
| .refine(ssn => isSsnValid(ssn, true), { error: 'SSN over 100 years old' }) |
There was a problem hiding this comment.
Tämä aiheuttaa nyt sen että jos tuonne syöttää vaikka tyhjän merkkijonon, virheeksi tulee aina "SSN over 100 years old". Lisäksi meillä on koodia joka testaa onko hetu tulevaisuudessa, joten tuo viesti johtaa niiden kohdalla todennäköisesti harhaan.
Tää pitäisi kirjoittaa (edellisen testin muutosten kera) esim.:
export const ssnSchemaWithAgeChecks = z
.string()
.trim()
.refine(isSsnValid, { error: 'SSN must be valid', abort: true })
.refine(ssn => isSsnValid(ssn, true), { error: 'SSN fails age checks' })
.toUpperCase()
jotta sieltä ei aina tule tuo ikään viittaava validointivirhe. Tuo ehdotus tuottaisi ns. progressiivisesti tarkemman virheen ja olisi todennäköisesti helpompi debugata eri virheiden kohdalla.
|
|
||
| const centurySeparator21st = ['A', 'B', 'C', 'D', 'E', 'F'] | ||
| const specialFakeSsnToBeAllowed = '010101-0101' | ||
| const centurySeparator20th = ['-', 'Y', 'X', 'W', 'V', 'U'] |
There was a problem hiding this comment.
Vois laajentaa testiä jotta nää tulis kaikki varmistettua ikätarkistuksen osalta:
test('rejects when person is over hundred years old, and age sanity check is on', () => {
assert.equal(validation.isSsnValid('231194+246w', true), false)
for (const separator of ['-', 'Y', 'X', 'W', 'V', 'U']) {
assert.equal(validation.isSsnValid(`010120${separator}123K`, true), false, separator)
assert.equal(validation.isSsnValid(`010180${separator}1232`, true), true, separator)
}
})2 participants
There's currently quite a few places where SSN inputs have spaces in them. There's no reason for not properly normalizing them in one place.
References: