Repository navigation
feat: emit ApprovalStateChangedEvent on approval state changes - #449
Conversation
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
There was a problem hiding this comment.
Thanks @xyonium for the pr. Sorry for the slow review.
…ocblock Implements IWebhookCompatibleEvent following the nextcloud/mail#12936 pattern: getWebhookSerializable() returns alphabetically-ordered camelCase keys mirroring the event getters, so webhook listeners can consume the approval state change. Also rewords the class docblock to clarify the event only fires on service-driven state changes, not when a tag is (un)assigned manually. Adds unit tests for the getters and the webhook serialization. Addresses review feedback from lukasdotcom on nextcloud#449. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Thanks @lukasdotcom! Addressed both points in b126370: the docblock now clarifies the event only fires on service-driven state changes (not manual tag assignment), and ApprovalStateChangedEvent is now webhook-compatible per the mail#12936 pattern (getWebhookSerializable() + unit tests). Ready for another look when you get a chance. Note: workflow_script#534 consumes this event for the %r placeholder, so it depends on this landing first. |
Could you make sure to sign your newest commit so the DCO check passes? |
…ocblock Implements IWebhookCompatibleEvent following the nextcloud/mail#12936 pattern: getWebhookSerializable() returns alphabetically-ordered camelCase keys mirroring the event getters, so webhook listeners can consume the approval state change. Also rewords the class docblock to clarify the event only fires on service-driven state changes, not when a tag is (un)assigned manually. Adds unit tests for the getters and the webhook serialization. Addresses review feedback from lukasdotcom on nextcloud#449. Signed-off-by: xyonium <shark_xc@hotmail.com>
b126370 to
8859eb5
Compare
The new OCA\Approval\Events\ApprovalStateChangedEvent is dispatched when an approval is requested, approved or rejected, right before the corresponding system tags are (un)assigned. Next to the acting user it carries the user who requested the approval. That information is otherwise lost on approve/reject, because the pending activity row is replaced by the approver's one before the tag change is triggered. This enables other apps, e.g. workflow engines like workflow_script, to react to approval state changes on behalf of the requester, see nextcloud/workflow_script#363 Signed-off-by: eli <shark_xc@hotmail.com> Signed-off-by: xyonium <shark_xc@hotmail.com>
Adds OCA\Approval\Events\ApprovalStateChangedEvent, dispatched on request, approve and reject. It carries the file id, the rule id, the new state and the actor, plus the user id of the requester (captured from the pending activity row before it is replaced), so listeners can act on behalf of the requester. The event is dispatched before the corresponding system tags change so listeners that react to tag assignment events (e.g. workflow engines) already have the context. Signed-off-by: xyonium <shark_xc@hotmail.com>
…ocblock Implements IWebhookCompatibleEvent following the nextcloud/mail#12936 pattern: getWebhookSerializable() returns alphabetically-ordered camelCase keys mirroring the event getters, so webhook listeners can consume the approval state change. Also rewords the class docblock to clarify the event only fires on service-driven state changes, not when a tag is (un)assigned manually. Adds unit tests for the getters and the webhook serialization. Addresses review feedback from lukasdotcom on nextcloud#449. Signed-off-by: xyonium <shark_xc@hotmail.com>
8859eb5 to
87de63b
Compare
|
Signed ✅ — thanks for pointing it out. I rewrote the branch so every commit is now authored by and carries |
lukasdotcom
left a comment
There was a problem hiding this comment.
Thanks. Just for future information: Don't remove the Co-authored by claude portions. Those are required by the AI policy at Nextcloud.
|
OK, I'll keep that in mind. |
…ocblock Implements IWebhookCompatibleEvent following the nextcloud/mail#12936 pattern: getWebhookSerializable() returns alphabetically-ordered camelCase keys mirroring the event getters, so webhook listeners can consume the approval state change. Also rewords the class docblock to clarify the event only fires on service-driven state changes, not when a tag is (un)assigned manually. Adds unit tests for the getters and the webhook serialization. Addresses review feedback from lukasdotcom on nextcloud#449. Assisted-by: ClaudeCode:kimi-k3 Signed-off-by: xyonium <shark_xc@hotmail.com>
Motivation
Apps reacting to approval state changes (e.g. via workflow engine rules on the approval tags) currently cannot know who requested an approval:
approval_activityrow holding the requester is replaced bystoreAction()before the tag change is triggeredThis was reported in nextcloud/workflow_script#363, where a flow script needs the requester's user id to route the file after approval.
What this PR does
Adds
OCA\Approval\Events\ApprovalStateChangedEvent, dispatched viadispatchTyped()when an approval isApprovalService::request())requestViaTagAssignment())approve())reject())The event carries:
getFileId()getRuleId()getNewState()Application::STATE_PENDING/STATE_APPROVED/STATE_REJECTEDgetActorUserId()getRequesterUserId()On approve/reject the requester is read via
getLastAction(..., STATE_PENDING)beforestoreAction()replaces that row, and the event is dispatched before the tags change, so listeners reacting to the tag assignment events (like workflow engines) already have the context within the same request.A unit test asserts the events and their payloads on request and approve.
Counterpart
This enables nextcloud/workflow_script#363 — consumed by nextcloud/workflow_script#534 which adds a
%rplaceholder for the requester's user id.