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

Updated cloudcare's date format to be more universal #35872

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjriley
Copy link
Contributor

@mjriley mjriley commented Mar 3, 2025

Product Description

A bug was preventing Norwegian-locale users from entering dates into CloudCare. This PR updates the date format to better accommodate non-US dates by changing the date format to be ISO-standard.

The new screen looks like:
image
where the expected date is represented as 'YYYY-MM-DD'. The previous format was 'M/D/YYYY'. While we could have fixed the bug by using the previous 'M/D/YYYY' format, we felt it would be confusing to non-US users not being able to determine which number was month and which was day. By following the ISO format, there shouldn't be that ambiguity.

Technical Summary

Associated ticket: https://dimagi.atlassian.net/browse/SAAS-15933

The basic issue is that, while the 'tokens' within our date template get localized, the format itself does not. TempusDominus treats 'M' slightly different from 'MM'. 'M' is put through formatter, which uses the standard Intl library to convert the month into a localized number. It is this library that is responsible for appending the extra period to the month, so there's not much we can do about that while using 'M'. However, if we use 'MM', the issue goes away, as the output of that does not include the extra period. While we could fix the issue by changing our current format of M/D/YYYY to MM/DD/YYYY, it felt cleaner to switch to the ISO standard.

Random things to point out:

  • This won't align with how the mobile app handles dates, but it already behaved differently. Note the mobile app's display:
    image. While the date will be ordered differently, note that the mobile app does represents days with a leading zero, while the previous HQ version did not
  • This does align with how we represent dates on the backend. This is what a date looks like after being submitted:
    image. Again, notice the leading zeroes.
  • The cleanest fix is likely to support real date localization, but that is probably better handled in a separate ticket.

Feature Flag

No feature flag

Safety Assurance

Verified locally that this fixes the Norwegian date issue while still supporting other locales.

Safety story

As mentioned above, this was locally tested. I verified that the form data was still represented correctly on the backend. I also checked the mobile app to make sure we weren't introducing a non-standard change.

I would like to verify that this new date format doesn't break any of the few other places in the code that use it.

Automated test coverage

No automated tests

QA Plan

QA shouldn't be necessary provided we verify the other locations that leverage this date format.

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@mjriley mjriley added the product/all-users-all-environments Change impacts all users on all environments label Mar 3, 2025
@mjriley mjriley requested a review from orangejenny March 3, 2025 21:03
@dimagimon dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Mar 3, 2025
Copy link
Contributor

@orangejenny orangejenny left a comment

Choose a reason for hiding this comment

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

This seems eminently reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/all-users-all-environments Change impacts all users on all environments Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants