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: Incomplete date-times are encoded as iso-formatted date-time #171

Conversation

lucaas
Copy link
Contributor

@lucaas lucaas commented Nov 8, 2023

Resolves FT-7e37292d-464c-44f4-9c62-a40705f886b4

  • I have added automatic tests where applicable
  • The PR title is suitable as a release note
  • The PR contains a description of what has been changed
  • The description contains manual test instructions

Changes

Update regex to not match incomplete date-times.

Here is how copilot describes the regex:

The active selection is a regular expression in TypeScript that is used to validate date-time strings in the ISO 8601 format. > This format is widely used in web applications and APIs for transmitting date and time data.

Let's break down the regular expression:

  • ^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d matches a date-time string with a year, month, day, hour, minute, and second. The T is a literal character that separates the date and time components.
  • (.\d+)? matches an optional fractional second, which if present, should be a period followed by one or more digits.
  • (([+-]\d\d:\d\d)|Z)?$ matches an optional timezone offset, which can be 'Z' (for UTC) or a positive or negative offset in the format HH:MM.
  • The i at the end of the regular expression makes the match case-insensitive.

For more details on the format: https://www.w3.org/TR/NOTE-datetime

Test

Validate code changes.

@lucaas lucaas requested a review from gismya November 8, 2023 10:14
@lucaas lucaas requested a review from a team as a code owner November 8, 2023 10:14
@gismya
Copy link
Contributor

gismya commented Nov 9, 2023

It's a sensible change. My initial reaction was that this should probably wait for the V2 release because of potential unintended effects, but after reviewing the relevant code, previous implementations, and how they are reflected in the database later this should probably be fine to consider simply a bug fix.

But if we only want to pick up previous date objects that have gone through .toISOString we should lock down the regex even further.

Copy link
Contributor

@gismya gismya left a comment

Choose a reason for hiding this comment

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

After investigating it, this seems like a good level of targeting isIsoDate.

@gismya gismya merged commit 0e0f721 into main Nov 21, 2023
2 checks passed
@gismya gismya deleted the backlog/string-with-four-digits-in-converted-to-iso-date-in-js-api branch November 21, 2023 09:04
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