fix: support non-gregory calendars [DHIS2-17617] #397

tomzemp · 2024-09-25T14:03:43Z

Background

This PR adds periods for non-Gregory calendars and updates the date logic throughout the app such that non-Gregorian dates "work" throughout the app to the extent that we can avoid logic problems when working with non-ISO dates.

This addresses the following tickets:

DHIS2-17617: adds non-ISO periods to selector bar
DHIS2-17619: reviews date logic throughout the app to function (as much as possible) with non-ISO dates

Additionally the following will be fixed by this PR:

DHIS2-18057: using correct display name of selected period (based on user's language)
DHIS2-18031: preventing crashes from languages like pt_BR where java code is invalid format (due to amendment in multi-calendar-dates)

Some common themes

Period generation

primarily this requires passing calendar to period generation
this required some fixes in multi-calendar-dates library (hence the dependency bump)

Avoiding use of Date()

most of our uses of Date initialization was a step in order to then be able to compare dates (e.g. is the period's start date before/after the organisation unit date's opening date).
I have tried to refactor to keep all dates in their original string (server timezone) form until we actually need to make the comparison.
I have moved the comparison logic to common functions, this allows us to centralize the logic and make a common implementation for non-Gregorian calendars. For Gregorian/ISO calendars, we can continue using dates, and for non-Gregorian dates, we can probably use string comparisons (needs testing)
I don't think we need a to do here because the only advantage of not using string comparison is some additional safeguarding against invalid dates, e.g. 2017-13-20 for Ethiopian calendar.

date manipulation

this is a bit of a subset of the above, but we had a lot of random date manipulation throughout the app (e.g. adding a ms/day to get some actual limit). I first want through and tried to rewrite everything to avoid these approaches (e.g. instead of adding a day to end date to get the next period's start date, I generated the next period and got the actual start date).
unfortunately with expiryDays, we arguably still do no need to add days to a given date, and we cannot do that for non-Gregorian dates. However, we only currently use this informational purposes at present, so it's not really dire if this functionality doesn't work with non-ISO calendars.
To-do: https://dhis2.atlassian.net/browse/LIBS-692

Handling time zones

the major use of time zone correction logic in the data entry app was to handle the problem where we got the now date (from new Date()) and needed to compare it with a server date. This is now not necessary as we can make use of multi-calendar-dates getNowInCalendar and then get a date/time string that reflects the wall clock time for the server. This allows us to refactor to remove a lot of code we had in the app for this.
there are a few cases where we still would like to "correct" a server date to client date/time stamp (audits and last updated information), but these cases are limited and I thought it would be easier for us to include the time zone in these cases.

for example:

To-do : https://dhis2.atlassian.net/browse/LIBS-693

relative time

we use moment's fromNow (replaced with from) for relative time values, which will not work with non-ISO dates, but again, this seems to be functionality that we can drop when calendar is not ISO without major effect (e.g. we can display last updated 2016-13-03 instead of last updated 5 minutes ago with non ISO calendars).

for example:
gregory:

ethiopian:

To-do: https://dhis2.atlassian.net/browse/LIBS-691

Testing

I have primarily tested with automated testing by trying to add/expand coverage for any code that is affected by these changes.
I have done some light manual testing (e.g. checked that the period generation works okay and that data entry seems to work). It's a bit hard to test some of these things manually as the

dhis2-bot · 2024-09-27T16:46:47Z

🚀 Deployed on https://pr-397--dhis2-data-entry.netlify.app

Mohammer5

Code LGTM! I love the extensive testing 😍
No change requests, just a couple of comments.
Will test in browser later

Once suggestion I have is to introduce a hook (e.g. useCalendar) so we don't have to do const { systemInfo = {} } = useConfig(); const { calendar = 'gregory' } = systemInfo all the time

This is just a thought we could explore in the future: What if we introduced a Zustand store that persists the calendar and timezone during runtime which gets populated before we display the app.

Then we abstract and move all date-related logic into src/shared/date, let these functions get the calendar value, so the app doesn't have to deal with that anymore.
The last piece would be to have date components that render the date appropriately.

Then we'd have isolated all the date-complexity into the date modules (src/shared/date & src/components/date/) and the rest off the application is unaware that different date format exist.

Another thought: Maybe we can move all the (non-period related) date logic into the multi-calendar-dates library as well. Especially if we do what I mentioned above, that code could result in something that could be useful to a lot more apps

Mohammer5 · 2024-10-08T02:28:36Z

src/shared/date/starting-years.js

+// equivalents of 1970, except for Nepali which is 3 years later
+// we don't have calendar details for nepali calendar from 1970 and back
+
+export const startingYears = {


Is this an exhaustive list?

Or does it cover all starting years relevant to dhis2?

Should this be moved to the multi-calendar-dates library or stay here?

Mohammer5 · 2024-10-08T02:34:53Z

src/shared/date/date-utils.js

+}
+
+// returns string in either 'YYYY-MM-DD' or 'YYYY-MM-DDTHH:MM.SSS' format (depending on input)
+// if non-gregory calendar, returns null


Should we add a Jira ticket for this? Maybe there's one already..

Mohammer5 · 2024-10-08T02:42:01Z

src/data-workspace/data-details-sidebar/basic-information.js

+                        <Tooltip content={lastUpdatedString}>
+                            {/* timeAgo will be null if non-Gregory calendar. TO DO: update */}
+                            {i18n.t(
+                                'Last updated {{- timeAgo}} by {{- name}}',


Just out of curiousity, what is the {{- ...}} syntax? I only know {{...}}

Mohammer5 · 2024-10-08T02:51:23Z

src/context-selection/period-selector-bar-item/use-periods.js

-    }, [periodType, currentDay, year, dateLimit, locale, openFuturePeriods])
+    }, [
+        periodType,
+        currentDayString,


I think there's an eslint rule we can disable here to prevent the warning

Mohammer5 · 2024-10-08T02:51:48Z

src/context-selection/period-selector-bar-item/year-navigator.js

+    onYearChange,
+    calendar,
+}) {
+    // console.log('maxYear', maxYear)


Suggested change

// console.log('maxYear', maxYear)

Mohammer5 · 2024-10-08T02:54:52Z

src/context-selection/period-selector-bar-item/use-deselect-on-period-type-change.js

Deleted because unused I suppose?

tomzemp · 2024-10-15T10:59:40Z

UPDATE:
I spoke with Jan B and he clarified that datetime stamps on the backend are (at least going forward) supposed to be stored and returned in ISO and that the front-end would have responsibility for displaying them in the system's calendar.

Since we have period start/end dates in the non-gregorian calendars, we need to, for example, compare dates between calendars (e.g. check that the org unit closing date (ISO) is after the period end date (ethiopian)).

This also means that some of the assumptions I had about relative time and displaying date/time stamps were not appropriate. I've gone through and refactored and made use of the conversion functions from the multi-calendar-dates conversion functions, so with the most recent commit, there should no longer be anything that is "to do" for non-gregory calendars. I've also created a component DateText to allow us to pass a date stamp and then isolate all the logic for conversion/formatting (I've also added the useTimeZoneConversion hook in that component to correct time differences).

Mohammer5 · 2024-10-15T14:16:00Z

src/bottom-bar/form-expiry-info.js

+        calendar: 'gregory',
+        timezone,
+    })
+    const dateTime = `${lockDate} (${timezone})`


If I understand this code correctly, then there's a good chance that the lockDate will be a server-date, so the dateTime that's being constructed (and displayed in the tooltip) here will always be a Gregorian date, regardless of the user's calendar.

Thanks! Fixed to use the component and hence convert to system calendar.

Mohammer5 · 2024-10-15T14:35:13Z

src/shared/date/date-text.js

+    const { calendar = 'gregory', dateFormat } = systemInfo
+    const { fromServerDate } = useTimeZoneConversion()
+
+    // NOTE: the passed date is assumed to be in ISO


Should we add validation for this and console.error in case it's not?

Yeah. I don't know if there's much we can do if we pass the wrong date because an Ethiopian date like 2016-03-14 is also a valid Gregorian date. But, I've added a step to check that the date is parsable as ISO, so a date like 2016-13-04 will fail and then I've set it to return Invalid date ({{date you passed}})

## [100.8.3](v100.8.2...v100.8.3) (2024-10-16) ### Bug Fixes * support non-gregory calendars [DHIS2-17617] ([#397](#397)) ([7c3e0c8](7c3e0c8))

dhis2-bot · 2024-10-16T10:25:36Z

🎉 This PR is included in version 100.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

tomzemp added 10 commits September 13, 2024 15:14

fix: first calendar clean up

9559102

fix: work on date logic cleanup

3873eba

fix: continued updates

632be3f

fix: more updates

a0d8437

fix: clean up

b796243

fix: selected period display

93bda61

fix: remove unnecessary comment

83e47cf

fix: update multi-calendar-dates dependency

43492f7

fix: use period name in invalid period warning

968311e

fix: tests, cleanup

7581c0e

dhis2-bot temporarily deployed to netlify September 27, 2024 16:46 Inactive

Merge branch 'master' into DHIS2-17617/multicalendar-periods

bc45720

dhis2-bot temporarily deployed to netlify September 27, 2024 16:54 Inactive

tomzemp added 3 commits September 30, 2024 14:48

fix: start date limit test

e5ef4d9

fix: useDateLimit tests

5847d55

fix: add more tests

37c5b90

dhis2-bot temporarily deployed to netlify October 2, 2024 09:58 Inactive

Merge branch 'master' into DHIS2-17617/multicalendar-periods

e0e220b

Mohammer5 approved these changes Oct 8, 2024

View reviewed changes

fix: handle ISO time stamps from backend

9722a21

dhis2-bot temporarily deployed to netlify October 14, 2024 17:47 Inactive

tomzemp mentioned this pull request Oct 14, 2024

fix: make datetime input support multicalendar [DHIS2-17618] #404

Merged

Mohammer5 approved these changes Oct 15, 2024

View reviewed changes

tomzemp added 3 commits October 16, 2024 10:11

fix: pr feedback

b944648

fix: use master yarn.lock

aa54378

Merge branch 'master' into DHIS2-17617/multicalendar-periods

9135688

dhis2-bot temporarily deployed to netlify October 16, 2024 08:24 Inactive

fix: pass correct year for yearly periods

a8348f9

dhis2-bot temporarily deployed to netlify October 16, 2024 09:59 Inactive

Mohammer5 approved these changes Oct 16, 2024

View reviewed changes

tomzemp merged commit 7c3e0c8 into master Oct 16, 2024
9 of 10 checks passed

dhis2-bot added a commit that referenced this pull request Oct 16, 2024

chore(release): cut 100.8.3 [skip release]

c161d8a

## [100.8.3](v100.8.2...v100.8.3) (2024-10-16) ### Bug Fixes * support non-gregory calendars [DHIS2-17617] ([#397](#397)) ([7c3e0c8](7c3e0c8))

dhis2-bot added the released label Oct 16, 2024

tomzemp mentioned this pull request Oct 22, 2024

fix: handle java language format [DHIS2-18031] #395

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: support non-gregory calendars [DHIS2-17617] #397

fix: support non-gregory calendars [DHIS2-17617] #397

tomzemp commented Sep 25, 2024 •

edited by jira bot

Loading

dhis2-bot commented Sep 27, 2024 •

edited

Loading

Mohammer5 left a comment

Mohammer5 Oct 8, 2024

Mohammer5 Oct 8, 2024

Mohammer5 Oct 8, 2024

Mohammer5 Oct 8, 2024

Mohammer5 Oct 8, 2024

Mohammer5 Oct 8, 2024

tomzemp commented Oct 15, 2024

Mohammer5 Oct 15, 2024

tomzemp Oct 16, 2024

Mohammer5 Oct 15, 2024

tomzemp Oct 16, 2024

dhis2-bot commented Oct 16, 2024

fix: support non-gregory calendars [DHIS2-17617] #397

fix: support non-gregory calendars [DHIS2-17617] #397

Conversation

tomzemp commented Sep 25, 2024 • edited by jira bot Loading

Background

Some common themes

Period generation

Avoiding use of Date()

date manipulation

Handling time zones

relative time

Testing

dhis2-bot commented Sep 27, 2024 • edited Loading

Mohammer5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomzemp commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhis2-bot commented Oct 16, 2024

tomzemp commented Sep 25, 2024 •

edited by jira bot

Loading

dhis2-bot commented Sep 27, 2024 •

edited

Loading