Skip to content

feat: emit ApprovalStateChangedEvent on approval state changes - #449

Merged
lukasdotcom merged 3 commits into
nextcloud:mainfrom
xyonium:feat/approval-state-changed-event
Oct 14, 2026
Merged

lukasdotcom merged 3 commits into
nextcloud:mainfrom
xyonium:feat/approval-state-changed-event

Conversation

@xyonium

@xyonium xyonium commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Apps reacting to approval state changes (e.g. via workflow engine rules on the approval tags) currently cannot know who requested an approval:

  • the session user at that moment is the approver (on approve/reject)
  • the approval_activity row holding the requester is replaced by storeAction() before the tag change is triggered

This 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 via dispatchTyped() when an approval is

  • requested (ApprovalService::request())
  • requested via manual/auto tag assignment (requestViaTagAssignment())
  • approved (approve())
  • rejected (reject())

The event carries:

getter content
getFileId() the file id
getRuleId() the approval rule id
getNewState() Application::STATE_PENDING / STATE_APPROVED / STATE_REJECTED
getActorUserId() the user performing the action (requester on request, approver otherwise)
getRequesterUserId() the user who requested the approval, if known

On approve/reject the requester is read via getLastAction(..., STATE_PENDING) before storeAction() 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 %r placeholder for the requester's user id.

@github-actions

Copy link
Copy Markdown

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

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.)

@lukasdotcom lukasdotcom left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xyonium for the pr. Sorry for the slow review.

Comment thread lib/Events/ApprovalStateChangedEvent.php Outdated
Comment thread lib/Events/ApprovalStateChangedEvent.php Outdated
xyonium added a commit to xyonium/approval that referenced this pull request Oct 14, 2026
…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>
@xyonium

xyonium commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

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.

@lukasdotcom

Copy link
Copy Markdown
Member

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?

xyonium added a commit to xyonium/approval that referenced this pull request Oct 14, 2026
…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>
@xyonium
xyonium force-pushed the feat/approval-state-changed-event branch from b126370 to 8859eb5 Compare October 14, 2026 14:05
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>
@xyonium
xyonium force-pushed the feat/approval-state-changed-event branch from 8859eb5 to 87de63b Compare October 14, 2026 14:18
@xyonium

xyonium commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Signed ✅ — thanks for pointing it out. I rewrote the branch so every commit is now authored by and carries Signed-off-by: xyonium <shark_xc@hotmail.com> (and dropped the stray AI co-author trailer). DCO checks are green now.

@lukasdotcom lukasdotcom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Just for future information: Don't remove the Co-authored by claude portions. Those are required by the AI policy at Nextcloud.

@lukasdotcom
lukasdotcom merged commit 3084796 into nextcloud:main Oct 14, 2026
43 checks passed
@xyonium

xyonium commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

OK, I'll keep that in mind.

xyonium added a commit to xyonium/approval that referenced this pull request Oct 16, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants