Skip to content

18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that #257

Description

@Aman-Mittal

Business value

Fineract returns dates as [year, month, day] with a 1-based month. Eighteen places convert one to a YYYY-MM-DD string by constructing a Date and formatting it back:

toIsoDate(new Date(actDateArray[0], actDateArray[1] - 1, actDateArray[2]))

src/app/core/utils/date-formatter.ts already exports a function that does this directly:

formatArrayDate(actDateArray)

The two produce identical output for a valid array — worth confirming, since the round trip through Date is not obviously lossless, and it is:

[2026, 1, 5]   → 2026-01-05   both
[2026, 12, 31] → 2026-12-31   both
[1999, 2, 28]  → 1999-02-28   both

The value in fixing it is the - 1. Eighteen hand-written copies of a month-offset conversion is eighteen chances to write [1] instead of [1] - 1, and an off-by-one month in a banking application is not a cosmetic bug — it moves an activation date, a disbursement date, or a closure date by a month, and the result is a plausible date rather than an obviously broken one, so nothing downstream flags it. The helper is written once and has a test.

It is also a tidier read: formatArrayDate(dates) says what it does; toIsoDate(new Date(d[0], d[1] - 1, d[2])) makes the reader verify the arithmetic.

Reproducing it

grep -rn 'toIsoDate(new Date(.*\[0\]' src/app --include=*.ts | grep -v spec   # 18
grep -rl 'formatArrayDate' src/app --include=*.ts | grep -v spec | wc -l      # 15 files already use it

Describing the change

Replace the round trip with the helper:

// before
const actDateArray = clientData.activationDate as unknown as number[];
if (actDateArray) {
  this.activationDate.set(toIsoDate(new Date(actDateArray[0], actDateArray[1] - 1, actDateArray[2])));
}

// after
this.activationDate.set(formatArrayDate(clientData.activationDate));

Two things to be careful about — please read these before starting.

  1. Only the toIsoDate(new Date(...)) sites. There are 13 further sites that do
    new Date(d[0], d[1] - 1, d[2]).toLocaleDateString(). Those are not the same thing:
    toLocaleDateString() formats for the browser's locale (8/7/2026), while formatArrayDate
    returns ISO (2026-08-07). Converting them would silently change what users see. Leave them
    alone; whether display dates should be locale-formatted is a separate question worth its own
    issue.

  2. formatArrayDate returns '-' for a missing or malformed value, not ''. That is right
    for display, and wrong for a value bound to ion-datetime, which would try to parse '-'.
    Most of the 18 sites are already inside an if (someDate) guard, so behaviour is unchanged —
    but check each one, and keep the guard where it exists.

One pull request per feature directory. The sites are spread across clients, loans, products, organization and transfers.

Scope

In scope: the 18 toIsoDate(new Date(arr[0], arr[1] - 1, arr[2])) sites.

Out of scope: the 13 toLocaleDateString() display sites, and any change to formatArrayDate itself.

Getting started

  • Helper: src/app/core/utils/date-formatter.ts (read the doc comments — they explain why toIsoDate avoids toISOString()).
  • npm run build and npm test must pass. Several of these components have specs that assert on the formatted value, which is the check that the substitution was faithful.
  • If a site has no spec covering it, adding one is welcome but not required.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions