Update mark notification as read method (#17518)

* update mark notification as read method

* Ensure notificationId is passed correctly to markNotificationAsRead

* Adds notification UI tests

Co-authored-by: sgiehl <stefan@matomo.org>

9 files changed, 113 insertions, 4 deletions

diff --git a/plugins/CoreHome/Controller.php b/plugins/CoreHome/Controller.php
index 1eb51f06f4..1254260eaa 100644
--- a/plugins/CoreHome/Controller.php
+++ b/plugins/CoreHome/Controller.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugins\CoreHome;
 use Exception;
 use Piwik\API\Request;
 use Piwik\Common;
+use Piwik\DataTable\Renderer\Json;
 use Piwik\Date;
 use Piwik\FrontController;
 use Piwik\Notification\Manager as NotificationManager;
@@ -42,7 +43,7 @@ class Controller extends \Piwik\Plugin\Controller
 
         parent::__construct();
     }
-    
+
     public function getDefaultAction()
     {
         return 'redirectToCoreHomeIndex';
@@ -156,8 +157,14 @@ class Controller extends \Piwik\Plugin\Controller
 
     public function markNotificationAsRead()
     {
+        Piwik::checkUserHasSomeViewAccess();
+        $this->checkTokenInUrl();
+
         $notificationId = Common::getRequestVar('notificationId');
         NotificationManager::cancel($notificationId);
+
+        Json::sendHeaderJSON();
+        return json_encode(true);
     }
 
     protected function getDefaultIndexView()
diff --git a/plugins/CoreHome/angularjs/notification/notification.controller.js b/plugins/CoreHome/angularjs/notification/notification.controller.js
index 62a0e7881c..80497e940f 100644
--- a/plugins/CoreHome/angularjs/notification/notification.controller.js
+++ b/plugins/CoreHome/angularjs/notification/notification.controller.js
@@ -14,12 +14,12 @@
          * Marks a persistent notification as read so it will not reappear on the next page
          * load.
          */
-        this.markNotificationAsRead = function () {
-            var notificationId = this.notificationId;
+        this.markNotificationAsRead = function (notificationId) {
             if (!notificationId) {
                 return;
             }
 
+            piwikApi.withTokenInUrl();
             piwikApi.post(
                 { // GET params
                     module: 'CoreHome',
diff --git a/plugins/CoreHome/angularjs/notification/notification.directive.html b/plugins/CoreHome/angularjs/notification/notification.directive.html
index be97d91d9a..82138156f1 100644
--- a/plugins/CoreHome/angularjs/notification/notification.directive.html
+++ b/plugins/CoreHome/angularjs/notification/notification.directive.html
@@ -1,5 +1,5 @@
 <div class="notification system">
-    <button type="button" class="close" data-dismiss="alert" ng-if="!noclear" ng-click="notification.markNotificationAsRead()">&times;</button>
+    <button type="button" class="close" data-dismiss="alert" ng-if="!noclear" ng-click="notification.markNotificationAsRead(notificationId)">&times;</button>
     <strong ng-if="title">{{ title }}</strong>
 
     <!-- ng-transclude causes directive child elements to be added here -->
diff --git a/plugins/CoreHome/config/test.php b/plugins/CoreHome/config/test.php
new file mode 100644
index 0000000000..9e625c4220
--- /dev/null
+++ b/plugins/CoreHome/config/test.php
@@ -0,0 +1,38 @@
+<?php
+
+use Piwik\Notification;
+
+return [
+    'observers.global' => DI\add([
+        [
+            'Request.dispatch',
+            DI\value(
+                function () {
+                    if (!empty($_GET['setNotifications']) && $_GET['setNotifications'] == 1) {
+                        // trigger some notification
+                        $notification          = new Notification('This is a persistent test notification');
+                        $notification->title   = 'Warning:';
+                        $notification->context = Notification::CONTEXT_WARNING;
+                        $notification->type    = Notification::TYPE_PERSISTENT;
+                        $notification->flags   = Notification::FLAG_CLEAR;
+                        Notification\Manager::notify('NotificationFixture_persistent_warning', $notification);
+
+                        $notification          = new Notification('This is another persistent test notification');
+                        $notification->title   = 'Error:';
+                        $notification->context = Notification::CONTEXT_ERROR;
+                        $notification->type    = Notification::TYPE_PERSISTENT;
+                        $notification->flags   = Notification::FLAG_CLEAR;
+                        Notification\Manager::notify('NotificationFixture_persistent_error', $notification);
+
+                        $notification          = new Notification('This is transient test notification');
+                        $notification->title   = 'Error:';
+                        $notification->context = Notification::CONTEXT_ERROR;
+                        $notification->type    = Notification::TYPE_TRANSIENT;
+                        $notification->flags   = Notification::FLAG_CLEAR;
+                        Notification\Manager::notify('NotificationFixture_transient_error', $notification);
+                    }
+                }
+            ),
+        ],
+    ]),
+];
\ No newline at end of file
diff --git a/plugins/CoreHome/tests/UI/Notifications_spec.js b/plugins/CoreHome/tests/UI/Notifications_spec.js
new file mode 100644
index 0000000000..1635c435d3
--- /dev/null
+++ b/plugins/CoreHome/tests/UI/Notifications_spec.js
@@ -0,0 +1,52 @@
+/*!
+ * Matomo - free/libre analytics platform
+ *
+ * Dashboard screenshot tests.
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+describe('Notifications', function () {
+    this.timeout(0);
+
+    this.fixture = "Piwik\\Tests\\Fixtures\\OneVisit";
+
+    var url = "?module=CoreAdminHome&action=home&idSite=1&period=day&date=yesterday";
+
+    it('should show notifications', async function () {
+        await page.goto(url + '&setNotifications=1');
+        await page.waitForNetworkIdle();
+
+        var elem = await page.waitForSelector('#notificationContainer');
+
+        expect(await elem.screenshot()).to.matchImage('loaded');
+    });
+
+    it('should still show persistent notifications on reload', async function () {
+        await page.goto(url);
+        await page.waitForNetworkIdle();
+
+        var elem = await page.waitForSelector('#notificationContainer');
+
+        expect(await elem.screenshot()).to.matchImage('reloaded');
+    });
+
+    it('should close a notification', async function () {
+        await page.click('.notification:first-child .close');
+        await page.waitForNetworkIdle();
+
+        var elem = await page.waitForSelector('#notificationContainer');
+
+        expect(await elem.screenshot()).to.matchImage('close');
+    });
+
+    it('should still be closed on reload', async function () {
+        await page.reload();
+        await page.waitForNetworkIdle();
+
+        var elem = await page.waitForSelector('#notificationContainer');
+
+        expect(await elem.screenshot()).to.matchImage('close_reload');
+    });
+});
diff --git a/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close.png b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close.png
new file mode 100644
index 0000000000..3d32433f03
--- /dev/null
+++ b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:3ab00ad8715daf05847c359e48ab19e7e82b26f40edd79e1445c0167a59e12a6
+size 5887
diff --git a/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close_reload.png b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close_reload.png
new file mode 100644
index 0000000000..3d32433f03
--- /dev/null
+++ b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_close_reload.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:3ab00ad8715daf05847c359e48ab19e7e82b26f40edd79e1445c0167a59e12a6
+size 5887
diff --git a/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_loaded.png b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_loaded.png
new file mode 100644
index 0000000000..fefd4863d1
--- /dev/null
+++ b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_loaded.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:003ae9a97b33f35347d699eb03700e0a38bd153b5d58e32e8991a18b590bede0
+size 17049
diff --git a/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_reloaded.png b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_reloaded.png
new file mode 100644
index 0000000000..f063ecc752
--- /dev/null
+++ b/plugins/CoreHome/tests/UI/expected-screenshots/Notifications_reloaded.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:30518830c1a132dbe2978eae8e5022a9c61f7705121bb1048f37e5283d50c8f4
+size 11767