You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Aman-Mittal (via GitHub)" <gi...@apache.org> on 2026/08/07 17:04:29 UTC
[I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Aman-Mittal opened a new issue, #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on issue #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257#issuecomment-5318066295
Reopening — this was closed as completed on 15 Aug, but five sites matching the pattern in scope are still on `main`:
```
src/app/features/groups/group-action-dialog.component.ts:231
src/app/features/products/fixed-deposits/fixed-deposit-form.component.ts:416
src/app/features/products/recurring-deposits/recurring-deposit-form.component.ts:470
src/app/features/products/savings-account-transaction-form.component.ts:231
src/app/features/working-capital/loan-products/wc-loan-product-form.component.ts:681
```
Reproduce with the narrower grep — the one in the description also matches `toIsoDate(new Date())` with no argument, which is today's date and not in scope:
```bash
grep -rnE 'toIsoDate\(new Date\([A-Za-z0-9_.]+\[0\]' src/app --include=*.ts | grep -v spec
```
Two of the per-directory PRs the issue asked for did land — @YinkaMetrics's organization pass (301ba8e5, #333) and @Berserk-hub150's products one (d06d2e33, #345) — which is presumably why this looked finished. `products` has three sites left, and `groups` and `working-capital` were not in the original list of affected directories at all.
Still a good first issue, and still one PR per feature directory: `groups`, `products`, `working-capital`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal closed issue #257: 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that
URL: https://github.com/apache/fineract-backoffice-ui/issues/257
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "Berserk-hub150 (via GitHub)" <gi...@apache.org>.
Berserk-hub150 commented on issue #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257#issuecomment-5302200190
Hi! I'd like to work on the **products** part of this issue.
I'll replace the manual array-to-Date round trip in savings-account-form.component.ts with the existing ormatArrayDate helper, keeping the scope limited to the products/ feature directory as requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal closed issue #257: 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that
URL: https://github.com/apache/fineract-backoffice-ui/issues/257
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "WilliamK112 (via GitHub)" <gi...@apache.org>.
WilliamK112 commented on issue #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257#issuecomment-5643627452
I’ll take the remaining **working-capital** slice only. I’ll replace the array branch in with the existing helper, preserve the non-array fallback and missing-value guard, and add or update focused coverage before running the relevant lint, format, unit, and build checks.\n\nAI assistance disclosure: I’m using OpenAI Codex to help inspect, implement, and validate the change; I will review the final diff and remain responsible for the contribution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "Yurii201811 (via GitHub)" <gi...@apache.org>.
Yurii201811 commented on issue #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257#issuecomment-5470951012
I'd like to take the remaining staff everyone localaccounts _appserverusr admin _appserveradm _lpadmin com.apple.sharepoint.group.1 _appstore _lpoperator _developer _analyticsusers com.apple.access_ftp com.apple.access_screensharing com.apple.access_ssh com.apple.access_remote_ae slice. I'll replace the manual array-to-Date-to-ISO conversion in with , keep the existing missing-value guard intact, and run the focused unit test plus the repository checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] 18 sites rebuild a Date just to reformat it, when formatArrayDate already does exactly that [fineract-backoffice-ui]
Posted by "ltyfulan9 (via GitHub)" <gi...@apache.org>.
ltyfulan9 commented on issue #257:
URL: https://github.com/apache/fineract-backoffice-ui/issues/257#issuecomment-5293695923
Hi, I'd like to work on the transfers part of this issue. I'll update the date conversion in standing-instruction-form.component.ts and add or update the related tests. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org