diff --git a/application/Espo/Classes/RecordHooks/Webhook/BeforeSave.php b/application/Espo/Classes/RecordHooks/Webhook/BeforeSave.php index 49edd568f4..774815fbee 100644 --- a/application/Espo/Classes/RecordHooks/Webhook/BeforeSave.php +++ b/application/Espo/Classes/RecordHooks/Webhook/BeforeSave.php @@ -64,6 +64,10 @@ class BeforeSave implements SaveHook public function process(Entity $entity): void { + if ($entity->skipOwn() && !$entity->getUserId()) { + $entity->setSkipOwn(false); + } + $this->checkEntityUserIsApi($entity); $this->processEntityEventData($entity); diff --git a/application/Espo/Core/Webhook/Manager.php b/application/Espo/Core/Webhook/Manager.php index 9654f75002..535c867feb 100644 --- a/application/Espo/Core/Webhook/Manager.php +++ b/application/Espo/Core/Webhook/Manager.php @@ -174,13 +174,16 @@ class Manager private function logDebugEvent(string $event, Entity $entity): void { - $this->log->debug("Webhook: $event on record {$entity->getId()}."); + $this->log->debug("Webhook: {event} on record {id}.", [ + 'id' => $entity->getId(), + 'event' => $event, + ]); } /** * Process 'create' event. */ - public function processCreate(Entity $entity): void + public function processCreate(Entity $entity, Options $options): void { $event = "{$entity->getEntityType()}.create"; @@ -193,7 +196,8 @@ class Manager $item ->setEvent($event) ->setTarget($entity) - ->setData($entity->getValueMap()); + ->setData($entity->getValueMap()) + ->setUserId($options->userId); $this->entityManager->saveEntity($item); @@ -203,7 +207,7 @@ class Manager /** * Process 'delete' event. */ - public function processDelete(Entity $entity): void + public function processDelete(Entity $entity, Options $options): void { $event = "{$entity->getEntityType()}.delete"; @@ -216,9 +220,8 @@ class Manager $item ->setEvent($event) ->setTarget($entity) - ->setData([ - 'id' => $entity->getId(), - ]); + ->setData(['id' => $entity->getId()]) + ->setUserId($options->userId); $this->entityManager->saveEntity($item); @@ -228,7 +231,7 @@ class Manager /** * Process 'update' event. */ - public function processUpdate(Entity $entity): void + public function processUpdate(Entity $entity, Options $options): void { $event = "{$entity->getEntityType()}.update"; @@ -256,17 +259,18 @@ class Manager $item ->setEvent($event) ->setTarget($entity) - ->setData($data); + ->setData($data) + ->setUserId($options->userId); $this->entityManager->saveEntity($item); $this->logDebugEvent($event, $entity); } - $this->processUpdateFields($entity, $data); + $this->processUpdateFields($entity, $data, $options); } - private function processUpdateFields(Entity $entity, stdClass $data): void + private function processUpdateFields(Entity $entity, stdClass $data, Options $options): void { foreach ($this->fieldUtil->getEntityTypeFieldList($entity->getEntityType()) as $field) { $itemEvent = "{$entity->getEntityType()}.fieldUpdate.$field"; @@ -278,7 +282,7 @@ class Manager continue; } - $this->processUpdateField($entity, $field, $itemEvent); + $this->processUpdateField($entity, $field, $itemEvent, $options); } } @@ -303,7 +307,7 @@ class Manager return $isChanged; } - private function processUpdateField(Entity $entity, string $field, string $itemEvent): void + private function processUpdateField(Entity $entity, string $field, string $itemEvent, Options $options): void { $itemData = (object) []; @@ -324,7 +328,8 @@ class Manager $item ->setEvent($itemEvent) ->setTarget($entity) - ->setData($itemData); + ->setData($itemData) + ->setUserId($options->userId); $this->entityManager->saveEntity($item); diff --git a/application/Espo/Core/Webhook/Options.php b/application/Espo/Core/Webhook/Options.php new file mode 100644 index 0000000000..98e2b89904 --- /dev/null +++ b/application/Espo/Core/Webhook/Options.php @@ -0,0 +1,40 @@ +. + * + * The interactive user interfaces in modified source and object code versions + * of this program must display Appropriate Legal Notices, as required under + * Section 5 of the GNU Affero General Public License version 3. + * + * In accordance with Section 7(b) of the GNU Affero General Public License version 3, + * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. + ************************************************************************/ + +namespace Espo\Core\Webhook; + +readonly class Options +{ + /** + * @param ?string $userId A user initiated an event. + */ + public function __construct( + public ?string $userId = null, + ) {} +} diff --git a/application/Espo/Core/Webhook/Queue.php b/application/Espo/Core/Webhook/Queue.php index 0b89fa3613..3713fdc473 100644 --- a/application/Espo/Core/Webhook/Queue.php +++ b/application/Espo/Core/Webhook/Queue.php @@ -64,7 +64,7 @@ class Queue private Config $config, private EntityManager $entityManager, private AclManager $aclManager, - private Log $log + private Log $log, ) {} public function process(): void @@ -92,35 +92,25 @@ class Queue } } - protected function createQueueFromEvent(WebhookEventQueueItem $item): void + protected function createQueueFromEvent(WebhookEventQueueItem $eventItem): void { - if (!$item->getTargetId() || !$item->getTargetType()) { + if (!$eventItem->getTargetId() || !$eventItem->getTargetType()) { return; } - $target = LinkParent::create($item->getTargetType(), $item->getTargetId()); + $target = LinkParent::create($eventItem->getTargetType(), $eventItem->getTargetId()); $webhooks = $this->entityManager ->getRDBRepositoryByClass(Webhook::class) ->where([ - 'event' => $item->getEvent(), + 'event' => $eventItem->getEvent(), 'isActive' => true, ]) ->order(Field::CREATED_AT) ->find(); foreach ($webhooks as $webhook) { - $queueItem = $this->entityManager->getRDBRepositoryByClass(WebhookQueueItem::class)->getNew(); - - $queueItem - ->setEvent($item->getEvent()) - ->setWebhook($webhook) - ->setTarget($target) - ->setStatus(WebhookQueueItem::STATUS_PENDING) - ->setAttempts(0) - ->setData($item->getData()); - - $this->entityManager->saveEntity($queueItem); + $this->createItem($eventItem, $webhook, $target); } } @@ -396,4 +386,27 @@ class Queue $this->entityManager->saveEntity($item); } + + private function createItem(WebhookEventQueueItem $eventItem, Webhook $webhook, LinkParent $target): void + { + if ( + $webhook->skipOwn() && + $webhook->getUserId() && + $eventItem->getUserId() === $webhook->getUserId() + ) { + return; + } + + $item = $this->entityManager->getRDBRepositoryByClass(WebhookQueueItem::class)->getNew(); + + $item + ->setEvent($eventItem->getEvent()) + ->setWebhook($webhook) + ->setTarget($target) + ->setStatus(WebhookQueueItem::STATUS_PENDING) + ->setAttempts(0) + ->setData($eventItem->getData()); + + $this->entityManager->saveEntity($item); + } } diff --git a/application/Espo/Entities/Webhook.php b/application/Espo/Entities/Webhook.php index b56111fc31..e91313e94d 100644 --- a/application/Espo/Entities/Webhook.php +++ b/application/Espo/Entities/Webhook.php @@ -64,4 +64,14 @@ class Webhook extends Entity { return $this->get('entityType'); } + + public function setSkipOwn(bool $skipOwn): self + { + return $this->set('skipOwn', $skipOwn); + } + + public function skipOwn(): bool + { + return (bool) $this->get('skipOwn'); + } } diff --git a/application/Espo/Entities/WebhookEventQueueItem.php b/application/Espo/Entities/WebhookEventQueueItem.php index 9e067f5d5d..c9ad4e71fa 100644 --- a/application/Espo/Entities/WebhookEventQueueItem.php +++ b/application/Espo/Entities/WebhookEventQueueItem.php @@ -59,6 +59,11 @@ class WebhookEventQueueItem extends Entity return $this->get('targetId'); } + public function getUserId(): ?string + { + return $this->get('userId'); + } + public function getData(): stdClass { return $this->get('data') ?? (object) []; @@ -69,6 +74,11 @@ class WebhookEventQueueItem extends Entity return $this->set('event', $event); } + public function setUserId(?string $userId): self + { + return $this->set('userId', $userId); + } + public function setTarget(LinkParent|Entity $target): self { return $this->setRelatedLinkOrEntity('target', $target); diff --git a/application/Espo/Hooks/Common/Webhook.php b/application/Espo/Hooks/Common/Webhook.php index 4a9e297d7b..f19b834dc2 100644 --- a/application/Espo/Hooks/Common/Webhook.php +++ b/application/Espo/Hooks/Common/Webhook.php @@ -31,6 +31,8 @@ namespace Espo\Hooks\Common; use Espo\Core\Hook\Hook\AfterRemove; use Espo\Core\Hook\Hook\AfterSave; +use Espo\Core\Webhook\Options; +use Espo\Entities\User; use Espo\ORM\Entity; use Espo\Core\ORM\Entity as CoreEntity; @@ -48,8 +50,11 @@ class Webhook implements AfterSave, AfterRemove { public static int $order = 101; - public function __construct(private Metadata $metadata, private WebhookManager $webhookManager) - {} + public function __construct( + private Metadata $metadata, + private WebhookManager $webhookManager, + private User $user, + ) {} public function afterSave(Entity $entity, SaveOptions $options): void { @@ -60,11 +65,19 @@ class Webhook implements AfterSave, AfterRemove return; } + $userIdParam = $entity->isNew() ? SaveOption::CREATED_BY_ID : SaveOption::MODIFIED_BY_ID; + + $eventOptions = new Options( + userId: $options->get($userIdParam) ?? $this->user->getId(), + ); + if ($entity->isNew()) { - $this->webhookManager->processCreate($entity); - } else { - $this->webhookManager->processUpdate($entity); + $this->webhookManager->processCreate($entity, $eventOptions); + + return; } + + $this->webhookManager->processUpdate($entity, $eventOptions); } public function afterRemove(Entity $entity, RemoveOptions $options): void @@ -76,7 +89,11 @@ class Webhook implements AfterSave, AfterRemove return; } - $this->webhookManager->processDelete($entity); + $eventOptions = new Options( + userId: $options->get(SaveOption::MODIFIED_BY_ID) ?? $this->user->getId(), + ); + + $this->webhookManager->processDelete($entity, $eventOptions); } private function toSkip(SaveOptions|RemoveOptions $options, Entity $entity): bool diff --git a/application/Espo/Resources/i18n/en_US/Webhook.json b/application/Espo/Resources/i18n/en_US/Webhook.json index ac285e1799..60bbfb8363 100644 --- a/application/Espo/Resources/i18n/en_US/Webhook.json +++ b/application/Espo/Resources/i18n/en_US/Webhook.json @@ -9,10 +9,14 @@ "user": "API User", "entityType": "Entity Type", "field": "Field", - "secretKey": "Secret Key" + "secretKey": "Secret Key", + "skipOwn": "Skip Own" }, "links": { "user": "User", "queueItems": "Queue Items" + }, + "tooltips": { + "skipOwn": "Do not send if the event was initiated by the user owning the webhook." } } diff --git a/application/Espo/Resources/i18n/en_US/WebhookEventQueueItem.json b/application/Espo/Resources/i18n/en_US/WebhookEventQueueItem.json index 3ceb8da10d..62997cd6f2 100644 --- a/application/Espo/Resources/i18n/en_US/WebhookEventQueueItem.json +++ b/application/Espo/Resources/i18n/en_US/WebhookEventQueueItem.json @@ -3,6 +3,11 @@ "event": "Event", "target": "Target", "data": "Data", - "isProcessed": "Is Processed" + "isProcessed": "Is Processed", + "user": "User" + }, + "links": { + "target": "Target", + "user": "User" } } diff --git a/application/Espo/Resources/layouts/Webhook/detail.json b/application/Espo/Resources/layouts/Webhook/detail.json index 2ef62facd0..9b1ca5ec7a 100644 --- a/application/Espo/Resources/layouts/Webhook/detail.json +++ b/application/Espo/Resources/layouts/Webhook/detail.json @@ -11,7 +11,7 @@ ], [ {"name": "secretKey"}, - false + {"name": "skipOwn"} ] ] } diff --git a/application/Espo/Resources/layouts/Webhook/detailSmall.json b/application/Espo/Resources/layouts/Webhook/detailSmall.json index 12211c3060..f5a653114f 100644 --- a/application/Espo/Resources/layouts/Webhook/detailSmall.json +++ b/application/Espo/Resources/layouts/Webhook/detailSmall.json @@ -6,13 +6,14 @@ {"name": "isActive"} ], [ - {"name": "url", "fullWidth": true} + {"name": "url"} ], [ - {"name": "user", "fullWidth": true} + {"name": "user"}, + {"name": "skipOwn"} ], [ - {"name": "secretKey", "fullWidth": true} + {"name": "secretKey"} ] ] } diff --git a/application/Espo/Resources/layouts/WebhookEventQueueItem/detail.json b/application/Espo/Resources/layouts/WebhookEventQueueItem/detail.json index e2ba835654..76800b6d6e 100644 --- a/application/Espo/Resources/layouts/WebhookEventQueueItem/detail.json +++ b/application/Espo/Resources/layouts/WebhookEventQueueItem/detail.json @@ -7,7 +7,7 @@ ], [ {"name": "target"}, - false + {"name": "user"} ], [ {"name": "data"} diff --git a/application/Espo/Resources/layouts/WebhookEventQueueItem/detailSmall.json b/application/Espo/Resources/layouts/WebhookEventQueueItem/detailSmall.json index e2ba835654..76800b6d6e 100644 --- a/application/Espo/Resources/layouts/WebhookEventQueueItem/detailSmall.json +++ b/application/Espo/Resources/layouts/WebhookEventQueueItem/detailSmall.json @@ -7,7 +7,7 @@ ], [ {"name": "target"}, - false + {"name": "user"} ], [ {"name": "data"} diff --git a/application/Espo/Resources/layouts/WebhookEventQueueItem/filters.json b/application/Espo/Resources/layouts/WebhookEventQueueItem/filters.json index 8c00ca867f..4046c1a135 100644 --- a/application/Espo/Resources/layouts/WebhookEventQueueItem/filters.json +++ b/application/Espo/Resources/layouts/WebhookEventQueueItem/filters.json @@ -2,5 +2,6 @@ "event", "target", "isProcessed", + "user", "createdAt" ] diff --git a/application/Espo/Resources/layouts/WebhookEventQueueItem/list.json b/application/Espo/Resources/layouts/WebhookEventQueueItem/list.json index e3a611bfbb..38625e894e 100644 --- a/application/Espo/Resources/layouts/WebhookEventQueueItem/list.json +++ b/application/Espo/Resources/layouts/WebhookEventQueueItem/list.json @@ -14,5 +14,10 @@ { "name": "createdAt", "width": 14 + }, + { + "name": "user", + "width": 15, + "hidden": true } ] diff --git a/application/Espo/Resources/metadata/clientDefs/WebhookEventQueueItem.json b/application/Espo/Resources/metadata/clientDefs/WebhookEventQueueItem.json index b40f588359..9c42d61665 100644 --- a/application/Espo/Resources/metadata/clientDefs/WebhookEventQueueItem.json +++ b/application/Espo/Resources/metadata/clientDefs/WebhookEventQueueItem.json @@ -5,6 +5,7 @@ "mergeDisabled": true, "exportDisabled": true, "textFilterDisabled": true, + "forceListViewSettings": true, "menu": { "list": { "dropdown": [ diff --git a/application/Espo/Resources/metadata/entityDefs/Webhook.json b/application/Espo/Resources/metadata/entityDefs/Webhook.json index 397b3d6649..0a758fde61 100644 --- a/application/Espo/Resources/metadata/entityDefs/Webhook.json +++ b/application/Espo/Resources/metadata/entityDefs/Webhook.json @@ -47,6 +47,10 @@ "layoutFiltersDisabled": true, "layoutListDisabled": true }, + "skipOwn": { + "type": "bool", + "tooltip": true + }, "createdAt": { "type": "datetime", "readOnly": true diff --git a/application/Espo/Resources/metadata/entityDefs/WebhookEventQueueItem.json b/application/Espo/Resources/metadata/entityDefs/WebhookEventQueueItem.json index 6ce7e445e8..48c6fda833 100644 --- a/application/Espo/Resources/metadata/entityDefs/WebhookEventQueueItem.json +++ b/application/Espo/Resources/metadata/entityDefs/WebhookEventQueueItem.json @@ -13,6 +13,9 @@ "target": { "type": "linkParent" }, + "user": { + "type": "link" + }, "data": { "type": "jsonObject" }, @@ -27,6 +30,10 @@ "links": { "target": { "type": "belongsToParent" + }, + "user": { + "type": "belongsTo", + "entity": "User" } }, "collection": { diff --git a/application/Espo/Resources/metadata/logicDefs/Webhook.json b/application/Espo/Resources/metadata/logicDefs/Webhook.json index f30caca6bb..467ae8fb2a 100644 --- a/application/Espo/Resources/metadata/logicDefs/Webhook.json +++ b/application/Espo/Resources/metadata/logicDefs/Webhook.json @@ -19,6 +19,19 @@ } ] } + }, + "skipOwn": { + "visible": { + "conditionGroup": [ + { + "type": "isNotEmpty", + "attribute": "userId", + "data": { + "field": "user" + } + } + ] + } } } } diff --git a/tests/integration/Espo/Webhook/ProcessingTest.php b/tests/integration/Espo/Webhook/ProcessingTest.php index fc237513dc..be48368777 100644 --- a/tests/integration/Espo/Webhook/ProcessingTest.php +++ b/tests/integration/Espo/Webhook/ProcessingTest.php @@ -29,16 +29,16 @@ namespace tests\integration\Espo\Webhook; +use Espo\Core\InjectableFactory; +use Espo\Core\ORM\Repository\Option\SaveOption; use Espo\Core\Webhook\Queue; use Espo\Core\Webhook\Sender; - use Espo\ORM\EntityManager; - use Espo\Entities\Webhook; - use Espo\Modules\Crm\Entities\Account; +use tests\integration\Core\BaseTestCase; -class ProcessingTest extends \tests\integration\Core\BaseTestCase +class ProcessingTest extends BaseTestCase { public function testProcessing1(): void { @@ -58,24 +58,25 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase ] ); - /* @var $em EntityManager */ - $em = $this->getContainer()->get('entityManager'); + $em = $this->getContainer()->getByClass(EntityManager::class); $em->createEntity(Webhook::ENTITY_TYPE, [ 'event' => 'Account.create', 'userId' => $user->getId(), 'url' => 'https://test', + 'skipOwn' => true, ]); $em->createEntity(Webhook::ENTITY_TYPE, [ 'event' => 'Account.update', 'userId' => $user->getId(), 'url' => 'https://test', + 'skipOwn' => true, ]); $app = $this->createApplication(); - $em = $app->getContainer()->get('entityManager'); + $em = $app->getContainer()->getByClass(EntityManager::class); $account1 = $em->createEntity(Account::ENTITY_TYPE, [ 'name' => 'test1', @@ -91,15 +92,22 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase 'name' => 'test3', ]); + $em->createEntity(Account::ENTITY_TYPE, [ + 'name' => 'test_skip', + 'assignedUserId' => $user->getId(), + ], [SaveOption::CREATED_BY_ID => $user->getId()]); + $dataList1 = [ $account1->getValueMap(), $account2->getValueMap(), ]; $account1->set('name', 'test-1-changed'); - $em->saveEntity($account1); + $account1->set('description', 'test-skip'); + $em->saveEntity($account1, [SaveOption::MODIFIED_BY_ID => $user->getId()]); + $dataList2 = [ (object) [ 'name' => $account1->get('name'), @@ -111,42 +119,52 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase $sender = $this->createMock(Sender::class); - /* @var $queue Queue */ + /** @var Queue $queue */ $queue = $app->getContainer() - ->get('injectableFactory') + ->getByClass(InjectableFactory::class) ->createWith(Queue::class, [ 'sender' => $sender, ]); + $invokedCount = $this->exactly(2); + + $notSame = false; + $sender - ->expects($this->exactly(2)) + ->expects($invokedCount) ->method('send') - ->withConsecutive( - [ - $this->callback( - function (Webhook $webhook){ - return $webhook->get('event') === 'Account.create'; - } - ), - $dataList1, - ], - [ - $this->callback( - function (Webhook $webhook){ - return $webhook->get('event') === 'Account.update'; - } - ), - $dataList2, - ] - ) - ->willReturn( - 200, - 200 - ); + ->willReturnCallback(function (Webhook $webhook, $dataList) use ($invokedCount, $dataList1, $dataList2, &$notSame) { + if ($invokedCount->getInvocationCount() === 1) { + if (count($dataList) !== count($dataList1)) { + $notSame = true; + } + + if ('Account.create' !== $webhook->getEvent()) { + $notSame = true; + } + + return 200; + } + + if ($invokedCount->getInvocationCount() === 2) { + if (count($dataList) !== count($dataList2)) { + $notSame = true; + } + + if ('Account.update' !== $webhook->getEvent()) { + $notSame = true; + } + + return 200; + } + + return 0; + }); $queue->process(); - $queue->process(); + + $this->assertFalse($notSame); } public function testProcessing2(): void @@ -177,10 +195,16 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase 'assignedUserId' => $user->getId(), ]); + $account2 = $em->createEntity(Account::ENTITY_TYPE, [ + 'name' => 'test2', + 'assignedUserId' => $user->getId(), + ]); + $em->createEntity(Webhook::ENTITY_TYPE, [ 'event' => 'Account.delete', 'userId' => $user->getId(), 'url' => 'https://test', + 'skipOwn' => true, ]); $app = $this->createApplication(); @@ -188,12 +212,7 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase $em = $app->getContainer()->get('entityManager'); $em->removeEntity($account1); - - $dataList1 = [ - (object) [ - 'id' => $account1->getId(), - ] - ]; + $em->removeEntity($account2, [SaveOption::MODIFIED_BY_ID => $user->getId()]); $sender = $this->createMock(Sender::class); @@ -204,26 +223,39 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase 'sender' => $sender, ]); + $dataList1 = [ + (object) [ + 'id' => $account1->getId(), + ] + ]; + + $invokedCount = $this->exactly(1); + + $notSame = false; + $sender - ->expects($this->exactly(1)) + ->expects($invokedCount) ->method('send') - ->withConsecutive( - [ - $this->callback( - function (Webhook $webhook){ - return $webhook->get('event') === 'Account.delete'; - } - ), - $dataList1, - ], - ) - ->willReturn( - 200, - ); + ->willReturnCallback(function (Webhook $webhook, $dataList) use ($invokedCount, $dataList1, &$notSame) { + if ($invokedCount->getInvocationCount() === 1) { + if ('Account.delete' !== $webhook->getEvent()) { + $notSame = true; + } + + if (json_encode($dataList) !== json_encode($dataList1)) { + $notSame = true; + } + + return 200; + } + + return 0; + }); $queue->process(); - $queue->process(); + + $this->assertFalse($notSame); } public function testProcessing3(): void @@ -284,25 +316,32 @@ class ProcessingTest extends \tests\integration\Core\BaseTestCase 'sender' => $sender, ]); + $invokedCount = $this->exactly(1); + + $notSame = false; + $sender - ->expects($this->exactly(1)) + ->expects($invokedCount) ->method('send') - ->withConsecutive( - [ - $this->callback( - function (Webhook $webhook){ - return $webhook->get('event') === 'Account.fieldUpdate.name'; - } - ), - $dataList1, - ], - ) - ->willReturn( - 200, - ); + ->willReturnCallback(function (Webhook $webhook, $dataList) use ($invokedCount, $dataList1, &$notSame) { + if ($invokedCount->getInvocationCount() === 1) { + if (json_encode($dataList) !== json_encode($dataList1)) { + $notSame = true; + } + + if ('Account.fieldUpdate.name' !== $webhook->getEvent()) { + $notSame = true; + } + + return 200; + } + + return 0; + }); $queue->process(); - $queue->process(); + + $this->assertFalse($notSame); } }