feat(utils): fixing date of birth error + format validation #3827
Conversation
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Ref: ENG-241 Ref: #1040 Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
… 241-bad-request-on-invalid-dob Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
| export function normalizeDobSafe(date: string): string | undefined { | ||
| const trimmedDate = date.trim(); | ||
| if (trimmedDate.length < 1) return undefined; | ||
| if (!validateDateOfBirth(trimmedDate)) throw new Error("Invalid date of birth."); | ||
| if (!validateDateOfBirthSafe(trimmedDate)) return undefined; | ||
| return buildDayjs(trimmedDate).format(ISO_DATE); | ||
| } |
There was a problem hiding this comment.
lol we were throwing an error inside of something called normalizeDobSafe?? 😬
There was a problem hiding this comment.
Worse, we only call it on a function that throws an error if the return is undefined... 😆
And I think this function is not just normalization, it's parsing. Normalization is just trying to make a value more "normalized", it should not return undefined if the value is not undefined, for example. This is parsing.
| @@ -16,6 +18,8 @@ export function normalizeGenderSafe(gender: string): "F" | "M" | "O" | "U" | und | |||
|
|
|||
| export function normalizeGender(gender: string): GenderAtBirth { | |||
| const genderOrUndefined = normalizeGenderSafe(gender); | |||
| if (!genderOrUndefined) throw new Error("Invalid gender"); | |||
| if (!genderOrUndefined) { | |||
| throw new BadRequestError("Invalid gender", undefined, { gender }); | |||
There was a problem hiding this comment.
nit maybe this should be InvalidArgumentError since its a general purpose function used to validate data?
There was a problem hiding this comment.
We don't have a InvalidArgumentError, are you suggesting we create one?
I think it makes sense, it's semantically better to throw InvalidArgumentError from the command inward than use BadRequestError (that hints to "request" as in "http request").
To minimize the impact on existing code we could make InvalidArgumentError extends BadRequestError.
Def not required for this PR, but I'd ticket it if we decide not to do it now.
| return validateIsPastOrPresent(patient.dob) && validateDateOfBirth(patient.dob); | ||
| validateDateOfBirth(patient.dob); | ||
| return true; |
There was a problem hiding this comment.
weird to me that this function can both return false and throw errors. Shouldn't we return false instead of throwing?
Ref: ENG-241
Issues:
Description
Testing
Release Plan