CCMSPUI-645: Added USER ID request param to get notification endpoint (…

…#157) * CCMSPUI-645 Initial fix Signed-off-by: Jamie Briggs <jamie.briggs@digital.justice.gov.uk> * CCMSPUI-645 Added additional user ID request param to get notification endpoint Signed-off-by: Jamie Briggs <jamie.briggs@digital.justice.gov.uk> * CCMSPUI-645 Added JavaDoc to NotificationRepository Signed-off-by: Jamie Briggs <jamie.briggs@digital.justice.gov.uk> --------- Signed-off-by: Jamie Briggs <jamie.briggs@digital.justice.gov.uk> Co-authored-by: Jamie Briggs <jamie.briggs@digital.justice.gov.uk>
ministryofjustice · Mar 4, 2025 · 79e39dc · 79e39dc
1 parent 4202dc4
commit 79e39dc
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 17 deletions.
diff --git a/data-api/open-api-specification.yml b/data-api/open-api-specification.yml
@@ -1255,6 +1255,12 @@ paths:
             type: 'integer'
             format: 'int64'
             example: '123456789'
+        - name: 'user-id'
+          in: 'query'
+          required: true
+          schema:
+            type: 'string'
+            example: '123456789'
         - name: 'provider-id'
           in: 'query'
           required: true

diff --git a/data-service/src/main/java/uk/gov/laa/ccms/data/controller/NotificationsController.java b/data-service/src/main/java/uk/gov/laa/ccms/data/controller/NotificationsController.java
@@ -38,13 +38,16 @@ public class NotificationsController implements NotificationsApi {
    * Retrieves a notification based on the provided notification ID and provider ID.
    *
    * @param notificationId the unique identifier of the notification to retrieve
+   * @param userId the unique identifier of the user assigned to the notification
    * @param providerId the unique identifier of the provider associated with the notification
    * @return a {@code ResponseEntity} containing the retrieved {@code Notification} if found,
    *         or a {@code ResponseEntity} with HTTP status 404 if the notification is not found.
    */
   @Override
-  public ResponseEntity<Notification> getNotification(Long notificationId, Long providerId) {
-    return notificationService.getNotification(notificationId, providerId).map(ResponseEntity::ok)
+  public ResponseEntity<Notification> getNotification(Long notificationId, String userId,
+      Long providerId) {
+    return notificationService.getNotification(notificationId, userId, providerId)
+        .map(ResponseEntity::ok)
         .orElse(ResponseEntity.notFound().build());
   }
 

diff --git a/data-service/src/main/java/uk/gov/laa/ccms/data/repository/NotificationRepository.java b/data-service/src/main/java/uk/gov/laa/ccms/data/repository/NotificationRepository.java
@@ -1,5 +1,6 @@
 package uk.gov.laa.ccms.data.repository;
 
+import java.util.Optional;
 import org.springframework.stereotype.Repository;
 import uk.gov.laa.ccms.data.entity.NotificationInfo;
 
@@ -16,4 +17,14 @@
 @Repository
 public interface NotificationRepository extends ReadOnlyRepository<NotificationInfo, Long> {
 
+  /**
+   * Finds and returns a {@link NotificationInfo} object matching both the notification ID and
+   *     user ID.
+   *
+   * @param notificationId notification identifier
+   * @param userId the user assigned to the notification
+   * @return an {@link Optional} containing a {@link NotificationInfo} if a notification is found
+   *     using the filters, or an empty {@link Optional} if a notification was not found.
+   */
+  Optional<NotificationInfo> findByNotificationIdAndUserId(Long notificationId, String userId);
 }
diff --git a/data-service/src/main/java/uk/gov/laa/ccms/data/service/NotificationService.java b/data-service/src/main/java/uk/gov/laa/ccms/data/service/NotificationService.java
@@ -106,15 +106,18 @@ public Optional<Notifications> getNotifications(final long providerId,
    * Retrieves a notification based on its ID and verifies the provider firm's ID.
    *
    * @param notificationId the unique identifier of the notification to retrieve
+   * @param userId the identifier of the user assigned to the notification to retrieve
    * @param providerFirmId the identifier of the provider firm to validate access
    * @return an Optional containing the Notification if found and accessible,
    *     otherwise an empty Optional
    * @throws ResponseStatusException if the provider firm ID does not match,
    *     returning a forbidden status
    */
   public Optional<Notification> getNotification(final long notificationId,
+      final String userId,
       final long providerFirmId) {
-    Optional<NotificationInfo> byId = notificationRepository.findById(notificationId);
+    Optional<NotificationInfo> byId = notificationRepository
+        .findByNotificationIdAndUserId(notificationId, userId);
 
     // Return empty if not found
     if (byId.isEmpty()) {

diff --git a/data-service/src/test/java/uk/gov/laa/ccms/data/controller/NotificationsControllerTest.java b/data-service/src/test/java/uk/gov/laa/ccms/data/controller/NotificationsControllerTest.java
@@ -126,10 +126,10 @@ void getNotifications_badRequest() throws Exception {
   void getNotification_returnsData() throws Exception {
     //Given
     Notification expected = new Notification().notificationId("123").providerFirmId("1");
-    when(notificationService.getNotification(123L, 1L)).thenReturn(Optional.of(expected));
+    when(notificationService.getNotification(123L, "2", 1L)).thenReturn(Optional.of(expected));
     // Then
     String jsonContent = objectMapper.writeValueAsString(expected);
-    this.mockMvc.perform(get("/notifications/123?provider-id=1"))
+    this.mockMvc.perform(get("/notifications/123?user-id=2&provider-id=1"))
         .andDo(print())
         .andExpect(status().isOk())
         .andExpect(content().json(jsonContent));
@@ -139,10 +139,10 @@ void getNotification_returnsData() throws Exception {
   @DisplayName("getNotification: Forbidden data")
   void getNotification_forbidden() throws Exception {
     //Given
-    when(notificationService.getNotification(123L, 1L))
+    when(notificationService.getNotification(123L, "2",1L))
         .thenThrow(new ResponseStatusException(FORBIDDEN, "Access Denied"));
     // Then
-    this.mockMvc.perform(get("/notifications/123?provider-id=1"))
+    this.mockMvc.perform(get("/notifications/123?user-id=2&provider-id=1"))
         .andDo(print())
         .andExpect(status().isForbidden());
   }
@@ -153,17 +153,27 @@ void getNotification_forbidden() throws Exception {
   void getNotification_notFound() throws Exception {
     //Given
     // Then
-    this.mockMvc.perform(get("/notifications/123?provider-id=1"))
+    this.mockMvc.perform(get("/notifications/123?user-id=2&provider-id=1"))
         .andDo(print())
         .andExpect(status().isNotFound());
   }
 
   @Test
-  @DisplayName("getNotification: Bad request")
+  @DisplayName("getNotification: Bad request missing provider ID")
   void getNotification_badRequest() throws Exception {
     //Given
     // Then
-    this.mockMvc.perform(get("/notifications/abc"))
+    this.mockMvc.perform(get("/notifications/abc?user-id=123"))
+        .andDo(print())
+        .andExpect(status().isBadRequest());
+  }
+
+  @Test
+  @DisplayName("getNotification: Bad request missing user ID")
+  void getNotification_badRequestMissingUser() throws Exception {
+    //Given
+    // Then
+    this.mockMvc.perform(get("/notifications/abc?provider-id=123"))
         .andDo(print())
         .andExpect(status().isBadRequest());
   }

diff --git a/data-service/src/test/java/uk/gov/laa/ccms/data/service/NotificationServiceTest.java b/data-service/src/test/java/uk/gov/laa/ccms/data/service/NotificationServiceTest.java
@@ -166,17 +166,18 @@ class GetNotificationTests{
     void shouldReturnNotification(){
       // Given
       long notificationId = 1L;
+      String userId = "123456";
       NotificationInfo notificationInfo = NotificationInfo.builder().providerFirmId(1L).build();
       Notification expected = new Notification().notificationId("1").providerFirmId("1");
-      when(notificationRepository.findById(notificationId))
+      when(notificationRepository.findByNotificationIdAndUserId(notificationId, userId))
           .thenReturn(Optional.of(notificationInfo));
       when(notificationMapper.mapToNotification(notificationInfo)).thenReturn(expected);
       // When
-      Optional<Notification> result = notificationService.getNotification(notificationId, 1L);
+      Optional<Notification> result = notificationService.getNotification(notificationId, userId, 1L);
       // Then
       assertTrue(result.isPresent());
       assertEquals(expected, result.get());
-      verify(notificationRepository, times(1)).findById(notificationId);
+      verify(notificationRepository, times(1)).findByNotificationIdAndUserId(notificationId, userId);
       verify(notificationMapper, times(1)).mapToNotification(notificationInfo);
     }
 
@@ -185,11 +186,12 @@ void shouldReturnNotification(){
     void shouldReturnEmpty(){
       // Given
       long notificationId = 1L;
+      String userId = "123456";
       // When
-      Optional<Notification> result = notificationService.getNotification(notificationId, 1L);
+      Optional<Notification> result = notificationService.getNotification(notificationId, userId, 1L);
       // Then
       assertTrue(result.isEmpty());
-      verify(notificationRepository, times(1)).findById(notificationId);
+      verify(notificationRepository, times(1)).findByNotificationIdAndUserId(notificationId, userId);
       verify(notificationMapper, times(0)).mapToNotification(any());
     }
 
@@ -198,12 +200,13 @@ void shouldReturnEmpty(){
     void shouldThrowException() {
       // Given
       long notificationId = 1L;
+      String userId = "123456";
       NotificationInfo notificationInfo = NotificationInfo.builder().providerFirmId(1L).build();
-      when(notificationRepository.findById(notificationId))
+      when(notificationRepository.findByNotificationIdAndUserId(notificationId, userId))
           .thenReturn(Optional.of(notificationInfo));
       // When / Then
       assertThrows(ResponseStatusException.class,
-          () -> notificationService.getNotification(notificationId, 500));
+          () -> notificationService.getNotification(notificationId, userId, 500));
     }
   }
 }