rymghosn commented on PR #6255: URL: https://github.com/apache/fineract/pull/6255#issuecomment-5989884830
@adamsaghy Thanks for the detailed review. I've rebased onto the latest `develop`, addressed all five points and rewritten the description to match the code: 1. **403, not 409:** both exceptions now extend `AbstractPlatformDomainRuleException`, and the two custom mappers are removed. `PlatformDomainRuleExceptionMapper` answers 403 on REST and batch, with a matching `httpStatusCode` of `403` in the body. 2. **Exemption:** only `isCheckerSuperUser()` skips the duplicate check now, the same rule `CommandSourceService#validateMakerChecker` uses to decide whether a submission is queued. A user holding both `ACTIVATE_CLIENT` and `ACTIVATE_CLIENT_CHECKER` is still a maker, so the check applies to them. The integration test uses exactly that role shape. 3. **"Same underlying change":** the key now includes the sub-resource id, so undoing two different transactions on one savings account is two different changes. It also includes the request payload (`command_as_json`), because two different changes can share the same resource and sub-resource (e.g. activating a client with two different dates). Only an identical resubmission is refused now. 4. **Who the maker is:** blocking across makers is intended. A second maker's identical submission would create the same redundant inbox entry for the checker. The description now says so, and the integration test covers it. 5. **CREATE carve-out:** documented in the description and in a one-line comment at the check. A command without a resource id has no existing resource to match a pending entry against. Tests: the unit tests cover each case above. A new `MakercheckerTest` integration test checks the HTTP 403 status, the body's `httpStatusCode` and the error codes, the cross-maker refusal, the different-payload case and the checker-only refusal. The full `MakercheckerTest` class passes on PostgreSQL. On #6282, which you asked about: the two are meant to complement each other. This PR handles a checker-only user with **no** matching pending entry (403 with guidance). #6282 handles one **with** a matching pending entry. Could you please re-review? 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
