Skip to content
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

fix: /report endpoint date validation #5575

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

jaskfla
Copy link
Contributor

@jaskfla jaskfla commented Apr 9, 2024

Issue: https://beyondessential.slack.com/archives/C01MAA3NKM1/p1712622483211609

Changes

Just removed ^ and $ from the regex pattern I was using to validate date parameters. Specifically, it’s the $ that breaks it, but I cannot for the life of me see why 😕

Strictly speaking, this now allows anything that has a substring in the YYYY-MM-DD format to pass validation, but these inputs aren’t ever hand-written anyway (as far as I know). Ultimately it would result in another error saying that the date is null (since the server would then fail to parse it)—just a slightly less helpful error message.

@jaskfla jaskfla changed the base branch from dev to baseline-2024-15 April 9, 2024 02:31
@jaskfla jaskfla changed the base branch from baseline-2024-15 to release-2024-15 April 9, 2024 02:33
@@ -6,7 +6,7 @@
import moment from 'moment';
import momentTimezone from 'moment-timezone';

export const ISO_DATE_PATTERN = /^\d{4}-\d{2}-\d{2}$/;
export const ISO_DATE_PATTERN = /\d{4}-\d{2}-\d{2}/;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also works:

Suggested change
export const ISO_DATE_PATTERN = /\d{4}-\d{2}-\d{2}/;
export const ISO_DATE_PATTERN = /^\d{4}-\d{2}-\d{2}/;

Feel free to commit the suggestion directly if you prefer it

@jaskfla jaskfla merged commit 54d4c16 into release-2024-15 Apr 10, 2024
48 checks passed
@jaskfla jaskfla deleted the fix-report-date-validation branch April 10, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants