Aman-Mittal commented on PR #39:
URL:
https://github.com/apache/fineract-consumer-facing/pull/39#issuecomment-4977946077
### 🟠Major (6)
3. Magic numbers — NON_CLOSED_STATUS_IDS = Set.of(100L, 200L, 300L, 303L,
304L) with no documentation of what these Fineract status codes mean.
4. initiateAdd / initiateUpdate lack @Transactional(readOnly=true) —
the existsByUserId... check runs without a transaction context, widening
the TOCTOU window.
5. accountType is accepted as raw String — the enum
BeneficiaryAccountType exists; use it directly in the DTO and let Jackson
handle deserialization errors cleanly.
6. UPDATE_ENDPOINT constant value ( "/api/v1/beneficiaries/update" )
doesn't match the actual PUT route — misleading as an action fingerprint
salt.
7. BeneficiaryQueryRepository imports the command-side Beneficiary
domain class — breaks CQRS package isolation.
8. call() Feign-exception wrapper duplicated across Beneficiaries, Loans,
Savings, Transfers — extract a shared utility.
### 🟡 Minor (5)
9. @ToString on BeneficiaryCommandData / BeneficiaryQueryData logs
transferLimit — use opt-in pattern.
10. SessionCommandData regression — removed the defensive
@ToString(onlyExplicitlyIncluded = true) .
11. maskEmail() returns unmasked string when input has no @ symbol.
12. DeviceFingerprintFilter also returns 401 for fingerprint mismatch —
should be 403.
13. Removed "browse mode / detail mode" comments in template DTOs — move to
Javadoc.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
