[rshdhere]

Description

JIRA: https://issues.apache.org/jira/browse/FINERACT-2536

Document commands have never notified webhooks subscribed to DOCUMENT events. Before FINERACT-2246, DocumentManagementApiResource called DocumentWritePlatformService directly instead of going through commandsSourceWritePlatformService, so the legacy SynchronousCommandProcessingService never published their HookEvents. The new CommandDispatcher runs its after-hooks, but none of them published one either. This adds the missing feature to the new command processing.

Changes

• HookableCommand (fineract-command): an opt-in interface through which a command declares the entity and action its webhooks subscribe to (hookEntityName() / hookActionName()).
• WebhookCommandHookAfter (fineract-core): a CommandHookAfter that publishes the HookEvent for HookableCommands. As in the legacy path, publication waits for the enclosing transaction to commit, and a failure to publish is logged without failing the command.
• Document commands declare DOCUMENT CREATE, UPDATE and DELETE.

Payload compared with the legacy path

• Same fields: entityName, actionName, createdBy, createdByName, createdByFullName, request, response, timestamp.
• When the response has officeId/clientId, they are copied to the top level of the payload and officeId is cleared inside response, as in SynchronousCommandProcessingService. TwilioHookProcessor and MessageGatewayHookProcessor look up the client there, so SMS hooks work for migrated client commands that opt in. Document responses carry neither, so document payloads have no top-level officeId/clientId.
• Difference: request is the serialized request DTO rather than the raw JSON body (e.g. the uploaded file content is not included).

Other commands migrated to the new processing can opt in the same way; I kept this PR to documents and am happy to follow up for the rest.

Testing

• WebhookCommandHookAfterTest: payload contents, top-level officeId/clientId when the response provides them, commands that don't opt in are skipped, publication is deferred until after commit, and publish failures don't propagate.
• Manually against a local instance with a webhook subscribed to DOCUMENT CREATE/UPDATE/DELETE and CLIENT CREATE: all three document events are delivered, and CLIENT CREATE is still delivered exactly once.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Write the commit message as per our guidelines
• Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
• Create/update unit or integration tests for verifying the changes made.
• Follow our coding conventions.
• Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
• This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
• If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.
• I followed the AI Policy.

Your assigned reviewer(s) will follow our guidelines for code reviews.

[IOhacker]

@vidakovic could you please give us feedback about this implementation?

[rshdhere]

@vidakovic PTAL

[adamsaghy]

Thanks for the PR. The DOCUMENT CREATE/UPDATE/DELETE hooks now fire, which is what FINERACT-2536 asks for. Two things in the description don't match the code:

1. The root cause. The description says DOCUMENT webhooks stopped firing when FINERACT-2246 moved documents to the new command processing. They never fired. Before 2ff0c62 (FINERACT-2246), DocumentManagementApiResource called documentWritePlatformService.createDocument/updateDocument/deleteDocument directly. It never went through commandsSourceWritePlatformService, and that has been true since the resource was added in 2012, so SynchronousCommandProcessingService.publishHookEvent was never reached for documents. Please fix the description and commit message. As written, this reads like a regression fix, and it is really a missing feature.

2. "Same payload fields as the legacy path". It isn't the same. The legacy publishHookEvent also puts top-level officeId and clientId into the payload, taken from the CommandProcessingResult, and nulls officeId inside response (SynchronousCommandProcessingService L417-424). WebhookCommandHookAfter only sends entityName, actionName, createdBy*, request, response and timestamp.

This matters beyond documents, because HookableCommand is meant to be the opt-in for every migrated command. TwilioHookProcessor (L107) and MessageGatewayHookProcessor (L97) only work when clientId is at the top level of the payload. Any migrated command that opts in, a client command for example, will publish the event, but SMS/Twilio hooks will quietly do nothing.

Please either:

• add officeId/clientId when the response provides them, or
• drop the "same payload fields" claim and state the difference (the payload shape, and that request is now the serialized request DTO rather than the raw JSON body).

Also please set up GPG signature and sign your commits

[rshdhere]

Thanks @adamsaghy, you're right on both counts.

1. I checked 2ff0c62^: DocumentManagementApiResource called DocumentWritePlatformService directly and never went through commandsSourceWritePlatformService, so DOCUMENT hooks never fired. I've reworded the commit message and the description to present this as a missing feature, not a regression fix.

2. I went with the first option. WebhookCommandHookAfter now copies officeId/clientId to the top level of the payload when the response has them, and sets officeId to null inside response, as SynchronousCommandProcessingService does. That way TwilioHookProcessor and MessageGatewayHookProcessor will work for migrated client commands that opt in. I added a test for this. I also updated the description to point out the remaining difference: request is now the serialized request DTO rather than the raw JSON body.

The commit was GPG-signed, but my public key wasn't on my GitHub account, so it didn't show as verified. I've added the key and force-pushed the re-signed commit.

[rshdhere]

@adamsaghy PTAL

[adamsaghy]

LGTM. Document create/update/delete now publish webhook events via HookableCommand, after commit, with the top-level officeId/clientId as in the legacy path; my earlier points are addressed. Please drop the LoanApplicationTermsTest commit when rebasing, develop already has that change.

[rshdhere]

Rebased on develop and dropped the LoanApplicationTermsTest commit.
Thanks for the review!

could you approve the workflow runs when you get a chance?