From 1c73ffc1fd551a0dc03f182435224f239800d9e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adam=20W=C3=B3js?= <adam@wojs.pl>
Date: Fri, 29 Jun 2018 11:56:10 +0200
Subject: [PATCH] EZEE-2081: NotificationService clean up     (#2368)

* EZEE-2081: Missing backticks around owner_id column

* EZEE-2081: Removed out of date SQL query to eznotification table creation from upgrade notes

* EZEE-2081: Removed unused import from eZ/Publish/SPI/Persistence/Handler

* EZEE-2081: Added \eZ\Publish\API\Repository\Values\Notification\CreateStruct validation

* EZEE-2081: Moved @throws from impl. to \eZ\Publish\API\Repository\NotificationService

* EZEE-2081: Moved NotificationRenderer to eZ\Publish\Core\Notification\Renderer\

* EZEE-2081: Replaced tag ezstudio.notification.renderer with ezpublish.notification.renderer

* EZEE-2081: CS

* EZEE-2081: Applied the CR suggestions
---
 Repository/NotificationService.php           | 35 +++++++++++-----
 Repository/Tests/NotificationServiceTest.php | 42 ++++++++++++++++++++
 2 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/Repository/NotificationService.php b/Repository/NotificationService.php
index 973784755..3c1f11823 100644
--- a/Repository/NotificationService.php
+++ b/Repository/NotificationService.php
@@ -12,20 +12,28 @@
 use eZ\Publish\API\Repository\Values\Notification\Notification;
 use eZ\Publish\API\Repository\Values\Notification\NotificationList;
 
+/**
+ * Service to manager user notifications. It works in the context of a current User (obtained from
+ * the PermissionResolver).
+ */
 interface NotificationService
 {
     /**
      * Get currently logged user notifications.
-
-     * @param int $offset
-     * @param int $limit
+     *
+     * @param int $offset the start offset for paging
+     * @param int $limit  the number of notifications returned
      *
      * @return \eZ\Publish\API\Repository\Values\Notification\NotificationList
      */
     public function loadNotifications(int $offset, int $limit): NotificationList;
 
     /**
-     * @param int $notificationId
+     * Load single notification (by ID).
+     *
+     * @param int $notificationId Notification ID
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
      *
      * @return \eZ\Publish\API\Repository\Values\Notification\Notification
      */
@@ -35,6 +43,9 @@ public function getNotification(int $notificationId): Notification;
      * Mark notification as read so it no longer bother the user.
      *
      * @param \eZ\Publish\API\Repository\Values\Notification\Notification $notification
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
+     * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
      */
     public function markNotificationAsRead(Notification $notification): void;
 
@@ -53,14 +64,20 @@ public function getPendingNotificationCount(): int;
     public function getNotificationCount(): int;
 
     /**
-     * @param \eZ\Publish\API\Repository\Values\Notification\Notification $notification
-     */
-    public function deleteNotification(Notification $notification): void;
-
-    /**
+     * Creates a new notification.
+     *
      * @param \eZ\Publish\API\Repository\Values\Notification\CreateStruct $createStruct
      *
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     *
      * @return \eZ\Publish\API\Repository\Values\Notification\Notification
      */
     public function createNotification(CreateStruct $createStruct): Notification;
+
+    /**
+     * Deletes a notification.
+     *
+     * @param \eZ\Publish\API\Repository\Values\Notification\Notification $notification
+     */
+    public function deleteNotification(Notification $notification): void;
 }
diff --git a/Repository/Tests/NotificationServiceTest.php b/Repository/Tests/NotificationServiceTest.php
index 936d58b55..1bc9a2161 100644
--- a/Repository/Tests/NotificationServiceTest.php
+++ b/Repository/Tests/NotificationServiceTest.php
@@ -154,4 +154,46 @@ public function testCreateNotification()
         $this->assertInstanceOf(Notification::class, $notification);
         $this->assertGreaterThan(0, $notification->id);
     }
+
+    /**
+     * @covers \eZ\Publish\API\Repository\NotificationService::createNotification()
+     * @depends testCreateNotification
+     * @expectedException \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     */
+    public function testCreateNotificationThrowsInvalidArgumentExceptionOnMissingOwner()
+    {
+        $repository = $this->getRepository();
+
+        /* BEGIN: Use Case */
+        $notificationService = $repository->getNotificationService();
+
+        $createStruct = new CreateStruct([
+            'type' => 'TEST',
+        ]);
+
+        // This call will fail because notification owner is not specified
+        $notificationService->createNotification($createStruct);
+        /* END: Use Case */
+    }
+
+    /**
+     * @covers \eZ\Publish\API\Repository\NotificationService::createNotification()
+     * @depends testCreateNotification
+     * @expectedException \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
+     */
+    public function testCreateNotificationThrowsInvalidArgumentExceptionOnMissingType()
+    {
+        $repository = $this->getRepository();
+
+        /* BEGIN: Use Case */
+        $notificationService = $repository->getNotificationService();
+
+        $createStruct = new CreateStruct([
+            'ownerId' => 14,
+        ]);
+
+        // This call will fail because notification type is not specified
+        $notificationService->createNotification($createStruct);
+        /* END: Use Case */
+    }
 }