Merge pull request #327 from nextcloud/bugfix/317/notification-urls

fix(notifications): Only define URLs and actions in one place
nextcloud · Sep 30, 2024 · b8bfab6 · b8bfab6
2 parents fc697a4 + 3b77831
commit b8bfab6
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 58 deletions.
diff --git a/lib/BackgroundJobs/AdminNotification.php b/lib/BackgroundJobs/AdminNotification.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -10,46 +12,25 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\QueuedJob;
 use OCP\IGroupManager;
-use OCP\IURLGenerator;
 use OCP\Notification\IManager;
 
 class AdminNotification extends QueuedJob {
-	protected IManager $manager;
-	protected IGroupManager $groupManager;
-	protected IURLGenerator $url;
-
-	public function __construct(ITimeFactory $time,
-		IManager $manager,
-		IGroupManager $groupManager,
-		IURLGenerator $url) {
+	public function __construct(
+		ITimeFactory $time,
+		protected IManager $manager,
+		protected IGroupManager $groupManager,
+	) {
 		parent::__construct($time);
-		$this->manager = $manager;
-		$this->groupManager = $groupManager;
-		$this->url = $url;
 	}
 
 	protected function run($argument): void {
 		$notification = $this->manager->createNotification();
 
 		$notification->setApp('survey_client')
-			->setDateTime(new \DateTime())
+			->setDateTime($this->time->getDateTime())
 			->setSubject('updated')
 			->setObject('dummy', '23');
 
-		$enableLink = $this->url->linkToOCSRouteAbsolute('survey_client.Endpoint.enableMonthly');
-		$enableAction = $notification->createAction();
-		$enableAction->setLabel('enable')
-			->setLink($enableLink, 'POST')
-			->setPrimary(true);
-		$notification->addAction($enableAction);
-
-		$disableLink = $this->url->linkToOCSRouteAbsolute('survey_client.Endpoint.disableMonthly');
-		$disableAction = $notification->createAction();
-		$disableAction->setLabel('disable')
-			->setLink($disableLink, 'DELETE')
-			->setPrimary(false);
-		$notification->addAction($disableAction);
-
 		$adminGroup = $this->groupManager->get('admin');
 		foreach ($adminGroup->getUsers() as $admin) {
 			$notification->setUser($admin->getUID());

diff --git a/lib/Notifier.php b/lib/Notifier.php
@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -11,24 +14,13 @@
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
+use OCP\Notification\UnknownNotificationException;
 
 class Notifier implements INotifier {
-
-	/** @var IFactory */
-	protected $l10nFactory;
-
-	/** @var IURLGenerator */
-	protected $url;
-
-	/**
-	 * Notifier constructor.
-	 *
-	 * @param IFactory $l10nFactory
-	 * @param IURLGenerator $url
-	 */
-	public function __construct(IFactory $l10nFactory, IURLGenerator $url) {
-		$this->l10nFactory = $l10nFactory;
-		$this->url = $url;
+	public function __construct(
+		protected IFactory $l10nFactory,
+		protected IURLGenerator $url,
+	) {
 	}
 
 	/**
@@ -42,7 +34,7 @@ public function getID(): string {
 	}
 
 	/**
-	 * Human readable name describing the notifier
+	 * Human-readable name describing the notifier
 	 *
 	 * @return string
 	 * @since 17.0.0
@@ -55,32 +47,35 @@ public function getName(): string {
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
 	 * @return INotification
-	 * @throws \InvalidArgumentException When the notification was not prepared by a notifier
+	 * @throws UnknownNotificationException When the notification was not prepared by a notifier
 	 */
 	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'survey_client') {
 			// Not my app => throw
-			throw new \InvalidArgumentException();
+			throw new UnknownNotificationException();
 		}
 
 		// Read the language from the notification
 		$l = $this->l10nFactory->get('survey_client', $languageCode);
 
 		$notification->setParsedSubject($l->t('Help improve Nextcloud'))
 			->setParsedMessage($l->t('Do you want to help us to improve Nextcloud by providing some anonymized data about your setup and usage? You can disable it at any time in the admin settings again.'))
-			->setLink($this->url->linkToRoute('settings.AdminSettings.index', ['section' => 'survey_client']))
-			->setIcon($this->url->imagePath('survey_client', 'app-dark.svg'));
+			->setLink($this->url->linkToRouteAbsolute('settings.AdminSettings.index', ['section' => 'survey_client']))
+			->setIcon($this->url->getAbsoluteURL($this->url->imagePath('survey_client', 'app-dark.svg')));
 
-		foreach ($notification->getActions() as $action) {
-			if ($action->getLabel() === 'disable') {
-				$action->setParsedLabel($l->t('Not now'))
-					->setLink($this->url->getAbsoluteURL('ocs/v2.php/apps/survey_client/api/v1/monthly'), 'DELETE');
-			} elseif ($action->getLabel() === 'enable') {
-				$action->setParsedLabel($l->t('Send usage'))
-					->setLink($this->url->getAbsoluteURL('ocs/v2.php/apps/survey_client/api/v1/monthly'), 'POST');
-			}
-			$notification->addParsedAction($action);
-		}
+		$enableAction = $notification->createAction();
+		$enableAction->setLabel('enable')
+			->setParsedLabel($l->t('Send usage'))
+			->setLink($this->url->linkToOCSRouteAbsolute('survey_client.Endpoint.enableMonthly'), 'POST')
+			->setPrimary(true);
+		$notification->addParsedAction($enableAction);
+
+		$disableAction = $notification->createAction();
+		$disableAction->setLabel('disable')
+			->setParsedLabel($l->t('Not now'))
+			->setLink($this->url->linkToOCSRouteAbsolute('survey_client.Endpoint.disableMonthly'), 'DELETE')
+			->setPrimary(false);
+		$notification->addParsedAction($disableAction);
 
 		return $notification;
 	}

diff --git a/psalm.xml b/psalm.xml
@@ -1,6 +1,10 @@
 <?xml version="1.0"?>
 <psalm
     errorLevel="4"
+    findUnusedBaselineEntry="true"
+    findUnusedCode="false"
+    resolveFromConfigFile="true"
+    phpVersion="8.1"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"