apache / apache/fineract-backoffice-ui

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

Open
#257 5 comments 0 reactions 0 assignees View on GitHub
angular good first issue javascript typescript
Dominant language
TypeScript
Stars
15
Forks
60
Avg merge
10h 15m
Merged PRs (30d)
108

Description

## 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:

```ts
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:

```ts
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:

```ts
// 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.

Contributor guide

Open the contributing guide

Research direction

Read src/app/core/utils/date-formatter.ts and use the provided grep command to locate the 18 toIsoDate(new Date(...)) sites across clients, loans, products, organization and transfers. Preserve existing guards, leave toLocaleDateString() sites unchanged, and run the affected specs plus npm run build and npm test. Done means all 18 in-scope sites use formatArrayDate without changing locale-formatted displays or malformed-value handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, typescript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.