diff --git a/.github/workflows/phpunit-mysql.yml b/.github/workflows/phpunit-mysql.yml index c0f4c69b3..ac583a47c 100644 --- a/.github/workflows/phpunit-mysql.yml +++ b/.github/workflows/phpunit-mysql.yml @@ -105,6 +105,8 @@ jobs: extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql coverage: none ini-file: development + # Temporary workaround for missing pcntl_* in PHP 8.3 + ini-values: disable_functions= env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/phpunit-oci.yml b/.github/workflows/phpunit-oci.yml index d03beb9d1..0452ef5aa 100644 --- a/.github/workflows/phpunit-oci.yml +++ b/.github/workflows/phpunit-oci.yml @@ -118,6 +118,8 @@ jobs: extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, oci8 coverage: none ini-file: development + # Temporary workaround for missing pcntl_* in PHP 8.3 + ini-values: disable_functions= env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/phpunit-pgsql.yml b/.github/workflows/phpunit-pgsql.yml index 2a23e02e6..e0e45fbe6 100644 --- a/.github/workflows/phpunit-pgsql.yml +++ b/.github/workflows/phpunit-pgsql.yml @@ -108,6 +108,8 @@ jobs: extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql coverage: none ini-file: development + # Temporary workaround for missing pcntl_* in PHP 8.3 + ini-values: disable_functions= env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/phpunit-sqlite.yml b/.github/workflows/phpunit-sqlite.yml index be9e33243..2ebfcb23c 100644 --- a/.github/workflows/phpunit-sqlite.yml +++ b/.github/workflows/phpunit-sqlite.yml @@ -97,6 +97,8 @@ jobs: extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite coverage: none ini-file: development + # Temporary workaround for missing pcntl_* in PHP 8.3 + ini-values: disable_functions= env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/appinfo/info.xml b/appinfo/info.xml index 999043d0b..48197104c 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -44,6 +44,7 @@ + OCA\Notifications\Command\Delete OCA\Notifications\Command\Generate OCA\Notifications\Command\TestPush diff --git a/docs/admin-notifications.md b/docs/admin-notifications.md index 8e8e2d21c..1dfd98f7a 100644 --- a/docs/admin-notifications.md +++ b/docs/admin-notifications.md @@ -10,24 +10,39 @@ Allows admins to generate notifications for users via the console or an HTTP end ``` $ sudo -u www-data ./occ notification:generate \ - admin "Short message up to 255 characters" \ - -l "Optional: longer message with more details, up to 4000 characters" + 'admin' 'Short message up to 255 characters' \ + -l 'Optional: longer message with more details, up to 4000 characters' ``` ### Help +> [!TIP] +> Specify an object type and object id to delete previous notifications about +> the same thing,e.g. when the notification is about an update for "LibX" to +> version "12", use `--object-type='update' --object-id='libx'`, so that a later +> notification for version "13" can automatically dismiss the notification for +> version "12" if it was not removed in the meantime. + +> [!TIP] +> Specify the `--output-id-only` option and store it to later be able to delete +> the generated notification using the `notification:delete` command. + ``` $ sudo -u www-data ./occ notification:generate --help Usage: notification:generate [options] [--] Arguments: - user-id User ID of the user to notify - short-message Short message to be sent to the user (max. 255 characters) + user-id User ID of the user to notify + short-message Short message to be sent to the user (max. 255 characters) Options: - -l, --long-message=LONG-MESSAGE Long mesage to be sent to the user (max. 4000 characters) [default: ""] - + --short-parameters=SHORT-PARAMETERS JSON encoded array of Rich objects to fill the short-message, see https://github.com/nextcloud/server/blob/master/lib/public/RichObjectStrings/Definitions.php for more information + -l, --long-message=LONG-MESSAGE Long message to be sent to the user (max. 4000 characters) [default: ""] + --long-parameters=LONG-PARAMETERS JSON encoded array of Rich objects to fill the long-message, see https://github.com/nextcloud/server/blob/master/lib/public/RichObjectStrings/Definitions.php for more information + --object-type=OBJECT-TYPE If an object type and id is provided, previous notifications with the same type and id will be deleted for this user (max. 64 characters) + --object-id=OBJECT-ID If an object type and id is provided, previous notifications with the same type and id will be deleted for this user (max. 64 characters) + --output-id-only When specified only the notification ID that was generated will be printed in case of success ``` ## HTTP request diff --git a/lib/Command/Delete.php b/lib/Command/Delete.php new file mode 100644 index 000000000..038100440 --- /dev/null +++ b/lib/Command/Delete.php @@ -0,0 +1,66 @@ +setName('notification:delete') + ->setDescription('Delete a generated admin notification for the given user') + ->addArgument( + 'user-id', + InputArgument::REQUIRED, + 'User ID of the user to notify' + ) + ->addArgument( + 'notification-id', + InputArgument::REQUIRED, + 'The notification ID returned by the "notification:generate" command' + ) + ; + } + + /** + * @param InputInterface $input + * @param OutputInterface $output + * @return int + */ + protected function execute(InputInterface $input, OutputInterface $output): int { + + $userId = (string)$input->getArgument('user-id'); + $notificationId = (int)$input->getArgument('notification-id'); + + try { + $notification = $this->notificationHandler->getById($notificationId, $userId); + } catch (NotificationNotFoundException) { + $output->writeln('Notification not found for user'); + return 1; + } + + $this->notificationManager->markProcessed($notification); + $output->writeln('Notification deleted successfully'); + return 0; + } +} diff --git a/lib/Command/Generate.php b/lib/Command/Generate.php index f9cbfd2ab..dd04ab745 100644 --- a/lib/Command/Generate.php +++ b/lib/Command/Generate.php @@ -9,10 +9,12 @@ namespace OCA\Notifications\Command; +use OCA\Notifications\App; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager; +use OCP\RichObjectStrings\IValidator; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -20,23 +22,14 @@ use Symfony\Component\Console\Output\OutputInterface; class Generate extends Command { - /** @var ITimeFactory */ - protected $timeFactory; - - /** @var IUserManager */ - protected $userManager; - - /** @var IManager */ - protected $notificationManager; - - public function __construct(ITimeFactory $timeFactory, - IUserManager $userManager, - IManager $notificationManager) { + public function __construct( + protected ITimeFactory $timeFactory, + protected IUserManager $userManager, + protected IManager $notificationManager, + protected IValidator $richValidator, + protected App $notificationApp, + ) { parent::__construct(); - - $this->timeFactory = $timeFactory; - $this->userManager = $userManager; - $this->notificationManager = $notificationManager; } protected function configure(): void { @@ -53,19 +46,49 @@ protected function configure(): void { InputArgument::REQUIRED, 'Short message to be sent to the user (max. 255 characters)' ) + ->addOption( + 'short-parameters', + null, + InputOption::VALUE_REQUIRED, + 'JSON encoded array of Rich objects to fill the short-message, see https://github.com/nextcloud/server/blob/master/lib/public/RichObjectStrings/Definitions.php for more information', + ) ->addOption( 'long-message', 'l', InputOption::VALUE_REQUIRED, - 'Long mesage to be sent to the user (max. 4000 characters)', + 'Long message to be sent to the user (max. 4000 characters)', '' ) + ->addOption( + 'long-parameters', + null, + InputOption::VALUE_REQUIRED, + 'JSON encoded array of Rich objects to fill the long-message, see https://github.com/nextcloud/server/blob/master/lib/public/RichObjectStrings/Definitions.php for more information', + ) + ->addOption( + 'object-type', + null, + InputOption::VALUE_REQUIRED, + 'If an object type and id is provided, previous notifications with the same type and id will be deleted for this user (max. 64 characters)', + ) + ->addOption( + 'object-id', + null, + InputOption::VALUE_REQUIRED, + 'If an object type and id is provided, previous notifications with the same type and id will be deleted for this user (max. 64 characters)', + ) ->addOption( 'dummy', 'd', InputOption::VALUE_NONE, 'Create a full-flexed dummy notification for client debugging with actions and parameters (short-message will be casted to integer and is the number of actions (max 3))' ) + ->addOption( + 'output-id-only', + null, + InputOption::VALUE_NONE, + 'When specified only the notification ID that was generated will be printed in case of success' + ) ; } @@ -75,10 +98,16 @@ protected function configure(): void { * @return int */ protected function execute(InputInterface $input, OutputInterface $output): int { - $userId = $input->getArgument('user-id'); - $subject = $input->getArgument('short-message'); - $message = $input->getOption('long-message'); + + $userId = (string)$input->getArgument('user-id'); + $subject = (string)$input->getArgument('short-message'); + $subjectParametersString = (string)$input->getOption('short-parameters'); + $message = (string)$input->getOption('long-message'); + $messageParametersString = (string)$input->getOption('long-parameters'); $dummy = $input->getOption('dummy'); + $idOnly = $input->getOption('output-id-only'); + $objectType = $input->getOption('object-type'); + $objectId = $input->getOption('object-id'); $user = $this->userManager->get($userId); if (!$user instanceof IUser) { @@ -97,24 +126,63 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } + if ($subjectParametersString !== '') { + $subjectParameters = json_decode($subjectParametersString, true); + if (!is_array($subjectParameters)) { + $output->writeln('Short message parameters is not a valid json array'); + return 1; + } + $this->richValidator->validate($subject, $subjectParameters); + $storeSubjectParameters = ['subject' => $subject, 'parameters' => $subjectParameters]; + } else { + $storeSubjectParameters = [$subject]; + } + + if ($message !== '' && $messageParametersString !== '') { + $messageParameters = json_decode($messageParametersString, true); + if (!is_array($messageParameters)) { + $output->writeln('Long message parameters is not a valid json array'); + return 1; + } + $this->richValidator->validate($message, $messageParameters); + $storeMessageParameters = ['message' => $message, 'parameters' => $messageParameters]; + } else { + $storeMessageParameters = [$message]; + } + $subjectTitle = 'cli'; } else { - $subject = (int)$subject; + $storeSubjectParameters = [(int)$subject]; + $storeMessageParameters = []; $subjectTitle = 'dummy'; } $notification = $this->notificationManager->createNotification(); $datetime = $this->timeFactory->getDateTime(); + $isCustomObject = $objectId !== null && $objectType !== null; + if (!$isCustomObject) { + $objectId = dechex($datetime->getTimestamp()); + $objectType = 'admin_notifications'; + } + try { $notification->setApp('admin_notifications') ->setUser($user->getUID()) - ->setDateTime($datetime) - ->setObject('admin_notifications', dechex($datetime->getTimestamp())) - ->setSubject($subjectTitle, [$subject]); + ->setObject($objectType, $objectId); + + if ($isCustomObject) { + $this->notificationManager->markProcessed($notification); + if (!$idOnly) { + $output->writeln('Previous notification for ' . $objectType . '/' . $objectId . ' marked as processed'); + } + } + + $notification->setDateTime($datetime) + ->setSubject($subjectTitle, $storeSubjectParameters); if ($message !== '') { - $notification->setMessage('cli', [$message]); + $notification->setMessage('cli', $storeMessageParameters); } $this->notificationManager->notify($notification); @@ -123,6 +191,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } + if ($idOnly) { + $output->writeln((string)$this->notificationApp->getLastInsertedId()); + } else { + $output->writeln('Notification with ID ' . $this->notificationApp->getLastInsertedId() . ''); + } return 0; } } diff --git a/tests/Integration/features/bootstrap/CommandLineTrait.php b/tests/Integration/features/bootstrap/CommandLineTrait.php new file mode 100644 index 000000000..bdab39e5f --- /dev/null +++ b/tests/Integration/features/bootstrap/CommandLineTrait.php @@ -0,0 +1,217 @@ + ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], + ]; + $process = proc_open('php console.php ' . $argString, $descriptor, $pipes, $this->ocPath, $env); + $this->lastStdOut = stream_get_contents($pipes[1]); + $this->lastStdErr = stream_get_contents($pipes[2]); + $this->lastCode = proc_close($process); + + if ($clearOpcodeCache) { + // Clean opcode cache + $client = new GuzzleHttp\Client(); + + if ($this->currentServer === 'REMOTE') { + $client->request('GET', $this->remoteServerUrl . 'apps/testing/clean_opcode_cache.php'); + } else { + $client->request('GET', $this->localServerUrl . 'apps/testing/clean_opcode_cache.php'); + } + } + + return $this->lastCode; + } + + /** + * @Given /^invoking occ with "([^"]*)"$/ + */ + public function invokingTheCommand(string $cmd, ?\Behat\Gherkin\Node\TableNode $table = null) { + if ($cmd !== 'table') { + if (str_contains($cmd, '{LAST_COMMAND_OUTPUT}')) { + echo 'Replacing {LAST_COMMAND_OUTPUT} with "' . trim($this->lastStdOut) . '"'; + } + $cmd = str_replace('{LAST_COMMAND_OUTPUT}', trim($this->lastStdOut), $cmd); + $args = explode(' ', $cmd); + } else { + $args = []; + foreach ($table->getRows() as $row) { + if ($row[0] === '') { + $args[] = $row[1]; + } elseif ($row[1] === '') { + $args[] = $row[0]; + } else { + $args[] = $row[0] . '=' . $row[1]; + } + } + } + + $this->runOcc($args); + } + + public function getLastStdOut(): string { + return $this->lastStdOut; + } + + /** + * Find exception texts in stderr + */ + public function findExceptions() { + $exceptions = []; + $captureNext = false; + // the exception text usually appears after an "[Exception"] row + foreach (explode("\n", $this->lastStdErr) as $line) { + if (preg_match('/\[Exception\]/', $line)) { + $captureNext = true; + continue; + } + if ($captureNext) { + $exceptions[] = trim($line); + $captureNext = false; + } + } + + return $exceptions; + } + + /** + * @Then /^the command was successful$/ + */ + public function theCommandWasSuccessful() { + $exceptions = $this->findExceptions(); + if ($this->lastCode !== 0) { + echo $this->lastStdErr; + + $msg = 'The command was not successful, exit code was ' . $this->lastCode . '.'; + if (!empty($exceptions)) { + $msg .= "\n" . ' Exceptions: ' . implode(', ', $exceptions); + } else { + $msg .= "\n" . ' ' . $this->lastStdOut; + $msg .= "\n" . ' ' . $this->lastStdErr; + } + throw new \Exception($msg); + } elseif (!empty($exceptions)) { + $msg = 'The command was successful but triggered exceptions: ' . implode(', ', $exceptions); + throw new \Exception($msg); + } + } + + /** + * @Then /^the command failed with exit code ([0-9]+)$/ + */ + public function theCommandFailedWithExitCode(int $exitCode) { + Assert::assertEquals($exitCode, $this->lastCode, 'The commands exit code did not match'); + } + + /** + * @Then /^the command failed with exception text "([^"]*)"$/ + */ + public function theCommandFailedWithException($exceptionText) { + $exceptions = $this->findExceptions(); + if (empty($exceptions)) { + throw new \Exception('The command did not throw any exceptions'); + } + + if (!in_array($exceptionText, $exceptions)) { + throw new \Exception('The command did not throw any exception with the text "' . $exceptionText . '"'); + } + } + + /** + * @Then /^the command output contains the text:$/ + * @Then /^the command output contains the text "([^"]*)"$/ + */ + public function theCommandOutputContainsTheText($text) { + if ($this->lastStdOut === '' && $this->lastStdErr !== '') { + Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stdout'); + Assert::assertTrue(false, 'The command did not output the expected text on stdout but stderr'); + } + + Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout'); + } + + /** + * @Then /^the command output is empty$/ + */ + public function theCommandOutputIsEmpty() { + Assert::assertEmpty($this->lastStdOut, 'The command did output unexpected text on stdout'); + } + + /** + * @Then /^the command output contains the list entry '([^']*)' with value '([^']*)'$/ + */ + public function theCommandOutputContainsTheListEntry(string $key, string $value): void { + if (preg_match('/^"ROOM\(([^"]+)\)"$/', $key, $matches)) { + $key = '"' . self::$identifierToToken[$matches[1]] . '"'; + } + $text = '- ' . $key . ': ' . $value; + + if ($this->lastStdOut === '' && $this->lastStdErr !== '') { + Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stdout'); + Assert::assertTrue(false, 'The command did not output the expected text on stdout but stderr'); + } + + Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout'); + } + + /** + * @Then /^the command error output contains the text "([^"]*)"$/ + */ + public function theCommandErrorOutputContainsTheText($text) { + if ($this->lastStdErr === '' && $this->lastStdOut !== '') { + Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout'); + Assert::assertTrue(false, 'The command did not output the expected text on stdout but stderr'); + } + + Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stderr'); + } +} diff --git a/tests/Integration/features/bootstrap/FeatureContext.php b/tests/Integration/features/bootstrap/FeatureContext.php index ed2c20adf..e603f76a0 100644 --- a/tests/Integration/features/bootstrap/FeatureContext.php +++ b/tests/Integration/features/bootstrap/FeatureContext.php @@ -45,6 +45,8 @@ class FeatureContext implements Context, SnippetAcceptingContext { /** @var string[] */ protected $appPasswords; + use CommandLineTrait; + /** * FeatureContext constructor. */ diff --git a/tests/Integration/features/cli-notification.feature b/tests/Integration/features/cli-notification.feature new file mode 100644 index 000000000..fe94a9297 --- /dev/null +++ b/tests/Integration/features/cli-notification.feature @@ -0,0 +1,45 @@ +Feature: cli-notification + Background: + Given user "test1" exists + Given as user "test1" + + Scenario: Create notification + Given invoking occ with "table" + | | notification:generate | + | | test1 | + | | {packages} require to be updated | + | --short-parameters | {"packages":{"type":"highlight","id":"count","name":"1 packages"}} | + | --long-message | Packages to update: {list} | + | --long-parameters | {"list":{"type":"highlight","id":"list","name":"package1"}} | + | --object-type | update | + | --object-id | apt | + | --output-id-only | | + And the command was successful + Then user "test1" has 1 notifications on v2 + And last notification on v2 matches + | app | admin_notifications | + | subject | 1 packages require to be updated | + | message | Packages to update: package1 | + | object_type | update | + | object_id | apt | + Given invoking occ with "table" + | | notification:generate | + | | test1 | + | | {packages} require to be updated | + | --short-parameters | {"packages":{"type":"highlight","id":"count","name":"2 packages"}} | + | --long-message | Packages to update: {list} | + | --long-parameters | {"list":{"type":"highlight","id":"list","name":"package1, package2"}} | + | --object-type | update | + | --object-id | apt | + | --output-id-only | | + And the command was successful + Then user "test1" has 1 notifications on v2 + And last notification on v2 matches + | app | admin_notifications | + | subject | 2 packages require to be updated | + | message | Packages to update: package1, package2 | + | object_type | update | + | object_id | apt | + Given invoking occ with "notification:delete test1 {LAST_COMMAND_OUTPUT}" + And the command was successful + Then user "test1" has 0 notifications on v2 diff --git a/tests/Unit/Command/GenerateTest.php b/tests/Unit/Command/GenerateTest.php index 338c90634..fd9036920 100644 --- a/tests/Unit/Command/GenerateTest.php +++ b/tests/Unit/Command/GenerateTest.php @@ -6,12 +6,15 @@ namespace OCA\Notifications\Tests\Unit\Command; +use OCA\Notifications\App; use OCA\Notifications\Command\Generate; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager; use OCP\Notification\INotification; +use OCP\RichObjectStrings\IValidator; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -22,12 +25,16 @@ * @group DB */ class GenerateTest extends \Test\TestCase { - /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ITimeFactory|MockObject */ protected $timeFactory; - /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserManager|MockObject */ protected $userManager; - /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IManager|MockObject */ protected $notificationManager; + /** @var IValidator|MockObject */ + protected $richValidator; + /** @var App|MockObject */ + protected $notificationApp; /** @var Generate */ protected $command; @@ -37,11 +44,15 @@ protected function setUp(): void { $this->timeFactory = $this->createMock(ITimeFactory::class); $this->userManager = $this->createMock(IUserManager::class); $this->notificationManager = $this->createMock(IManager::class); + $this->richValidator = $this->createMock(IValidator::class); + $this->notificationApp = $this->createMock(App::class); $this->command = new Generate( $this->timeFactory, $this->userManager, - $this->notificationManager + $this->notificationManager, + $this->richValidator, + $this->notificationApp, ); } @@ -139,14 +150,12 @@ public function testExecute($userId, $short, $long, $createNotification, $notify } $input = $this->createMock(InputInterface::class); - $input->expects($this->exactly(2)) - ->method('getArgument') + $input->method('getArgument') ->willReturnMap([ ['user-id', $userId], ['short-message', $short], ]); - $input->expects($this->exactly(2)) - ->method('getOption') + $input->method('getOption') ->willReturnMap([ ['long-message', $long], ['dummy', false],