Conversation
MatiasArriola
left a comment
There was a problem hiding this comment.
Thanks @idelcano! Overall the PR looks good, and it's nice to see test coverage.
Some code review comments only (I haven't tested):
- We would need resolving the conflicts with development before merging
userMonitorintg.tsparseUserDateOverridesFile: Replace manual json validation with aCodec. For reference seelocalConfigCodecin the same file. Then you can keep the ISO date check as a second semantic validation step, or wrap that in a refined/custom codec.- this would make the code consistent and also validate that the json effectively is correctly shaped (i.e.
users[n].idis mandatory)
- this would make the code consistent and also validate that the json effectively is correctly shaped (i.e.
- The PR says
--created-date-overrides-filehas no effect without--filtered-by-month, but the file is parsed and validated even without--filtered-by-monthset.- Maybe we can move the overrides parsing behind the
filteredByMonthguard - not really important but for sake of correctness
- Maybe we can move the overrides parsing behind the
All fixed |
cgbautista
left a comment
There was a problem hiding this comment.
Works as expected, both to force a recent date and for forcing an old date.
issue: https://app.clickup.com/t/4528615/869dw5eb9
Extends the
run-2fa-reportercommand with a new--created-date-overrides-fileflag.When
--filtered-by-monthis active, the reporter only disables invalid 2FA users whoseaccount is older than
disableAfterMonths(from the datastore). This new flag lets youoverride the creation date used for that check on a per-user basis: instead of the real
DHIS2 creation date, the script uses the date provided in a JSON file.
This is useful when an account exists in DHIS2 from long ago but should be treated as
recently created for this check — e.g. to protect a re-onboarded user from being disabled.
The override is a pure date substitution and works in both directions: a user given a
recent date is protected, and a user given an old-enough date is still disabled.
Also adds the previously-missing README documentation for the existing
--filtered-by-monthflag.Implementation notes
UserDateOverrideentity and the pureapplyUserDateOverridesfunction live in thedomain layer next to
filterByCreationDate.parseUserDateOverridesFile),consistent with how
getApiFromConfigFileis handled.createdDateis validated as a valid ISO 8601 date using the same parser (luxon) that thedate filter relies on, so a non-ISO value fails loudly instead of silently skipping a user.
usernamein the file is optional and is not processed — it's only there for readability.Usage
Overrides file (
overrides.json):{ "createdDate": "2026-06-10", "users": [ { "id": "uid1" }, { "id": "uid2", "username": "pepito" } ] } Run: yarn start usermonitoring run-2fa-reporter \ --config-file config.json \ --disable-users \ --filtered-by-month \ --created-date-overrides-file overrides.json --created-date-overrides-file has no effect without --filtered-by-month; the script logs a warning if the file is passed without it. Test plan - [ ] Run without --created-date-overrides-file → existing behaviour unchanged. - [ ] Override a user (who would be disabled) with a recent createdDate → user is not disabled. - [ ] Override a recently-created user with an old createdDate → user is disabled. - [ ] Pass the file without --filtered-by-month → warning is logged, file has no effect. - [ ] Pass a malformed / non-JSON file, a file missing createdDate/users, or a non-ISO date → script fails with a clear error message. - [ ] Unit tests: yarn test-unit (80 tests passing).