fix(#11423): centralize locale-independent date strings by pinankshah8-Aa · Pull Request #11451 · medic/cht-core

pinankshah8-Aa · 2026-09-22T20:02:28Z

Description

Implements the first PR in the agreed two-PR split for #11423.

Adds locale-independent local ISO-date and interval-tag helpers to @medic/calendar-interval.
Routes target-tag writers/readers and DHIS target lookup through the same interval-tag logic.
Routes task-state dates and DHIS completeDate through locale-independent ASCII formatting.
Preserves Gregorian fallback when Bikram Sambat conversion is unavailable or out of range.
Adds regression coverage for non-ASCII Moment post-formatting, Gregorian/Bikram Sambat parity, task-date comparisons, and fallback behavior.

This PR intentionally excludes the display-related sites (message-utils, format-date.service.ts, and date-filter.component.ts) and the #11439 site. Those remain deferred to PR 2 of the maintainer-requested split.

Closes #11423.

The display-related PR 2 remains planned as the separately reviewed follow-up requested by the maintainer.

Testing

npm test --prefix shared-libs/calendar-interval — 42 passing
npm test --prefix shared-libs/rules-engine — 225 passing
focused api/tests/mocha/services/export/dhis.spec.js — 15 passing
npm run unit --prefix webapp -- --watch=false --code-coverage=false --include=../tests/karma/ts/services/rules-engine.service.spec.ts — 60 passing
npm run lint — passed
git diff --check origin/master...HEAD — passed

Code review checklist

Readable: the shared helpers replace duplicated locale-sensitive formatting paths.
Tested: focused unit/regression tests and repository lint pass as listed above.
Backwards compatible: existing display localization is unchanged; Bikram Sambat failures retain Gregorian fallback.
AI disclosure: I used ChatGPT/Codex to inspect affected code paths, draft portions of the implementation and regression tests, and help review/debug the validation setup. I personally reviewed every change, corrected the control flow during self-review, verified the final diff and scope, and ran the test/lint commands listed above. I understand the implementation and remain responsible for it.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

github-actions · 2026-09-22T20:05:41Z

All checks passed — thank you @pinankshah8-Aa! This PR is ready for review.

pinankshah8-Aa · 2026-09-23T08:28:59Z

Hi, the ci-integration-sentinel-k3d job failed in the existing circular client-side muting test at tests/integration/sentinel/transitions/muting.spec.js:1989. The other 173 Sentinel tests passed, and PR #11451 does not modify Sentinel or muting-related code.

I don’t have permission to rerun the upstream workflow. Could a maintainer please rerun the failed job when convenient?

Failed job: https://github.com/medic/cht-core/actions/runs/35777669176/job/107042181009?pr=11451

pinankshah8-Aa · 2026-09-30T04:39:52Z

Hi, following up on the September 23 rerun request. The PR remains open and mergeable, and the Conventional Commits, CodeQL, and zizmor workflows passed. The remaining Build and test failure is still the existing ci-integration-sentinel-k3d muting test, with the other 173 Sentinel tests passing; this PR does not modify Sentinel or muting code.

Could a maintainer please rerun the failed job or let me know if any additional action is needed from me?

binokaryg

Thanks @pinankshah8-Aa, this follows the agreed split closely and the parity tests between the writer in provider-wireup and the readers in the webapp and the DHIS export are exactly what I hoped for. Two things need a change before merge, and a few smaller notes added in-line.

binokaryg · 2026-10-02T05:47:47Z

+  } catch {
+    return gregorianTag;


Before this PR only provider-wireup.getTargetDocTag had a fallback, and it logged a console.warn when toBik threw. The webapp's getTargetIntervalTag and the DHIS targetDocsInMonth had no fallback and would throw.

The new helper catches everything and returns the Gregorian tag with no log line, so all three call sites lose the signal. The worst case is the DHIS export: if toBik fails for the requested month, the export now returns a well-formed payload with every value at 0 instead of an error.

For the webapp, the writer and the reader could end up on different calendars for one month and nothing would tell us.

Could you keep the warning inside the helper so every caller gets it? I would also consider letting the DHIS export surface the error rather than fall back, since a wrong export is worse than a failed one, but the log line is the minimum.

Fixed in bca53db. The shared helper now preserves the Gregorian fallback and emits a warning with the conversion error, so all callers retain the diagnostic signal. I also added an assertion that the warning is emitted.

binokaryg · 2026-10-02T05:49:32Z

    dataSet,
-    completeDate: moment().format('YYYY-MM-DD'),
+    completeDate: CalendarInterval.toLocalIsoDate(moment()),
    period: moment(from).format('YYYYMM'),


completeDate moved to toLocalIsoDate, but the line below it is still moment(from).format('YYYYMM').

With the ne locale active that produces २०२५०३, which DHIS2 will reject.

The new regression test does not catch this because its postformat only rewrites strings that contain a -, and YYYYMM has none.

Please route period through .locale('en') as well, or add a YYYYMM variant to the helper, and drop the includes('-') guard from the test so it covers the whole payload. (api/tests/mocha/services/export/dhis.spec.js, line 113)

Fixed in bca53db. The period now formats with the English locale, and the regression locale now postformats every numeric string, including YYYYMM, so the expected payload verifies both completeDate and period remain ASCII.

binokaryg · 2026-10-02T05:51:32Z

-    } else {
-      interval = moment(timestamp).format('YYYY-MM');
-    }
+    const interval = CalendarInterval.getIntervalTag({ end: timestamp }, useBikramSambatMonths);


Not blocking: getIntervalTag takes an interval but only reads interval.end, which is why dhis.js has to build a fake { end: timestamp }. Taking the end timestamp directly would make the signature honest and remove the { end: number } type in the webapp service.

Also applies to: shared-libs/calendar-interval/src/index.js, line 146

binokaryg · 2026-10-02T05:52:30Z

+
+/** Formats the end month of an interval as an ASCII Gregorian or Bikram Sambat tag. */
+const getIntervalTag = (interval, useBikramSambatMonths) => {
+  const gregorianTag = moment(interval.end).locale('en').format('YYYY-MM');


Not blocking: An invalid interval.end makes the Gregorian fallback the literal string Invalid date, which then becomes part of the allDocs range key. Returning undefined or throwing would be safer than querying target~Invalid date~.

binokaryg · 2026-10-02T05:53:29Z

+      chai.expect(service.getIntervalTag(interval, true)).to.equal('2081-11');
+    });
+
+    it('falls back to the Gregorian tag for an out-of-range Bikram Sambat date', () => {


Non-blocking: The out-of-range fallback test uses 2150-05-15 as a date the bikram-sambat table does not cover. A future upgrade of that package could extend the table and break the test with no change here. The next test already stubs toBik to throw, so this one could do the same, or use a date before the Bikram Sambat epoch.

Signed-off-by: Pinank Shah <226854365+pinankshah8-Aa@users.noreply.github.com>

pinankshah8-Aa · 2026-10-08T08:37:30Z

Hi @binokaryg, I’ve addressed the remaining review notes in 7a33722, including the direct timestamp signature, invalid-timestamp handling, and deterministic fallback coverage. All current workflows now pass, including the full Build and test workflow. When convenient, could you please re-review the updated PR?

pinankshah8-Aa · 2026-10-16T04:12:17Z

Hi @binokaryg, following up on the requested changes addressed in 7a337223. All current checks pass, including the full build/test workflow and Sonar. When you have time, could you please re-review? Thank you.

binokaryg

Thanks @pinankshah8-Aa, all five points from the first round are addressed and I am happy to approve.

Two cosmetic nits are in-line, neither should hold up the merge. One more note for the display follow-up PR rather than this one:

getReportingMonth in webapp/src/ts/services/rules-engine.service.ts (lines 638-639) parses whatever tag comes back as a Bikram Sambat month. Now that the helper falls back instead of throwing, a Gregorian tag's month would be read as a Bikram Sambat month index and show the wrong month name, where before the throw landed on the subtitle at line 644. This only happens when toBik fails, which for the current table means dates before about 1943 or after 2034, so nothing today. Since it is display behaviour, I would rather handle it in the display follow-up PR, for example by having the helper also say which calendar it used, or by checking the tag's year there.

binokaryg · 2026-10-16T05:26:20Z

        sinon.stub(rulesStateStore, 'getUseBikramSambatMonths').returns(true);
        const bikramSambat = require('bikram-sambat');
        const toBik = sinon.stub(bikramSambat, 'toBik').throws(new Error('invalid date'));
-        const warning = sinon.stub(console, 'warn');


nit: The console.warn stub and its assertion were removed from this test, so the fallback warning and its stack now print in the middle of the rules-engine run. The same happens for the new out-of-range case at shared-libs/calendar-interval/test/index.js, lines 851-852. The test right after it already stubs console.warn and asserts on it, so these two could do the same.

binokaryg · 2026-10-16T05:27:44Z

+/** Formats a timestamp as an ASCII Gregorian or Bikram Sambat month tag. */
+const getIntervalTag = (endTimestamp, useBikramSambatMonths) => {
+  const endDate = moment(endTimestamp);
+  if (!Number.isFinite(endTimestamp) || !endDate.isValid()) {


nit: Once Number.isFinite passes, endDate.isValid() can no longer be false, and line 159 parses the timestamp a second time although endDate already holds it. toLocalIsoDate(endDate) works because the helper accepts a moment. Purely cosmetic.

fix(medic#11423): centralize locale-independent date strings

1a415ae

github-actions Bot added the Waiting for contributor label Sep 22, 2026

github-actions Bot added Ready for review and removed Waiting for contributor labels Sep 22, 2026

Merge branch 'master' into 11423-local-date-helpers

3ff1f09

binokaryg self-requested a review October 2, 2026 05:42

binokaryg requested changes Oct 2, 2026

View reviewed changes

pinankshah8-Aa and others added 3 commits October 2, 2026 12:40

fix(medic#11423): preserve date conversion diagnostics

bca53db

Signed-off-by: Pinank Shah <226854365+pinankshah8-Aa@users.noreply.github.com>

Merge branch 'master' into 11423-local-date-helpers

69c4d5e

fix(medic#11423): address date helper review feedback

7a33722

Signed-off-by: Pinank Shah <226854365+pinankshah8-Aa@users.noreply.github.com>

Merge branch 'master' into 11423-local-date-helpers

4b7d64f

binokaryg self-requested a review October 16, 2026 05:21

binokaryg approved these changes Oct 16, 2026

View reviewed changes

Conversation

pinankshah8-Aa commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Code review checklist

License

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

pinankshah8-Aa commented Sep 23, 2026

Uh oh!

pinankshah8-Aa commented Sep 30, 2026

Uh oh!

binokaryg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinankshah8-Aa commented Oct 8, 2026

Uh oh!

pinankshah8-Aa commented Oct 16, 2026

Uh oh!

binokaryg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pinankshah8-Aa commented Sep 22, 2026 •

edited

Loading