Updated files according to Michael's review, overcoming many errors to implement complete functionality to allow standard users the ability to deny feedback requests (as a new method with separate permission). The notification service is not yet complete, and some review facets need to be revisited. This is a valuable commit to save progress and clarify working state of long-outstanding PR draft.

thelenw · thelenw · commit 05143b527ca8 · 2024-11-11T20:47:34.000-06:00
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/CreatorDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/CreatorDTO.java
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/DenyFeedbackRequestDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/DenyFeedbackRequestDTO.java
@@ -1,26 +1,29 @@
 package com.objectcomputing.checkins.services.feedback_request.DTO;
 
+import io.micronaut.core.annotation.Introspected;
 import jakarta.validation.Valid;
 import jakarta.validation.constraints.NotBlank;
 import jakarta.validation.constraints.NotNull;
 
+@Introspected
+
 public class DenyFeedbackRequestDTO {
 
     @NotBlank(message = "Reason cannot be blank")
     private String reason;
 
     @NotNull(message = "Denier cannot be null")
     @Valid
-    private DenierDTO denier;
+    private UserDTO denier;
 
     @NotNull(message = "Creator cannot be null")
     @Valid
-    private CreatorDTO creator;
+    private UserDTO creator;
 
     // Constructors
     public DenyFeedbackRequestDTO() {}
 
-    public DenyFeedbackRequestDTO(String reason, DenierDTO denier, CreatorDTO creator) {
+    public DenyFeedbackRequestDTO(String reason, UserDTO denier, UserDTO creator) {
         this.reason = reason;
         this.denier = denier;
         this.creator = creator;
@@ -31,11 +34,11 @@ public String getReason() {
         return reason;
     }
 
-    public DenierDTO getDenier() {
+    public UserDTO getDenier() {
         return denier;
     }
 
-    public CreatorDTO getCreator() {
+    public UserDTO getCreator() {
         return creator;
     }
 
@@ -44,11 +47,11 @@ public void setReason(String reason) {
         this.reason = reason;
     }
 
-    public void setDenier(DenierDTO denier) {
+    public void setDenier(UserDTO denier) {
         this.denier = denier;
     }
 
-    public void setCreator(CreatorDTO creator) {
+    public void setCreator(UserDTO creator) {
         this.creator = creator;
     }
 }
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/UserDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/DTO/UserDTO.java
@@ -4,18 +4,29 @@
 import jakarta.validation.constraints.NotNull;
 import java.util.UUID;
 
-public class DenierDTO {
+import io.micronaut.core.annotation.Introspected;
 
-    @NotNull(message = "Denier ID cannot be null")
+
+@Introspected
+
+public class UserDTO {
+
+    @NotNull(message = "User ID cannot be null")
     private UUID id;
 
-    @NotBlank(message = "Denier name cannot be blank")
+    @NotBlank(message = "User name cannot be blank")
     private String name;
 
     // Constructors
-    public DenierDTO() {}
+    public UserDTO() {}
+
+    // Constructor with only ID (for cases where only ID is required)
+    public UserDTO(UUID id) {
+        this.id = id;
+    }
 
-    public DenierDTO(UUID id, String name) {
+    // Constructor with ID and name (for cases where both ID and name are required)
+    public UserDTO(UUID id, String name) {
         this.id = id;
         this.name = name;
     }
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java
@@ -3,8 +3,7 @@
 import com.objectcomputing.checkins.services.permissions.Permission;
 import com.objectcomputing.checkins.services.permissions.RequiredPermission;
 import com.objectcomputing.checkins.services.feedback_request.DTO.DenyFeedbackRequestDTO;
-import com.objectcomputing.checkins.services.feedback_request.DTO.DenierDTO;
-import com.objectcomputing.checkins.services.feedback_request.DTO.CreatorDTO;
+import com.objectcomputing.checkins.services.feedback_request.DTO.UserDTO;
 import com.objectcomputing.checkins.services.notification.NotificationService;
 import io.micronaut.core.annotation.Nullable;
 import io.micronaut.core.convert.format.Format;
@@ -137,21 +136,21 @@ public List<FeedbackRequestResponseDTO> findByValues(
      * @return {@link FeedbackRequestResponseDTO} with updated denial status
      */
     @Post("/{id}/deny")
-@RequiredPermission(Permission.CAN_DENY_FEEDBACK_REQUEST)
-public HttpResponse<FeedbackRequestResponseDTO> denyFeedbackRequest(
-    @PathVariable("id") @NotNull UUID id, 
-    @Body @Valid DenyFeedbackRequestDTO body
-) {
-    FeedbackRequest feedbackRequest = feedbackReqServices.getById(id);
-    if (feedbackRequest == null) {
-        return HttpResponse.notFound();
+    @RequiredPermission(Permission.CAN_DENY_FEEDBACK_REQUEST)
+    public HttpResponse<FeedbackRequestResponseDTO> denyFeedbackRequest(
+        @PathVariable("id") @NotNull UUID id, 
+        @Body @Valid DenyFeedbackRequestDTO body
+    ) {
+        FeedbackRequest feedbackRequest = feedbackReqServices.getById(id);
+        if (feedbackRequest == null) {
+            return HttpResponse.notFound();
     }
 
     String reason = body.getReason();
-    DenierDTO denier = body.getDenier();
-    CreatorDTO creator = body.getCreator();
+    UserDTO denier = body.getDenier();
+    UserDTO creator = body.getCreator();
 
-    if (!feedbackRequest.isDenied() && reason != null && !reason.trim().isEmpty()) {
+    if (!feedbackRequest.isDenied() && reason != null && !reason.trim().isEmpty() && denier != null && creator != null) {
         FeedbackRequestUpdateDTO dto = new FeedbackRequestUpdateDTO();
         dto.setId(feedbackRequest.getId());
         dto.setDueDate(feedbackRequest.getDueDate());
@@ -171,9 +170,10 @@ public HttpResponse<FeedbackRequestResponseDTO> denyFeedbackRequest(
         );
 
         return HttpResponse.ok(fromEntity(updatedFeedbackRequest));
-    }
+    } else {
+        return HttpResponse.badRequest();
 
-    return HttpResponse.ok(fromEntity(feedbackRequest));
+    }
 }
 
     private FeedbackRequestResponseDTO fromEntity(FeedbackRequest feedbackRequest) {
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestDenialDTO.txt b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestDenialDTO.txt
@@ -0,0 +1,59 @@
+package com.objectcomputing.checkins.services.feedback_request;
+
+import io.micronaut.core.annotation.Introspected;
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import com.objectcomputing.checkins.services.feedback_request.DTO.UserDTO;
+
+
+@Introspected
+
+public class FeedbackRequestDenialDTO {
+
+    @NotBlank(message = "Reason cannot be blank")
+    private String reason;
+
+    @NotNull(message = "Denier cannot be null")
+    @Valid
+    private UserDTO denier;
+
+    @NotNull(message = "Creator cannot be null")
+    @Valid
+    private UserDTO creator;
+
+    // Constructors
+    public FeedbackRequestDenialDTO() {}
+
+    public FeedbackRequestDenialDTO(String reason, UserDTO denier, UserDTO creator) {
+        this.reason = reason;
+        this.denier = denier;
+        this.creator = creator;
+    }
+
+    // Getters
+    public String getReason() {
+        return reason;
+    }
+
+    public UserDTO getDenier() {
+        return denier;
+    }
+
+    public UserDTO getCreator() {
+        return creator;
+    }
+
+    // Setters
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    public void setDenier(UserDTO denier) {
+        this.denier = denier;
+    }
+
+    public void setCreator(UserDTO creator) {
+        this.creator = creator;
+    }
+}
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java
@@ -215,6 +215,10 @@ public FeedbackRequest update(FeedbackRequestUpdateDTO feedbackRequestUpdateDTO)
             throw new BadArgException("Send date of feedback request must be before the due date.");
         }
 
+        if (feedbackRequest.isDenied() && (feedbackRequestUpdateDTO.getReason() == null || feedbackRequestUpdateDTO.getReason().trim().isEmpty())) {
+            throw new BadArgException("A reason must be provided for denying the request.");
+        }
+
         FeedbackRequest storedRequest = feedbackReqRepository.update(feedbackRequest);
         MemberProfile reviewer = memberProfileServices.getById(storedRequest.getRecipientId());
         MemberProfile requestee = memberProfileServices.getById(storedRequest.getRequesteeId());
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestUpdateDTO.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestUpdateDTO.java
@@ -4,6 +4,7 @@
 import io.micronaut.core.annotation.Nullable;
 import io.swagger.v3.oas.annotations.media.Schema;
 import jakarta.validation.constraints.NotNull;
+import com.objectcomputing.checkins.services.feedback_request.DTO.UserDTO;
 import lombok.Getter;
 import lombok.Setter;
 
diff --git a/server/src/main/java/com/objectcomputing/checkins/services/notification/NotificationServiceImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/notification/NotificationServiceImpl.java
@@ -4,6 +4,7 @@
 import com.objectcomputing.checkins.notifications.email.EmailSender;
 import com.objectcomputing.checkins.services.memberprofile.MemberProfile;
 import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices;
+import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
 import jakarta.inject.Singleton;
@@ -21,14 +22,17 @@ public class NotificationServiceImpl implements NotificationService {
 
     private final EmailSender emailSender;
     private final MemberProfileServices memberProfileServices;
+    private final CurrentUserServices currentUserServices;
 
     @Inject
     public NotificationServiceImpl(
             @Named(MailJetFactory.HTML_FORMAT) EmailSender emailSender,
-            MemberProfileServices memberProfileServices
+            MemberProfileServices memberProfileServices,
+            CurrentUserServices currentUserServices
     ) {
         this.emailSender = emailSender;
         this.memberProfileServices = memberProfileServices;
+        this.currentUserServices = currentUserServices;
     }
 
     @Override
@@ -57,9 +61,6 @@ public void sendNotification(@NonNull UUID userId, @NonNull String message) {
     }
 
     private MemberProfile getCurrentDenier() {
-        // This method should fetch the profile of the user who denied the request
-        // This could be passed as a parameter or retrieved from the session or context
-        // If not available, you might need to redesign how this information is passed to the service
-        throw new UnsupportedOperationException("Not implemented");
+        return currentUserServices.getCurrentUser();
     }
 }
diff --git a/web-ui/src/api/feedback.js b/web-ui/src/api/feedback.js
@@ -131,7 +131,10 @@ export const denyFeedbackRequest = async (requestId, reason, denier, creator, co
     data: { 
       reason: reason, 
       denier: denier, 
-      creator: creator,
+      creator: {
+        id: creator.id,
+        name: "Anonymous"
+      }
     },
     headers: {
       'X-CSRF-Header': cookie,
