From 8d9a6e468e2fdfb559e784b5d01515be65116f6d Mon Sep 17 00:00:00 2001 From: xyonium Date: Sun, 27 Sep 2026 12:18:01 +0800 Subject: [PATCH 1/3] feat: emit ApprovalStateChangedEvent on approval state changes 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 https://github.com/nextcloud/workflow_script/issues/363 Signed-off-by: eli Signed-off-by: xyonium --- lib/Events/ApprovalStateChangedEvent.php | 67 ++++++++++++++++++++++ lib/Service/ApprovalService.php | 29 ++++++++++ tests/unit/Service/ApprovalServiceTest.php | 39 ++++++++++++- 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 lib/Events/ApprovalStateChangedEvent.php diff --git a/lib/Events/ApprovalStateChangedEvent.php b/lib/Events/ApprovalStateChangedEvent.php new file mode 100644 index 00000000..0027e08b --- /dev/null +++ b/lib/Events/ApprovalStateChangedEvent.php @@ -0,0 +1,67 @@ +fileId; + } + + public function getRuleId(): int { + return $this->ruleId; + } + + /** + * One of Application::STATE_PENDING, Application::STATE_APPROVED or Application::STATE_REJECTED + */ + public function getNewState(): int { + return $this->newState; + } + + /** + * User who performed the action: + * the requester on request, the approver on approve/reject + */ + public function getActorUserId(): ?string { + return $this->actorUserId; + } + + /** + * User who requested the approval, if known + */ + public function getRequesterUserId(): ?string { + return $this->requesterUserId; + } +} diff --git a/lib/Service/ApprovalService.php b/lib/Service/ApprovalService.php index 48db25b9..3d9408c6 100644 --- a/lib/Service/ApprovalService.php +++ b/lib/Service/ApprovalService.php @@ -10,8 +10,10 @@ use DateTime; use OCA\Approval\Activity\ActivityManager; use OCA\Approval\AppInfo\Application; +use OCA\Approval\Events\ApprovalStateChangedEvent; use OCA\Approval\Exceptions\OutdatedEtagException; use OCP\App\IAppManager; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\ICachedMountFileInfo; use OCP\Files\Config\IUserMountCache; use OCP\Files\FileInfo; @@ -50,6 +52,7 @@ public function __construct( private LoggerInterface $logger, private ?string $userId, private IUserMountCache $userMountCache, + private IEventDispatcher $eventDispatcher, ) { } @@ -381,8 +384,17 @@ public function approve(int $fileId, ?string $userId, string $etag, string $mess try { if ($this->tagObjectMapper->haveTag((string)$fileId, 'files', $rule['tagPending']) && $this->userIsAuthorizedByRule($userId, $rule, 'approvers')) { + // get the requester before storeAction() replaces the pending activity row + $pendingAction = $this->ruleService->getLastAction($fileId, $ruleId, Application::STATE_PENDING); + // store activity in our tables $this->ruleService->storeAction($fileId, $ruleId, $userId, Application::STATE_APPROVED, $message); + + // dispatch before the tags change so listeners reacting to the tag events have the context + $this->eventDispatcher->dispatchTyped( + new ApprovalStateChangedEvent($fileId, $ruleId, Application::STATE_APPROVED, $userId, $pendingAction['userId'] ?? null) + ); + // Change tags $this->tagObjectMapper->assignTags((string)$fileId, 'files', $rule['tagApproved']); $this->tagObjectMapper->unassignTags((string)$fileId, 'files', $rule['tagPending']); @@ -424,9 +436,17 @@ public function reject(int $fileId, ?string $userId, string $etag, string $messa try { if ($this->tagObjectMapper->haveTag((string)$fileId, 'files', $rule['tagPending']) && $this->userIsAuthorizedByRule($userId, $rule, 'approvers')) { + // get the requester before storeAction() replaces the pending activity row + $pendingAction = $this->ruleService->getLastAction($fileId, $ruleId, Application::STATE_PENDING); + // store activity in our tables $this->ruleService->storeAction($fileId, $ruleId, $userId, Application::STATE_REJECTED, $message); + // dispatch before the tags change so listeners reacting to the tag events have the context + $this->eventDispatcher->dispatchTyped( + new ApprovalStateChangedEvent($fileId, $ruleId, Application::STATE_REJECTED, $userId, $pendingAction['userId'] ?? null) + ); + // Change tags $this->tagObjectMapper->assignTags((string)$fileId, 'files', $rule['tagRejected']); $this->tagObjectMapper->unassignTags((string)$fileId, 'files', $rule['tagPending']); @@ -485,6 +505,11 @@ public function request(int $fileId, int $ruleId, ?string $requesterUserId, bool // store activity in our tables $this->ruleService->storeAction($fileId, $ruleId, $requesterUserId, Application::STATE_PENDING); + // dispatch before the tag is assigned so listeners reacting to the tag event have the context + $this->eventDispatcher->dispatchTyped( + new ApprovalStateChangedEvent($fileId, $ruleId, Application::STATE_PENDING, $requesterUserId, $requesterUserId) + ); + $this->tagObjectMapper->assignTags((string)$fileId, 'files', $rule['tagPending']); // still produce an activity entry for the user who requests @@ -530,6 +555,10 @@ public function requestViaTagAssignment(int $fileId, int $ruleId, string $reques // store activity in our tables $this->ruleService->storeAction($fileId, $ruleId, $requesterUserId, Application::STATE_PENDING); + $this->eventDispatcher->dispatchTyped( + new ApprovalStateChangedEvent($fileId, $ruleId, Application::STATE_PENDING, $requesterUserId, $requesterUserId) + ); + // still produce an activity entry for the user who requests $this->activityManager->triggerEvent( ActivityManager::APPROVAL_OBJECT_NODE, $fileId, diff --git a/tests/unit/Service/ApprovalServiceTest.php b/tests/unit/Service/ApprovalServiceTest.php index 40da6fea..194cd43a 100644 --- a/tests/unit/Service/ApprovalServiceTest.php +++ b/tests/unit/Service/ApprovalServiceTest.php @@ -10,7 +10,9 @@ use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Approval\Activity\ActivityManager; use OCA\Approval\AppInfo\Application; +use OCA\Approval\Events\ApprovalStateChangedEvent; use OCP\App\IAppManager; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\Files\IRootFolder; use OCP\ICacheFactory; @@ -97,7 +99,8 @@ protected function setUp(): void { $c->get(IL10N::class), $c->get(LoggerInterface::class), 'user1', - $c->get(IUserMountCache::class) + $c->get(IUserMountCache::class), + $c->get(IEventDispatcher::class) ); // add some tags @@ -427,4 +430,38 @@ public function testRequestWithCreateSharesWhenUserCannotShareReturnsError(): vo $this->ruleService->deleteRule($ruleId); } + + public function testApprovalStateChangedEventIsEmitted(): void { + $app = new Application(); + $eventDispatcher = $app->getContainer()->get(IEventDispatcher::class); + + $emittedEvents = []; + $eventDispatcher->addListener(ApprovalStateChangedEvent::class, function (ApprovalStateChangedEvent $event) use (&$emittedEvents) { + $emittedEvents[] = $event; + }); + + // create a file + $uf1 = $this->root->getUserFolder('user1'); + $file = $uf1->newFile('eventFile.txt', 'content'); + + // requesting approval emits an event carrying the requester + $this->approvalService->request($file->getId(), $this->idRule1, 'user1', false); + $this->assertCount(1, $emittedEvents); + $event = $emittedEvents[0]; + $this->assertSame($file->getId(), $event->getFileId()); + $this->assertEquals($this->idRule1, $event->getRuleId()); + $this->assertSame(Application::STATE_PENDING, $event->getNewState()); + $this->assertSame('user1', $event->getActorUserId()); + $this->assertSame('user1', $event->getRequesterUserId()); + + // approving emits an event still carrying the requester + $this->approvalService->approve($file->getId(), 'user1', $file->getEtag()); + $this->assertCount(2, $emittedEvents); + $event = $emittedEvents[1]; + $this->assertSame($file->getId(), $event->getFileId()); + $this->assertEquals($this->idRule1, $event->getRuleId()); + $this->assertSame(Application::STATE_APPROVED, $event->getNewState()); + $this->assertSame('user1', $event->getActorUserId()); + $this->assertSame('user1', $event->getRequesterUserId()); + } } From 0c97110972508064f14d8b3c1eefb6b0ed92153a Mon Sep 17 00:00:00 2001 From: xyonium Date: Wed, 14 Oct 2026 22:17:22 +0800 Subject: [PATCH 2/3] feat: emit ApprovalStateChangedEvent on approval state changes 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 From 87de63b22f9b5046a7c446d2724d5a6eaf085f0b Mon Sep 17 00:00:00 2001 From: xyonium Date: Wed, 14 Oct 2026 22:14:47 +0800 Subject: [PATCH 3/3] feat: make ApprovalStateChangedEvent webhook-compatible and clarify docblock 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/approval#449. Signed-off-by: xyonium --- lib/Events/ApprovalStateChangedEvent.php | 21 +++++-- .../Events/ApprovalStateChangedEventTest.php | 55 +++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/unit/Events/ApprovalStateChangedEventTest.php diff --git a/lib/Events/ApprovalStateChangedEvent.php b/lib/Events/ApprovalStateChangedEvent.php index 0027e08b..29517d9a 100644 --- a/lib/Events/ApprovalStateChangedEvent.php +++ b/lib/Events/ApprovalStateChangedEvent.php @@ -10,20 +10,23 @@ namespace OCA\Approval\Events; use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IWebhookCompatibleEvent; /** * Emitted when the approval state of a file changes: * requested, approved or rejected. * - * It is dispatched right before the corresponding system tag is - * (un)assigned, so listeners that react to tag assignment events - * (e.g. workflow engines) already have the context available. + * When the state change goes through the approval service, it is dispatched + * right before the corresponding system tag is (un)assigned, so listeners + * that react to tag assignment events (e.g. workflow engines) already have + * the context available. A tag can also be (un)assigned manually, in which + * case no such event is emitted. * * Next to the user who performed the action, it carries the ID of the * user who requested the approval, so listeners can act on behalf of * the requester and not only of the user who performed the latest action. */ -class ApprovalStateChangedEvent extends Event { +class ApprovalStateChangedEvent extends Event implements IWebhookCompatibleEvent { public function __construct( private int $fileId, @@ -64,4 +67,14 @@ public function getActorUserId(): ?string { public function getRequesterUserId(): ?string { return $this->requesterUserId; } + + public function getWebhookSerializable(): array { + return [ + 'actorUserId' => $this->actorUserId, + 'fileId' => $this->fileId, + 'newState' => $this->newState, + 'requesterUserId' => $this->requesterUserId, + 'ruleId' => $this->ruleId, + ]; + } } diff --git a/tests/unit/Events/ApprovalStateChangedEventTest.php b/tests/unit/Events/ApprovalStateChangedEventTest.php new file mode 100644 index 00000000..2a6d6230 --- /dev/null +++ b/tests/unit/Events/ApprovalStateChangedEventTest.php @@ -0,0 +1,55 @@ +assertSame(123, $event->getFileId()); + $this->assertSame(456, $event->getRuleId()); + $this->assertSame(2, $event->getNewState()); + $this->assertSame('robert', $event->getActorUserId()); + $this->assertSame('alice', $event->getRequesterUserId()); + } + + public function testNullRequester(): void { + $event = new ApprovalStateChangedEvent(123, 456, 1, 'alice', null); + + $this->assertSame('alice', $event->getActorUserId()); + $this->assertNull($event->getRequesterUserId()); + } + + public function testGetWebhookSerializableWithRequester(): void { + $event = new ApprovalStateChangedEvent(123, 456, 2, 'robert', 'alice'); + + $this->assertSame([ + 'actorUserId' => 'robert', + 'fileId' => 123, + 'newState' => 2, + 'requesterUserId' => 'alice', + 'ruleId' => 456, + ], $event->getWebhookSerializable()); + } + + public function testGetWebhookSerializableWithoutRequester(): void { + $event = new ApprovalStateChangedEvent(123, 456, 1, 'alice', null); + + $this->assertSame([ + 'actorUserId' => 'alice', + 'fileId' => 123, + 'newState' => 1, + 'requesterUserId' => null, + 'ruleId' => 456, + ], $event->getWebhookSerializable()); + } +}