Bhavya-Sonigra commented on PR #6794:
URL: https://github.com/apache/fineract/pull/6794#issuecomment-6539062875
Hi @galovics , thank you very much for the thorough review and great catches!
All points have been addressed and verified against a live local instance:
1. **Date format / backward compatibility (`closingDate`)**:
- Reverted `closingDate` to `String` in `GLClosureCreateRequest` and
parsed it via `DateUtils.parseLocalDate(closingDate, dateFormat, locale)` in
`GLClosureWritePlatformServiceJpaRepositoryImpl` (following
`TransactionSearchRequest`).
- This preserves full backward compatibility with both the web app and
documented request formats (e.g. `"06 December 2012"`, `"dd MMMM yyyy"`,
`"en"`).
2. **Validation message keys**:
- Added all `org.apache.fineract.accounting.closure.*` message templates
for `id`, `officeId`, `closingDate`, and `comments` in
`fineract-validation/src/main/resources/ValidationMessages.properties`.
3. **Resilience4j retry configuration**:
- Added `resilience4j.retry.instances.*` configuration blocks for
`commandGLClosureCreate`, `commandGLClosureUpdate`, and
`commandGLClosureDelete` in
`fineract-provider/src/main/resources/application.properties`, limiting retries
specifically to optimistic-locking and transient concurrency exceptions.
4. **PUT comments protection**:
- Added `@NotNull` validation on `comments` in `GLClosureUpdateRequest`
and a null-guard in the write service, ensuring an empty update `{}` is
rejected with 400 Bad Request instead of wiping existing comments.
5. **Dead code removal**:
- Removed `GLClosureJsonInputParams` and cleaned up a legacy
cross-reference in `AccountingRuleWritePlatformServiceJpaRepositoryImpl`.
6. **Integration test**:
- Added `GLClosureIntegrationTest` covering:
- Creation using the documented non-ISO payload (`"06 December 2012"`,
`"dd MMMM yyyy"`, `"en"`) and verifying the parsed date.
- Verification that an empty update `{}` returns 400 Bad Request.
- Updating comments and deleting closures.
All unit tests (`fineract-accounting:test`), the new integration test
(`integration-tests`), `spotlessCheck`, and `rat` license checks are passing.
--
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]
