From 18b6e6eb3049de20f3f4d0affcad2fc48e237ffc Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 17:16:01 -0300 Subject: [PATCH 01/11] Implement filter at Activity Signed-off-by: Vitor Mattos --- appinfo/info.xml | 5 +- lib/Activity/{Setting.php => FileToSign.php} | 25 +++- lib/Activity/Filter.php | 67 +++++++++ lib/Activity/Listener.php | 54 ++++++-- lib/Activity/Provider/SignRequest.php | 26 ++-- lib/Command/Developer/Reset.php | 8 ++ lib/Events/SendSignNotificationEvent.php | 6 + lib/Listener/NotificationListener.php | 21 ++- lib/Notification/Notifier.php | 10 +- lib/Service/IdentifyMethod/Account.php | 2 + lib/Service/RequestSignatureService.php | 57 ++++++-- tests/integration/composer.json | 2 +- tests/integration/composer.lock | 128 +++++++++++++++++- .../features/bootstrap/FeatureContext.php | 3 + .../integration/features/sign/request.feature | 23 +++- 15 files changed, 370 insertions(+), 67 deletions(-) rename lib/Activity/{Setting.php => FileToSign.php} (79%) create mode 100644 lib/Activity/Filter.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 1ecc7e69b8..24552dd543 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -43,8 +43,11 @@ - OCA\Libresign\Activity\Setting + OCA\Libresign\Activity\FileToSign + + OCA\Libresign\Activity\Filter + OCA\Libresign\Activity\Provider\SignRequest diff --git a/lib/Activity/Setting.php b/lib/Activity/FileToSign.php similarity index 79% rename from lib/Activity/Setting.php rename to lib/Activity/FileToSign.php index aa9ce80742..cda76c5bc0 100644 --- a/lib/Activity/Setting.php +++ b/lib/Activity/FileToSign.php @@ -24,18 +24,21 @@ namespace OCA\Libresign\Activity; -use OCA\Libresign\AppInfo\Application; use OCP\Activity\ActivitySettings; use OCP\IL10N; -class Setting extends ActivitySettings { +class FileToSign extends ActivitySettings { public function __construct( protected IL10N $l, ) { } + /** + * @return string Lowercase a-z and underscore only identifier. The type of table activity + * @since 20.0.0 + */ public function getIdentifier(): string { - return Application::APP_ID; + return 'file_to_sign'; } /** @@ -70,12 +73,24 @@ public function getPriority(): int { * {@inheritdoc} */ public function canChangeNotification(): bool { - return false; + return true; + } + /** + * {@inheritdoc} + */ + public function canChangeMail() { + return true; + } + /** + * {@inheritdoc} + */ + public function isDefaultEnabledMail() { + return true; } /** * {@inheritdoc} */ public function isDefaultEnabledNotification(): bool { - return false; + return $this->isDefaultEnabledMail() && $this->canChangeMail(); } } diff --git a/lib/Activity/Filter.php b/lib/Activity/Filter.php new file mode 100644 index 0000000000..ee39e70f99 --- /dev/null +++ b/lib/Activity/Filter.php @@ -0,0 +1,67 @@ + + * + * @author Vitor Mattos + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Libresign\Activity; + +use OCA\Libresign\AppInfo\Application; +use OCP\Activity\IFilter; +use OCP\IL10N; +use OCP\IURLGenerator; + +class Filter implements IFilter { + public function __construct( + protected IL10N $l, + protected IURLGenerator $url, + ) { + $this->l = $l; + $this->url = $url; + } + + public function getIdentifier() { + return Application::APP_ID; + } + + public function getName() { + return 'LibreSign'; + } + + public function getPriority() { + return 31; + } + + public function getIcon() { + return $this->url->getAbsoluteURL($this->url->imagePath('libresign', 'app-dark.svg')); + } + + public function filterTypes(array $types) { + return ['file_to_sign']; + } + + public function allowedApps() { + return [ + 'file_to_sign', + Application::APP_ID, + ]; + } +} diff --git a/lib/Activity/Listener.php b/lib/Activity/Listener.php index 9a11a1a8d5..1c35d6b2e0 100644 --- a/lib/Activity/Listener.php +++ b/lib/Activity/Listener.php @@ -25,13 +25,16 @@ namespace OCA\Libresign\Activity; use OCA\Libresign\AppInfo\Application; +use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Events\SendSignNotificationEvent; +use OCA\Libresign\Service\AccountService; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; use OCP\Activity\IManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; use Psr\Log\LoggerInterface; @@ -45,13 +48,17 @@ public function __construct( protected IUserSession $userSession, protected LoggerInterface $logger, protected ITimeFactory $timeFactory, + protected AccountService $accountService, + protected IURLGenerator $url, ) { } public function handle(Event $event): void { + /** @var SendSignNotificationEvent $event */ match (get_class($event)) { SendSignNotificationEvent::class => $this->generateNewSignNotificationActivity( $event->getSignRequest(), + $event->getLibreSignFile(), $event->getIdentifyMethod(), $event->isNew() ), @@ -66,6 +73,7 @@ public function handle(Event $event): void { */ protected function generateNewSignNotificationActivity( SignRequest $signRequest, + FileEntity $libreSignFile, IIdentifyMethod $identifyMethod, bool $isNew ): void { @@ -79,28 +87,52 @@ protected function generateNewSignNotificationActivity( try { $event ->setApp(Application::APP_ID) - ->setType(Application::APP_ID) + ->setType('file_to_sign') ->setAuthor($actorId) ->setObject('sign', $signRequest->getId(), 'signRequest') ->setTimestamp($this->timeFactory->getTime()) ->setAffectedUser($identifyMethod->getEntity()->getIdentifierValue()); if ($isNew) { - $event->setSubject('new_sign_request', [ - 'from' => $actor->getUID(), - 'signer' => $identifyMethod->getEntity()->getIdentifierValue(), - 'signRequest' => $signRequest->getId(), - ]); + $subject = 'new_sign_request'; } else { - $event->setSubject('update_sign_request', [ - 'from' => $actor->getUID(), - 'signer' => $identifyMethod->getEntity()->getIdentifierValue(), - 'signRequest' => $signRequest->getId(), - ]); + $subject = 'update_sign_request'; } + $event->setSubject($subject, [ + 'from' => $this->getUserParameter( + $actor->getUID(), + $actor->getDisplayName(), + ), + 'file' => $this->getFileParameter($signRequest, $libreSignFile), + 'signer' => $this->getUserParameter( + $identifyMethod->getEntity()->getIdentifierValue(), + $signRequest->getDisplayName(), + ), + ]); $this->activityManager->publish($event); } catch (\InvalidArgumentException $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return; } } + + protected function getFileParameter(SignRequest $signRequest, FileEntity $libreSignFile) { + return [ + 'type' => 'file', + 'id' => $libreSignFile->getNodeId(), + 'name' => $libreSignFile->getName(), + 'path' => $libreSignFile->getName(), + 'link' => $this->url->linkToRouteAbsolute('libresign.page.sign', ['uuid' => $signRequest->getUuid()]), + ]; + } + + protected function getUserParameter( + string $userId, + $displayName, + ): array { + return [ + 'type' => 'user', + 'id' => $userId, + 'name' => $displayName, + ]; + } } diff --git a/lib/Activity/Provider/SignRequest.php b/lib/Activity/Provider/SignRequest.php index 306b9c687e..5915c5bd0e 100644 --- a/lib/Activity/Provider/SignRequest.php +++ b/lib/Activity/Provider/SignRequest.php @@ -52,27 +52,31 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null): $event->setIcon($this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'app-dark.svg'))); } - if ($event->getSubject() === 'new_sign_request') { + if (in_array($event->getSubject(), ['new_sign_request', 'update_sign_request'])) { $l = $this->languageFactory->get(Application::APP_ID, $language); $parameters = $event->getSubjectParameters(); - $subject = $l->t('{from} invited you to sign a file'); + $subject = $this->getParsedSubject($l, $event->getSubject()); $event->setParsedSubject( str_replace( - '{from}', - $this->userManager->getDisplayName($parameters['from']) ?? $parameters['from'], + ['{from}', '{file}'], + [ + $parameters['from']['name'], + $parameters['file']['name'], + ], $subject - )); + )) + ->setRichSubject($subject, $parameters); } return $event; } - protected function getUser(string $uid): array { - return [ - 'type' => 'user', - 'id' => $uid, - 'name' => $this->userManager->getDisplayName($uid) ?? $uid, - ]; + private function getParsedSubject($l, $subject) { + if ($subject === 'new_sign_request') { + return $l->t('{from} invited you to sign {file}'); + } elseif ($subject === 'update_sign_request') { + return $l->t('{from} made changes on {file}'); + } } } diff --git a/lib/Command/Developer/Reset.php b/lib/Command/Developer/Reset.php index 054e51932f..06658985f3 100644 --- a/lib/Command/Developer/Reset.php +++ b/lib/Command/Developer/Reset.php @@ -180,6 +180,14 @@ private function resetActivity(string $user): void { $delete->andWhere($delete->expr()->eq('amq_affecteduser', $delete->createNamedParameter($user))); } $delete->executeStatement(); + + $delete = $this->db->getQueryBuilder(); + $delete->delete('activity') + ->where($delete->expr()->eq('app', $delete->createNamedParameter(Application::APP_ID))); + if ($user) { + $delete->andWhere($delete->expr()->eq('user', $delete->createNamedParameter($user))); + } + $delete->executeStatement(); } catch (\Throwable $e) { } } diff --git a/lib/Events/SendSignNotificationEvent.php b/lib/Events/SendSignNotificationEvent.php index 54e9ad7bb9..c3dcfc437c 100644 --- a/lib/Events/SendSignNotificationEvent.php +++ b/lib/Events/SendSignNotificationEvent.php @@ -24,6 +24,7 @@ namespace OCA\Libresign\Events; +use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; use OCP\EventDispatcher\Event; @@ -31,11 +32,16 @@ class SendSignNotificationEvent extends Event { public function __construct( private SignRequest $signRequest, + private FileEntity $libreSignFile, private IIdentifyMethod $identifyMethod, private bool $isNew ) { } + public function getLibreSignFile(): FileEntity { + return $this->libreSignFile; + } + public function getSignRequest(): SignRequest { return $this->signRequest; } diff --git a/lib/Listener/NotificationListener.php b/lib/Listener/NotificationListener.php index 5b6df5fd8e..4df74d0df4 100644 --- a/lib/Listener/NotificationListener.php +++ b/lib/Listener/NotificationListener.php @@ -25,12 +25,14 @@ namespace OCA\Libresign\Listener; use OCA\Libresign\AppInfo\Application as AppInfoApplication; +use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Events\SendSignNotificationEvent; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\IURLGenerator; use OCP\Notification\IManager; /** @@ -39,7 +41,8 @@ class NotificationListener implements IEventListener { public function __construct( private IManager $notificationManager, - private ITimeFactory $timeFactory + private ITimeFactory $timeFactory, + protected IURLGenerator $url, ) { } @@ -47,6 +50,7 @@ public function handle(Event $event): void { if ($event instanceof SendSignNotificationEvent) { $this->sendNewSignNotification( $event->getSignRequest(), + $event->getLibreSignFile(), $event->getIdentifyMethod(), $event->isNew() ); @@ -55,6 +59,7 @@ public function handle(Event $event): void { private function sendNewSignNotification( SignRequest $signRequest, + FileEntity $libreSignFile, IIdentifyMethod $identifyMethod, bool $isNew ): void { @@ -66,13 +71,23 @@ private function sendNewSignNotification( ->setUser($identifyMethod->getEntity()->getIdentifierValue()); if ($isNew) { $notification->setSubject('new_sign_request', [ - 'signRequest' => $signRequest->getId(), + 'file' => $this->getFileParameter($signRequest, $libreSignFile), ]); } else { $notification->setSubject('update_sign_request', [ - 'signRequest' => $signRequest->getId(), + 'file' => $this->getFileParameter($signRequest, $libreSignFile), ]); } $this->notificationManager->notify($notification); } + + protected function getFileParameter(SignRequest $signRequest, FileEntity $libreSignFile) { + return [ + 'type' => 'file', + 'id' => $libreSignFile->getNodeId(), + 'name' => $libreSignFile->getName(), + 'path' => $libreSignFile->getName(), + 'link' => $this->url->linkToRouteAbsolute('libresign.page.sign', ['uuid' => $signRequest->getUuid()]), + ]; + } } diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 08c63b1d83..37b21a1cf0 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -27,7 +27,6 @@ use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\SignRequestMapper; -use OCP\AppFramework\Db\DoesNotExistException; use OCP\IL10N; use OCP\IURLGenerator; use OCP\L10N\IFactory; @@ -75,14 +74,9 @@ private function parseSignRequest( bool $update ): INotification { $parameters = $notification->getSubjectParameters(); - try { - $signRequest = $this->signRequestMapper->getById($parameters['signRequest']); - } catch (DoesNotExistException $th) { - throw new \InvalidArgumentException(); - } $notification ->setIcon($this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg'))) - ->setLink($this->urlGenerator->linkToRouteAbsolute('libresign.page.sign', ['uuid' => $signRequest->getUuid()])); + ->setLink($parameters['file']['link']); $notification->setParsedSubject($l->t('There is a file for you to sign')); if ($update) { $notification->setParsedMessage($l->t('Changes have been made in a file that you have to sign.')); @@ -92,7 +86,7 @@ private function parseSignRequest( ->setParsedLabel($l->t('View')) ->setPrimary(true) ->setLink( - $this->urlGenerator->linkToRouteAbsolute('libresign.page.sign', ['uuid' => $signRequest->getUuid()]), + $parameters['file']['link'], IAction::TYPE_WEB ); $notification->addParsedAction($signAction); diff --git a/lib/Service/IdentifyMethod/Account.php b/lib/Service/IdentifyMethod/Account.php index 54074d5fbb..1918742e44 100644 --- a/lib/Service/IdentifyMethod/Account.php +++ b/lib/Service/IdentifyMethod/Account.php @@ -78,8 +78,10 @@ public function notify(bool $isNew): void { return; } $signRequest = $this->identifyMethodService->getSignRequestMapper()->getById($this->getEntity()->getSignRequestId()); + $libresignFile = $this->identifyMethodService->getFileMapper()->getById($signRequest->getFileId()); $this->eventDispatcher->dispatchTyped(new SendSignNotificationEvent( $signRequest, + $libresignFile, $this, $isNew )); diff --git a/lib/Service/RequestSignatureService.php b/lib/Service/RequestSignatureService.php index 27d9e0cf69..7ccb0edb41 100644 --- a/lib/Service/RequestSignatureService.php +++ b/lib/Service/RequestSignatureService.php @@ -192,17 +192,21 @@ private function associateToSigners(array $data, int $fileId): array { if (isset($user['identifyMethods'])) { foreach ($user['identifyMethods'] as $identifyMethod) { $return[] = $this->associateToSigner( - data: [ + identifyMethods: [ $identifyMethod['method'] => $identifyMethod['value'], ], - user: $user, + displayName: $user['displayName'] ?? '', + description: $user['description'] ?? '', + notify: $user['notify'] ?? true, fileId: $fileId, ); } } else { $return[] = $this->associateToSigner( - data: $user['identify'], - user: $user, + identifyMethods: $user['identify'], + displayName: $user['displayName'] ?? '', + description: $user['description'] ?? '', + notify: $user['notify'] ?? true, fileId: $fileId, ); } @@ -211,22 +215,45 @@ private function associateToSigners(array $data, int $fileId): array { return $return; } - private function associateToSigner(array $data, array $user, int $fileId): SignRequestEntity { - $identifyMethods = $this->identifyMethod->getByUserData($data); + private function associateToSigner(array $identifyMethods, string $displayName, string $description, bool $notify, int $fileId): SignRequestEntity { + $identifyMethodsIncances = $this->identifyMethod->getByUserData($identifyMethods); $signRequest = $this->getSignRequestByIdentifyMethod( - current($identifyMethods), + current($identifyMethodsIncances), $fileId ); - $this->setDataToUser($signRequest, $user, $fileId); + $displayName = $this->getDisplayNameFromIdentifyMethodIfEmpty($identifyMethodsIncances, $displayName); + $this->setDataToUser($signRequest, $displayName, $description, $fileId); $this->saveSignRequest($signRequest); - foreach ($identifyMethods as $identifyMethod) { + foreach ($identifyMethodsIncances as $identifyMethod) { $identifyMethod->getEntity()->setSignRequestId($signRequest->getId()); - $identifyMethod->willNotifyUser($user['notify'] ?? true); + $identifyMethod->willNotifyUser($notify); $identifyMethod->save(); } return $signRequest; } + /** + * @param IIdentifyMethod[] $identifyMethodsIncances + * @param string $displayName + * @return string + */ + private function getDisplayNameFromIdentifyMethodIfEmpty(array $identifyMethodsIncances, string $displayName): string { + if (!empty($displayName)) { + return $displayName; + } + foreach ($identifyMethodsIncances as $identifyMethod) { + if ($identifyMethod->getName() === 'account') { + return $this->userManager->get($identifyMethod->getEntity()->getIdentifierValue())->getDisplayName(); + } + } + foreach ($identifyMethodsIncances as $identifyMethod) { + if ($identifyMethod->getName() !== 'account') { + return $identifyMethod->getEntity()->getIdentifierValue(); + } + } + return ''; + } + private function saveVisibleElements(array $data, FileEntity $file): array { if (empty($data['visibleElements'])) { return []; @@ -279,16 +306,16 @@ public function saveSignRequest(SignRequestEntity $signRequest): void { /** * @psalm-suppress MixedMethodCall */ - private function setDataToUser(SignRequestEntity $signRequest, array $user, int $fileId): void { + private function setDataToUser(SignRequestEntity $signRequest, string $displayName, string $description, int $fileId): void { $signRequest->setFileId($fileId); if (!$signRequest->getUuid()) { $signRequest->setUuid(UUIDUtil::getUUID()); } - if (!empty($user['displayName'])) { - $signRequest->setDisplayName($user['displayName']); + if (!empty($displayName)) { + $signRequest->setDisplayName($displayName); } - if (!empty($user['description'])) { - $signRequest->setDescription($user['description']); + if (!empty($description)) { + $signRequest->setDescription($description); } if (!$signRequest->getId()) { $signRequest->setCreatedAt(time()); diff --git a/tests/integration/composer.json b/tests/integration/composer.json index 406103af1a..6145956dc9 100644 --- a/tests/integration/composer.json +++ b/tests/integration/composer.json @@ -9,7 +9,7 @@ "rpkamp/mailhog-behat-extension": "^1.0" }, "require-dev": { - "libresign/nextcloud-behat": "^0.10.1" + "libresign/nextcloud-behat": "^0.11.0" }, "config": { "allow-plugins": { diff --git a/tests/integration/composer.lock b/tests/integration/composer.lock index 4582a17e9b..481740a883 100644 --- a/tests/integration/composer.lock +++ b/tests/integration/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "609c40154da7c932ec432cd993e54dca", + "content-hash": "50bb633009c1c1b485543b2241cf7756", "packages": [ { "name": "behat/behat", @@ -5086,6 +5086,57 @@ } ], "packages-dev": [ + { + "name": "estahn/json-query-wrapper", + "version": "v1.0.0", + "source": { + "type": "git", + "url": "https://github.com/estahn/json-query-wrapper.git", + "reference": "79516b22ba364db552fc6dca719fcedc57ca5862" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/estahn/json-query-wrapper/zipball/79516b22ba364db552fc6dca719fcedc57ca5862", + "reference": "79516b22ba364db552fc6dca719fcedc57ca5862", + "shasum": "" + }, + "require": { + "php": "^7.1|^8", + "symfony/process": "^4|^5" + }, + "require-dev": { + "phpunit/phpunit": "^7|^8|^9", + "vimeo/psalm": "4.x-dev" + }, + "type": "library", + "autoload": { + "psr-4": { + "JsonQueryWrapper\\": "src/JsonQueryWrapper" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Enrico Stahn", + "email": "enrico.stahn@gmail.com" + } + ], + "description": "Wrapper for jq, a lightweight and flexible command-line JSON processor", + "homepage": "https://github.com/estahn/json-query-wrapper", + "keywords": [ + "jq", + "json", + "query" + ], + "support": { + "issues": "https://github.com/estahn/json-query-wrapper/issues", + "source": "https://github.com/estahn/json-query-wrapper/tree/v1.0.0" + }, + "time": "2021-01-11T23:19:55+00:00" + }, { "name": "libresign/behat-builtin-extension", "version": "v0.6.1", @@ -5134,20 +5185,21 @@ }, { "name": "libresign/nextcloud-behat", - "version": "v0.10.1", + "version": "v0.11.0", "source": { "type": "git", "url": "https://github.com/LibreSign/nextcloud-behat.git", - "reference": "232409b762fb3ca4d896253bab254422feaebc8a" + "reference": "e1e90894119ffee9d9ca51bb11a429f76dace6d9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/LibreSign/nextcloud-behat/zipball/232409b762fb3ca4d896253bab254422feaebc8a", - "reference": "232409b762fb3ca4d896253bab254422feaebc8a", + "url": "https://api.github.com/repos/LibreSign/nextcloud-behat/zipball/e1e90894119ffee9d9ca51bb11a429f76dace6d9", + "reference": "e1e90894119ffee9d9ca51bb11a429f76dace6d9", "shasum": "" }, "require": { "behat/behat": "^3.13", + "estahn/json-query-wrapper": "*", "guzzlehttp/guzzle": "^7.8", "libresign/behat-builtin-extension": "^0.6.1", "php": ">=8.0", @@ -5187,9 +5239,71 @@ ], "support": { "issues": "https://github.com/LibreSign/nextcloud-behat/issues", - "source": "https://github.com/LibreSign/nextcloud-behat/tree/v0.10.1" + "source": "https://github.com/LibreSign/nextcloud-behat/tree/v0.11.0" + }, + "time": "2024-03-09T19:41:46+00:00" + }, + { + "name": "symfony/process", + "version": "v5.4.36", + "source": { + "type": "git", + "url": "https://github.com/symfony/process.git", + "reference": "4fdf34004f149cc20b2f51d7d119aa500caad975" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/process/zipball/4fdf34004f149cc20b2f51d7d119aa500caad975", + "reference": "4fdf34004f149cc20b2f51d7d119aa500caad975", + "shasum": "" }, - "time": "2024-03-08T17:31:57+00:00" + "require": { + "php": ">=7.2.5", + "symfony/polyfill-php80": "^1.16" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Process\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Executes commands in sub-processes", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/process/tree/v5.4.36" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2024-02-12T15:49:53+00:00" } ], "aliases": [], diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 55e7f92d1d..1e790bfda8 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -306,6 +306,9 @@ public function userNotifications(string $user, TableNode $body = null): void { ); $jsonBody = json_decode($this->response->getBody()->getContents(), true); + if ($this->response->getStatusCode() === 500) { + throw new Exception('Internal failure when access notifications endpoint'); + } $data = $jsonBody['ocs']['data']; if ($body === null) { diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 3f45db2c69..f3bfaf0f7a 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -1,9 +1,7 @@ Feature: request-signature - Background: Create users - Given user "signer1" exists - Scenario: Get error when try to request to sign isn't manager - Given as user "signer1" + Given user "signer1" exists + And as user "signer1" And run the command "libresign:configure:openssl --cn test" When sending "post" to ocs "/apps/libresign/api/v1/request-signature" | file | {"base64":""} | @@ -29,6 +27,7 @@ Feature: request-signature Scenario: Request to sign with error using different authenticated account Given as user "admin" + And user "signer1" exists And run the command "libresign:configure:openssl --cn test" And set the email of user "signer1" to "signer1@domain.test" And reset notifications of user "signer1" @@ -50,6 +49,7 @@ Feature: request-signature Scenario: Request to sign with error when the user is not authenticated Given as user "admin" + And user "signer1" exists And run the command "libresign:configure:openssl --cn test" And reset notifications of user "signer1" And my inbox is empty @@ -70,6 +70,7 @@ Feature: request-signature Scenario: Request to sign with error when the authenticated user have an email different of signer Given as user "admin" + And user "signer1" exists And run the command "libresign:configure:openssl --cn test" And reset notifications of user "signer1" And set the email of user "signer1" to "signer1@domain.test" @@ -181,12 +182,12 @@ Feature: request-signature Scenario: Request to sign with success using account as identifier Given as user "admin" + And user "signer1" exists And run the command "libresign:developer:reset --all" And run the command "libresign:configure:openssl --cn test" And set the email of user "signer1" to "signer1@domain.test" And reset notifications of user "signer1" And my inbox is empty - And run the command "user:setting signer1 activity notify_email_libresign 1" When sending "post" to ocs "/apps/libresign/api/v1/request-signature" | file | {"url":"/apps/libresign/develop/pdf"} | | users | [{"identify":{"account":"signer1"}}] | @@ -195,6 +196,15 @@ Feature: request-signature And user signer1 has the following notifications | app | object_type | object_id | subject | | libresign | sign | document | There is a file for you to sign | + And sending "get" to ocs "/apps/activity/api/v2/activity/libresign?since=0" + Then the response should be a JSON array with the following mandatory values + | key | value | + | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + # Email is sent asyncronous by queue to an external application, we need to wait a bit + And wait for 2 second + And run the command "activity:send-mails" + And there should be 1 emails in my inbox + And I open the latest email to "signer1@domain.test" with subject "Activity at Nextcloud" and body "admin invited you to sign Date: Sat, 9 Mar 2024 19:38:59 -0300 Subject: [PATCH 03/11] Fix integration tests after change notification text Signed-off-by: Vitor Mattos --- .../features/bootstrap/FeatureContext.php | 4 ++-- .../page/sign_identify_account.feature | 22 ++++++++++--------- .../integration/features/sign/request.feature | 16 +++++++++----- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 1e790bfda8..ad82583336 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -353,11 +353,11 @@ public function iFetchTheSignerUuidFromNotification(): void { $found = array_filter( $data, function ($notification) { - return $notification['subject'] === 'There is a file for you to sign'; + return $notification['subject'] === 'admin invited you to sign document'; } ); if (empty($found)) { - throw new Exception('Notification with the subject [There is a file for you to sign] not found'); + throw new Exception('Notification with the subject [admin invited you to sign document] not found'); } $found = current($found); diff --git a/tests/integration/features/page/sign_identify_account.feature b/tests/integration/features/page/sign_identify_account.feature index fc22cf93dd..fc7a62e2e3 100644 --- a/tests/integration/features/page/sign_identify_account.feature +++ b/tests/integration/features/page/sign_identify_account.feature @@ -13,11 +13,12 @@ Feature: page/sign_identify_account | users | [{"identify":{"account":"signer1"}}] | | name | document | And the response should have a status code 200 - And user signer1 has the following notifications - | app | object_type | object_id | subject | - | libresign | sign | document | There is a file for you to sign | - And as user "signer1" - And sending "get" to ocs "/apps/libresign/api/v1/file/list" + When as user "signer1" + And sending "get" to ocs "/apps/notifications/api/v2/notifications" + Then the response should be a JSON array with the following mandatory values + | key | value | + | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + When sending "get" to ocs "/apps/libresign/api/v1/file/list" And the response should have a status code 200 And the file to sign contains | key | value | @@ -66,11 +67,12 @@ Feature: page/sign_identify_account | users | [{"identify":{"account":"signer1"}}] | | name | document | And the response should have a status code 200 - And user signer1 has the following notifications - | app | object_type | object_id | subject | - | libresign | sign | document | There is a file for you to sign | - And as user "signer1" - And sending "get" to ocs "/apps/libresign/api/v1/file/list" + When as user "signer1" + And sending "get" to ocs "/apps/notifications/api/v2/notifications" + Then the response should be a JSON array with the following mandatory values + | key | value | + | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + When sending "get" to ocs "/apps/libresign/api/v1/file/list" And the response should have a status code 200 And the file to sign contains | key | value | diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 4ee8d7468a..1e23fcaceb 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -257,9 +257,11 @@ Feature: request-signature | users | [{"identify":{"account":"signer1"}}] | | name | document | Then the response should have a status code 200 - And user signer1 has the following notifications - | app | object_type | object_id | subject | - | libresign | sign | document | There is a file for you to sign | + When as user "signer1" + And sending "get" to ocs "/apps/notifications/api/v2/notifications" + Then the response should be a JSON array with the following mandatory values + | key | value | + | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| And there should be 0 emails in my inbox Scenario: Request to sign with success using email as identifier @@ -313,9 +315,11 @@ Feature: request-signature | users | [{"identify":{"email":"signer1@domain.test"}},{"identify":{"account":"signer1"}}] | | name | document | Then the response should have a status code 200 - And user signer1 has the following notifications - | app | object_type | object_id | subject | - | libresign | sign | document | There is a file for you to sign | + When as user "signer1" + And sending "get" to ocs "/apps/notifications/api/v2/notifications" + Then the response should be a JSON array with the following mandatory values + | key | value | + | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| And there should be 1 emails in my inbox And I open the latest email to "signer1@domain.test" with subject "LibreSign: There is a file for you to sign" From 092a05d56403b0dd6b1ab887fe5ca19f0016cda2 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 19:45:55 -0300 Subject: [PATCH 04/11] Remove steps that don't work at GitHub Actions Signed-off-by: Vitor Mattos --- tests/integration/features/sign/request.feature | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 1e23fcaceb..0b07276795 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -202,11 +202,6 @@ Feature: request-signature Then the response should be a JSON array with the following mandatory values | key | value | | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| - # Email is sent asyncronous by queue to an external application, we need to wait a bit - And wait for 2 second - And run the command "activity:send-mails" - And there should be 1 emails in my inbox - And I open the latest email to "signer1@domain.test" with subject "Activity at Nextcloud" and body "admin invited you to sign Date: Sat, 9 Mar 2024 20:10:54 -0300 Subject: [PATCH 05/11] Add pending definition Signed-off-by: Vitor Mattos --- lib/Activity/Listener.php | 1 + lib/Activity/Provider/SignRequest.php | 10 ++++++++-- lib/Listener/NotificationListener.php | 1 + lib/Notification/Notifier.php | 10 ++++++++-- tests/integration/features/sign/request.feature | 6 +++--- 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/Activity/Listener.php b/lib/Activity/Listener.php index 9a6f4092ad..0d233a571e 100644 --- a/lib/Activity/Listener.php +++ b/lib/Activity/Listener.php @@ -110,6 +110,7 @@ protected function generateNewSignNotificationActivity( 'signRequest' => [ 'type' => 'sign-request', 'id' => $signRequest->getId(), + 'name' => $actor->getDisplayName(), ], ]); $this->activityManager->publish($event); diff --git a/lib/Activity/Provider/SignRequest.php b/lib/Activity/Provider/SignRequest.php index b2c52dc85f..8657574a1f 100644 --- a/lib/Activity/Provider/SignRequest.php +++ b/lib/Activity/Provider/SignRequest.php @@ -57,8 +57,14 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null): 'required' => true, 'description' => 'The id of SignRequest object', 'example' => '12345', - ] - ] + ], + 'name' => [ + 'since' => '28.0.0', + 'required' => true, + 'description' => 'The display name of signer', + 'example' => 'John Doe', + ], + ], ]; if ($this->activityManager->getRequirePNG()) { diff --git a/lib/Listener/NotificationListener.php b/lib/Listener/NotificationListener.php index f59d7c1361..8144508ac0 100644 --- a/lib/Listener/NotificationListener.php +++ b/lib/Listener/NotificationListener.php @@ -90,6 +90,7 @@ private function sendNewSignNotification( 'signRequest' => [ 'type' => 'sign-request', 'id' => $signRequest->getId(), + 'name' => $actor->getDisplayName(), ], ]); $this->notificationManager->notify($notification); diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index d94529cefa..acb0cfe632 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -67,8 +67,14 @@ public function prepare(INotification $notification, string $languageCode): INot 'required' => true, 'description' => 'The id of SignRequest object', 'example' => '12345', - ] - ] + ], + 'name' => [ + 'since' => '28.0.0', + 'required' => true, + 'description' => 'The display name of signer', + 'example' => 'John Doe', + ], + ], ]; $l = $this->factory->get(Application::APP_ID, $languageCode); diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 0b07276795..d57c36603d 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -194,11 +194,11 @@ Feature: request-signature | name | document | Then the response should have a status code 200 When as user "signer1" - And sending "get" to ocs "/apps/notifications/api/v2/notifications" - Then the response should be a JSON array with the following mandatory values + Then sending "get" to ocs "/apps/notifications/api/v2/notifications" + And the response should be a JSON array with the following mandatory values | key | value | | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| - And sending "get" to ocs "/apps/activity/api/v2/activity/libresign?since=0" + When sending "get" to ocs "/apps/activity/api/v2/activity/libresign?since=0" Then the response should be a JSON array with the following mandatory values | key | value | | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| From 116fce15fc669e6a5c192c8d2ac429c5b0b62a54 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 21:27:48 -0300 Subject: [PATCH 06/11] Respect Activity settings app to send notification Signed-off-by: Vitor Mattos --- lib/Listener/NotificationListener.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/Listener/NotificationListener.php b/lib/Listener/NotificationListener.php index 8144508ac0..f601adcfcb 100644 --- a/lib/Listener/NotificationListener.php +++ b/lib/Listener/NotificationListener.php @@ -70,6 +70,9 @@ private function sendNewSignNotification( if (!$actor instanceof IUser) { return; } + if ($this->isNotificationDisabledAtActivity($identifyMethod)) { + return; + } $notification = $this->notificationManager->createNotification(); $notification ->setApp(AppInfoApplication::APP_ID) @@ -96,6 +99,21 @@ private function sendNewSignNotification( $this->notificationManager->notify($notification); } + public function isNotificationDisabledAtActivity(IIdentifyMethod $identifyMethod): bool { + $activityUserSettings = \OC::$server->get(\OCA\Activity\UserSettings::class); + if ($activityUserSettings) { + $notificationSetting = $activityUserSettings->getUserSetting( + $identifyMethod->getEntity()->getIdentifierValue(), + 'notification', + 'file_to_sign' + ); + if (!$notificationSetting) { + return true; + } + } + return false; + } + protected function getFileParameter(SignRequest $signRequest, FileEntity $libreSignFile) { return [ 'type' => 'file', From d1f6c3111cf65dd58fd56b29ddf617829d3d0446 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 22:50:35 -0300 Subject: [PATCH 07/11] Moved notify by email to event Signed-off-by: Vitor Mattos --- lib/Activity/FileToSign.php | 2 +- lib/Activity/Listener.php | 6 +- lib/AppInfo/Application.php | 4 + lib/Listener/MailNotifyListener.php | 94 +++++++++++++++++++ .../IdentifyMethod/AbstractIdentifyMethod.php | 15 ++- lib/Service/IdentifyMethod/Account.php | 15 --- lib/Service/IdentifyMethod/Email.php | 14 --- .../IdentifyMethod/IIdentifyMethod.php | 2 +- .../IdentifyMethod/IdentifyMethodService.php | 6 ++ .../integration/features/sign/request.feature | 2 + tests/psalm-baseline.xml | 3 +- 11 files changed, 129 insertions(+), 34 deletions(-) create mode 100644 lib/Listener/MailNotifyListener.php diff --git a/lib/Activity/FileToSign.php b/lib/Activity/FileToSign.php index c675689bd5..b1cac606e9 100644 --- a/lib/Activity/FileToSign.php +++ b/lib/Activity/FileToSign.php @@ -91,6 +91,6 @@ public function isDefaultEnabledMail() { * {@inheritdoc} */ public function isDefaultEnabledNotification(): bool { - return false; + return true; } } diff --git a/lib/Activity/Listener.php b/lib/Activity/Listener.php index 0d233a571e..d842d1b8de 100644 --- a/lib/Activity/Listener.php +++ b/lib/Activity/Listener.php @@ -91,7 +91,11 @@ protected function generateNewSignNotificationActivity( ->setAuthor($actorId) ->setObject('signRequest', $signRequest->getId()) ->setTimestamp($this->timeFactory->getTime()) - ->setAffectedUser($identifyMethod->getEntity()->getIdentifierValue()); + ->setAffectedUser($identifyMethod->getEntity()->getIdentifierValue()) + // Activity notification was replaced by Notification app + // At notification app we can define the view and dismiss action + // Activity dont have this feature + ->setGenerateNotification(false); if ($isNew) { $subject = 'new_sign_request'; } else { diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 4657c2bad0..c40c9c4530 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -31,6 +31,7 @@ use OCA\Libresign\Files\TemplateLoader as FilesTemplateLoader; use OCA\Libresign\Listener\BeforeNodeDeletedListener; use OCA\Libresign\Listener\LoadSidebarListener; +use OCA\Libresign\Listener\MailNotifyListener; use OCA\Libresign\Listener\NotificationListener; use OCA\Libresign\Listener\SignedListener; use OCA\Libresign\Middleware\GlobalInjectionMiddleware; @@ -77,5 +78,8 @@ public function register(IRegistrationContext $context): void { // Notification listeners $context->registerEventListener(SendSignNotificationEvent::class, NotificationListener::class); + + // MailNotify listener + $context->registerEventListener(SendSignNotificationEvent::class, MailNotifyListener::class); } } diff --git a/lib/Listener/MailNotifyListener.php b/lib/Listener/MailNotifyListener.php new file mode 100644 index 0000000000..d8547272c4 --- /dev/null +++ b/lib/Listener/MailNotifyListener.php @@ -0,0 +1,94 @@ + + * + * @author Vitor Mattos + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Libresign\Listener; + +use OCA\Libresign\Db\File as FileEntity; +use OCA\Libresign\Db\SignRequest; +use OCA\Libresign\Events\SendSignNotificationEvent; +use OCA\Libresign\Service\IdentifyMethod\IdentifyMethodService; +use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; +use OCA\Libresign\Service\MailService; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\IUser; +use OCP\IUserManager; +use OCP\IUserSession; +use Psr\Log\LoggerInterface; + +class MailNotifyListener implements IEventListener { + public function __construct( + protected IUserSession $userSession, + protected IUserManager $userManager, + protected IdentifyMethodService $identifyMethodService, + protected MailService $mail, + private LoggerInterface $logger, + ) { + } + + public function handle(Event $event): void { + /** @var SendSignNotificationEvent $event */ + match (get_class($event)) { + SendSignNotificationEvent::class => $this->sendMailNotification( + $event->getSignRequest(), + $event->getLibreSignFile(), + $event->getIdentifyMethod(), + $event->isNew() + ), + }; + } + + protected function sendMailNotification( + SignRequest $signRequest, + FileEntity $libreSignFile, + IIdentifyMethod $identifyMethod, + bool $isNew, + ): void { + $actor = $this->userSession->getUser(); + if (!$actor instanceof IUser) { + return; + } + try { + $email = ''; + if ($identifyMethod->getName() === 'account') { + $email = $this->userManager + ->get($identifyMethod->getEntity()->getIdentifierValue()) + ->getEMailAddress(); + } elseif ($identifyMethod->getName() === 'email') { + $email = $identifyMethod->getEntity()->getIdentifierValue(); + } + if (empty($email)) { + return; + } + if ($isNew) { + $this->mail->notifyUnsignedUser($signRequest, $email); + return; + } + $this->mail->notifySignDataUpdated($signRequest, $email); + } catch (\InvalidArgumentException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + return; + } + } +} diff --git a/lib/Service/IdentifyMethod/AbstractIdentifyMethod.php b/lib/Service/IdentifyMethod/AbstractIdentifyMethod.php index f7ae385f1b..5e7a475f62 100644 --- a/lib/Service/IdentifyMethod/AbstractIdentifyMethod.php +++ b/lib/Service/IdentifyMethod/AbstractIdentifyMethod.php @@ -27,6 +27,7 @@ use InvalidArgumentException; use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\IdentifyMethod; +use OCA\Libresign\Events\SendSignNotificationEvent; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Helper\JSActions; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\AbstractSignatureMethod; @@ -102,7 +103,19 @@ public function getSettings(): array { return $this->settings; } - public function notify(bool $isNew): void { + public function notify(bool $isNew): bool { + if (!$this->willNotify) { + return false; + } + $signRequest = $this->identifyMethodService->getSignRequestMapper()->getById($this->getEntity()->getSignRequestId()); + $libresignFile = $this->identifyMethodService->getFileMapper()->getById($signRequest->getFileId()); + $this->identifyMethodService->getEventDispatcher()->dispatchTyped(new SendSignNotificationEvent( + $signRequest, + $libresignFile, + $this, + $isNew + )); + return true; } public function willNotifyUser(bool $willNotify): void { diff --git a/lib/Service/IdentifyMethod/Account.php b/lib/Service/IdentifyMethod/Account.php index 1918742e44..4d7b0c147b 100644 --- a/lib/Service/IdentifyMethod/Account.php +++ b/lib/Service/IdentifyMethod/Account.php @@ -25,7 +25,6 @@ namespace OCA\Libresign\Service\IdentifyMethod; use OCA\Libresign\Db\IdentifyMethodMapper; -use OCA\Libresign\Events\SendSignNotificationEvent; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Helper\JSActions; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ClickToSign; @@ -73,20 +72,6 @@ public function __construct( ); } - public function notify(bool $isNew): void { - if (!$this->willNotify) { - return; - } - $signRequest = $this->identifyMethodService->getSignRequestMapper()->getById($this->getEntity()->getSignRequestId()); - $libresignFile = $this->identifyMethodService->getFileMapper()->getById($signRequest->getFileId()); - $this->eventDispatcher->dispatchTyped(new SendSignNotificationEvent( - $signRequest, - $libresignFile, - $this, - $isNew - )); - } - public function validateToRequest(): void { $signer = $this->userManager->get($this->entity->getIdentifierValue()); if (!$signer) { diff --git a/lib/Service/IdentifyMethod/Email.php b/lib/Service/IdentifyMethod/Email.php index c4ca986ff2..93c859b858 100644 --- a/lib/Service/IdentifyMethod/Email.php +++ b/lib/Service/IdentifyMethod/Email.php @@ -30,7 +30,6 @@ use OCA\Libresign\Helper\JSActions; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ClickToSign; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\EmailToken; -use OCA\Libresign\Service\MailService; use OCA\Libresign\Service\SessionService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IRootFolder; @@ -40,7 +39,6 @@ class Email extends AbstractIdentifyMethod { public function __construct( protected IdentifyMethodService $identifyMethodService, - private MailService $mail, private IdentifyMethodMapper $identifyMethodMapper, private IRootFolder $root, private ITimeFactory $timeFactory, @@ -61,18 +59,6 @@ public function __construct( ); } - public function notify(bool $isNew): void { - if (!$this->willNotify) { - return; - } - $signRequest = $this->identifyMethodService->getSignRequestMapper()->getById($this->getEntity()->getSignRequestId()); - if ($isNew) { - $this->mail->notifyUnsignedUser($signRequest, $this->getEntity()->getIdentifierValue()); - return; - } - $this->mail->notifySignDataUpdated($signRequest, $this->getEntity()->getIdentifierValue()); - } - public function validateToRequest(): void { $this->throwIfInvalidEmail(); } diff --git a/lib/Service/IdentifyMethod/IIdentifyMethod.php b/lib/Service/IdentifyMethod/IIdentifyMethod.php index 9d1a3338a9..6d6fcba9f0 100644 --- a/lib/Service/IdentifyMethod/IIdentifyMethod.php +++ b/lib/Service/IdentifyMethod/IIdentifyMethod.php @@ -43,7 +43,7 @@ public function getSignatureMethods(): array; public function signatureMethodsToArray(): array; public function getSettings(): array; public function willNotifyUser(bool $willNotify): void; - public function notify(bool $isNew): void; + public function notify(bool $isNew): bool; public function validateToRequest(): void; public function validateToCreateAccount(string $value): void; public function validateToIdentify(): void; diff --git a/lib/Service/IdentifyMethod/IdentifyMethodService.php b/lib/Service/IdentifyMethod/IdentifyMethodService.php index 56dfe95096..0c4ca480eb 100644 --- a/lib/Service/IdentifyMethod/IdentifyMethodService.php +++ b/lib/Service/IdentifyMethod/IdentifyMethodService.php @@ -31,6 +31,7 @@ use OCA\Libresign\Service\SessionService; use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\Files\IRootFolder; use OCP\IL10N; @@ -45,6 +46,7 @@ public function __construct( private IdentifyMethodMapper $identifyMethodMapper, private SessionService $sessionService, private ITimeFactory $timeFactory, + private IEventDispatcher $eventDispatcher, private IRootFolder $root, private IAppConfig $appConfig, private SignRequestMapper $signRequestMapper, @@ -108,6 +110,10 @@ public function getSavedSettings(): array { return $this->savedSettings; } + public function getEventDispatcher(): IEventDispatcher { + return $this->eventDispatcher; + } + public function getSessionService(): SessionService { return $this->sessionService; } diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index d57c36603d..a3e751429b 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -193,6 +193,8 @@ Feature: request-signature | users | [{"identify":{"account":"signer1"}}] | | name | document | Then the response should have a status code 200 + And there should be 1 emails in my inbox + And I open the latest email to "signer1@domain.test" with subject "LibreSign: There is a file for you to sign" When as user "signer1" Then sending "get" to ocs "/apps/notifications/api/v2/notifications" And the response should be a JSON array with the following mandatory values diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 6609380357..bbedfb9ba6 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,7 +1,8 @@ - + + registerEventListener registerEventListener registerEventListener registerEventListener From 206c75fc8814108dd5211e372359205f574347c2 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 22:55:37 -0300 Subject: [PATCH 08/11] Check if class exists before get class Signed-off-by: Vitor Mattos --- lib/Listener/NotificationListener.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Listener/NotificationListener.php b/lib/Listener/NotificationListener.php index f601adcfcb..ef7695e28f 100644 --- a/lib/Listener/NotificationListener.php +++ b/lib/Listener/NotificationListener.php @@ -100,6 +100,9 @@ private function sendNewSignNotification( } public function isNotificationDisabledAtActivity(IIdentifyMethod $identifyMethod): bool { + if (!class_exists(\OCA\Activity\UserSettings::class)) { + return false; + } $activityUserSettings = \OC::$server->get(\OCA\Activity\UserSettings::class); if ($activityUserSettings) { $notificationSetting = $activityUserSettings->getUserSetting( From 33d2ff4ab6d59d213a9b4bb3f992cdadc979c1e2 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 23:07:25 -0300 Subject: [PATCH 09/11] Fix integration test Signed-off-by: Vitor Mattos --- tests/integration/features/sign/request.feature | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index a3e751429b..1f87f9008b 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -142,7 +142,8 @@ Feature: request-signature | action | 4500 | | title | Link expired | Given my inbox is empty - When sending "post" to ocs "/apps/libresign/api/v1/sign/uuid//renew/email" + When as user "admin" + And sending "post" to ocs "/apps/libresign/api/v1/sign/uuid//renew/email" Then the response should have a status code 200 And the response should be a JSON array with the following mandatory values | key | value | From f79a7cd0766ba0c41fc0f48bef7b8430840408d8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Sat, 9 Mar 2024 23:27:49 -0300 Subject: [PATCH 10/11] Remove problematic step Signed-off-by: Vitor Mattos --- tests/integration/features/sign/request.feature | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 1f87f9008b..7d9ca1a8b5 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -194,8 +194,6 @@ Feature: request-signature | users | [{"identify":{"account":"signer1"}}] | | name | document | Then the response should have a status code 200 - And there should be 1 emails in my inbox - And I open the latest email to "signer1@domain.test" with subject "LibreSign: There is a file for you to sign" When as user "signer1" Then sending "get" to ocs "/apps/notifications/api/v2/notifications" And the response should be a JSON array with the following mandatory values From 092ab26379baad749815ec3ca7aabe54317b2f3b Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Mon, 11 Mar 2024 09:57:49 -0300 Subject: [PATCH 11/11] Change text Signed-off-by: Vitor Mattos --- lib/Activity/Provider/SignRequest.php | 2 +- lib/Notification/Notifier.php | 2 +- tests/integration/features/bootstrap/FeatureContext.php | 4 ++-- .../features/page/sign_identify_account.feature | 4 ++-- tests/integration/features/sign/request.feature | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Activity/Provider/SignRequest.php b/lib/Activity/Provider/SignRequest.php index 8657574a1f..6ab49f15e4 100644 --- a/lib/Activity/Provider/SignRequest.php +++ b/lib/Activity/Provider/SignRequest.php @@ -95,7 +95,7 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null): private function getParsedSubject($l, $subject) { if ($subject === 'new_sign_request') { - return $l->t('{from} invited you to sign {file}'); + return $l->t('{from} requested your signature on {file}'); } elseif ($subject === 'update_sign_request') { return $l->t('{from} made changes on {file}'); } diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index acb0cfe632..a22f354ac9 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -98,7 +98,7 @@ private function parseSignRequest( $notification ->setIcon($this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'app-dark.svg'))) ->setLink($parameters['file']['link']); - $subject = $l->t('{from} invited you to sign {file}'); + $subject = $l->t('{from} requested your signature on {file}'); $notification->setParsedSubject( str_replace( ['{from}', '{file}'], diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index ad82583336..7993aad7c0 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -353,11 +353,11 @@ public function iFetchTheSignerUuidFromNotification(): void { $found = array_filter( $data, function ($notification) { - return $notification['subject'] === 'admin invited you to sign document'; + return $notification['subject'] === 'admin requested your signature on document'; } ); if (empty($found)) { - throw new Exception('Notification with the subject [admin invited you to sign document] not found'); + throw new Exception('Notification with the subject [admin requested your signature on document] not found'); } $found = current($found); diff --git a/tests/integration/features/page/sign_identify_account.feature b/tests/integration/features/page/sign_identify_account.feature index fc7a62e2e3..cb3b1ac044 100644 --- a/tests/integration/features/page/sign_identify_account.feature +++ b/tests/integration/features/page/sign_identify_account.feature @@ -17,7 +17,7 @@ Feature: page/sign_identify_account And sending "get" to ocs "/apps/notifications/api/v2/notifications" Then the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| When sending "get" to ocs "/apps/libresign/api/v1/file/list" And the response should have a status code 200 And the file to sign contains @@ -71,7 +71,7 @@ Feature: page/sign_identify_account And sending "get" to ocs "/apps/notifications/api/v2/notifications" Then the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| When sending "get" to ocs "/apps/libresign/api/v1/file/list" And the response should have a status code 200 And the file to sign contains diff --git a/tests/integration/features/sign/request.feature b/tests/integration/features/sign/request.feature index 7d9ca1a8b5..b202eade3f 100644 --- a/tests/integration/features/sign/request.feature +++ b/tests/integration/features/sign/request.feature @@ -198,11 +198,11 @@ Feature: request-signature Then sending "get" to ocs "/apps/notifications/api/v2/notifications" And the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| When sending "get" to ocs "/apps/activity/api/v2/activity/libresign?since=0" Then the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| Scenario: Request to sign with error using account as identifier with invalid email Given as user "admin" @@ -257,7 +257,7 @@ Feature: request-signature And sending "get" to ocs "/apps/notifications/api/v2/notifications" Then the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| And there should be 0 emails in my inbox Scenario: Request to sign with success using email as identifier @@ -315,7 +315,7 @@ Feature: request-signature And sending "get" to ocs "/apps/notifications/api/v2/notifications" Then the response should be a JSON array with the following mandatory values | key | value | - | ocs | (jq).data\|.[].subject == "admin invited you to sign document"| + | ocs | (jq).data\|.[].subject == "admin requested your signature on document"| And there should be 1 emails in my inbox And I open the latest email to "signer1@domain.test" with subject "LibreSign: There is a file for you to sign"