Merge branch 'develop' into feature-2647/volunteer-tracking-bugfixes

Author: mkimberlin

server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentServicesImpl.java

@@ -43,8 +43,12 @@ public ReviewAssignment save(ReviewAssignment reviewAssignment) {
         return newAssignment;
     }
 
+    // Now that uniqueness constraints have been placed on the
+    // reviewee-reviewer-reviewPeriod, this method needs to be synchronized
+    // to avoid multiple calls from the client-side overlapping and attempting
+    // to create the same review assignments multiple times.
     @Override
-    public List<ReviewAssignment> saveAll(UUID reviewPeriodId, List<ReviewAssignment> reviewAssignments, boolean deleteExisting) {
+    public synchronized List<ReviewAssignment> saveAll(UUID reviewPeriodId, List<ReviewAssignment> reviewAssignments, boolean deleteExisting) {
 
         if(deleteExisting) {
             LOG.warn("Deleting all review assignments for review period {}", reviewPeriodId);

server/src/main/java/com/objectcomputing/checkins/services/role/member_roles/MemberRoleController.java

@@ -11,7 +11,7 @@
 import java.util.List;
 import java.util.UUID;
 
-@Secured(SecurityRule.IS_ANONYMOUS)
+@Secured(SecurityRule.IS_AUTHENTICATED)
 @ExecuteOn(TaskExecutors.BLOCKING)
 @Controller("/services/roles/members")
 public class MemberRoleController {

server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java

@@ -16,28 +16,26 @@
 @JsonDeserialize(using = SettingOptionDeserializer.class)
 public enum SettingOption {
     LOGO_URL("The logo url", Category.THEME, Type.FILE),
-    PULSE_EMAIL_FREQUENCY("The Pulse Email Frequency (weekly, bi-weekly, monthly)", Category.CHECK_INS, Type.STRING);
+    PULSE_EMAIL_FREQUENCY("The Pulse Email Frequency", Category.CHECK_INS, Type.STRING, List.of("weekly", "bi-weekly", "monthly"));
 
     private final String description;
     private final Category category;
     private final Type type;
+    private final List<String> values;
 
     SettingOption(String description, Category category, Type type) {
         this.description = description;
         this.category = category;
         this.type = type;
+        this.values = List.of();
     }
 
-    public String getDescription() {
-        return description;
-    }
-
-    public Category getCategory() {
-        return category;
-    }
-
-    public Type getType() {
-        return type;
+    SettingOption(String description, Category category, Type type,
+                  List<String> values) {
+        this.description = description;
+        this.category = category;
+        this.type = type;
+        this.values = values;
     }
 
     public static List<SettingOption> getOptions(){

server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOptionSerializer.java

@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 
 import java.io.IOException;
+import java.util.List;
 
 public class SettingOptionSerializer extends StdSerializer<SettingOption> {
 
@@ -28,6 +29,15 @@ public void serialize(
         generator.writeString(settingOption.getCategory().name());
         generator.writeFieldName("type");
         generator.writeString(settingOption.getType().name());
+        List<String> values = settingOption.getValues();
+        if (!values.isEmpty()) {
+          generator.writeFieldName("values");
+          generator.writeStartArray(values.size());
+          for(String value : values) {
+            generator.writeString(value);
+          }
+          generator.writeEndArray();
+        }
         generator.writeEndObject();
     }
-}
+}

server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java

@@ -76,10 +76,8 @@ public SettingsResponseDTO findByName(@PathVariable @NotNull String name) {
     public List<SettingsResponseDTO> getOptions() {
         List<SettingOption> options = SettingOption.getOptions();
         return options.stream().map(option -> {
-                   // Default to an empty value and "invalid" UUID.
-                   // This can be used by the client to determine pre-existance.
                    String value = "";
-                   UUID uuid = new UUID(0, 0);
+                   UUID uuid = null;
                    try {
                        Setting s = settingsServices.findByName(option.name());
                        uuid = s.getId();
@@ -89,6 +87,7 @@ public List<SettingsResponseDTO> getOptions() {
                    return new SettingsResponseDTO(
                                   uuid, option.name(), option.getDescription(),
                                   option.getCategory(), option.getType(),
+                                  option.getValues(),
                                   value);
                }).toList();
     }
@@ -150,6 +149,7 @@ private SettingsResponseDTO fromEntity(Setting entity) {
         dto.setDescription(option.getDescription());
         dto.setCategory(option.getCategory());
         dto.setType(option.getType());
+        dto.setValues(option.getValues());
         dto.setValue(entity.getValue());
         return dto;
     }

server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsResponseDTO.java

@@ -9,6 +9,7 @@
 import lombok.Setter;
 
 import java.util.UUID;
+import java.util.List;
 
 @Setter
 @Getter
@@ -36,6 +37,10 @@ public class SettingsResponseDTO {
     @Schema(description = "type of the setting")
     private SettingOption.Type type;
 
+    @NotNull
+    @Schema(description = "possible values for the setting")
+    private List<String> values;
+
     @NotBlank
     @Schema(description = "value of the setting")
     private String value;

server/src/main/resources/db/common/V115__review_assignments_modify_primary_key.sql

@@ -0,0 +1,7 @@
+DELETE FROM review_assignments o USING review_assignments n
+WHERE o.id < n.id AND (o.reviewee_id = n.reviewee_id AND
+                       o.reviewer_id = n.reviewer_id AND
+                       o.review_period_id = n.review_period_id);
+
+ALTER TABLE review_assignments
+ADD CONSTRAINT unique_assignment UNIQUE (reviewee_id, reviewer_id, review_period_id)

server/src/main/resources/db/dev/R__Load_testing_data.sql

@@ -1291,12 +1291,12 @@ VALUES
 INSERT INTO review_periods
 (id, name, review_status, review_template_id, self_review_template_id, launch_date, self_review_close_date, close_date, period_start_date, period_end_date)
 VALUES
-    ('12345678-e29c-4cf4-9ea4-6baa09405c57', 'Review Period 1', 'PLANNING', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-09-01', '2024-09-02', '2024-09-03', '2024-01-01', '2024-12-31');
+    ('12345678-e29c-4cf4-9ea4-6baa09405c57', 'Review Period 1', 'PLANNING', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-09-01 06:00:00', '2024-09-02 06:00:00', '2024-09-03 06:00:00', '2024-01-01 06:00:00', '2024-08-31 06:00:00');
 
 INSERT INTO review_periods
 (id, name, review_status, review_template_id, self_review_template_id, launch_date, self_review_close_date, close_date, period_start_date, period_end_date)
 VALUES
-    ('12345678-e29c-4cf4-9ea4-6baa09405c58', 'Review Period 2', 'PLANNING', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-09-10', '2024-09-11', '2024-09-12', '2024-01-01', '2024-12-31');
+    ('12345678-e29c-4cf4-9ea4-6baa09405c58', 'Review Period 2', 'PLANNING', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-09-10 06:00:00', '2024-09-11 06:00:00', '2024-09-12 06:00:00', '2024-01-01 06:00:00', '2024-08-31 06:00:00');
 
 -- CAE - Self-Review feedback request, Creator: Big Boss
 INSERT INTO feedback_requests

server/src/test/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentControllerTest.java

@@ -146,6 +146,11 @@ void testGETFindAssignmentsByPeriodIdDefaultAssignments() {
 
         final HttpResponse<Set<ReviewAssignment>> response = client.toBlocking().exchange(request, Argument.setOf(ReviewAssignment.class));
 
+        // A review period only has default review assignments added to it when
+        // the review period is created through the controller.  And, they are
+        // no longer added to the review period when retrieving the review
+        // assignments for a specific review period.  Therefore, this review
+        // period should have zero review assignments associated with it.
         assertNotNull(response.body());
         assertEquals(0, Objects.requireNonNull(response.body()).size());
     }
@@ -283,4 +288,4 @@ void deleteReviewAssignmentWithoutPermissions() {
         assertNotNull(responseException.getResponse());
         assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus());
     }
-}
+}

web-ui/src/api/reviewassignments.js

@@ -0,0 +1,44 @@
+import { resolve } from './api.js';
+
+const reviewAssignmentsUrl = '/services/review-assignments';
+
+export const getReviewAssignments = async (id, cookie) => {
+  return resolve({
+    url: `${reviewAssignmentsUrl}/period/${id}`,
+    headers: { 'X-CSRF-Header': cookie, Accept: 'application/json' }
+  });
+};
+
+export const createReviewAssignments = async (id, assignments, cookie) => {
+  return resolve({
+    method: 'POST',
+    url: `${reviewAssignmentsUrl}/${id}`,
+    data: assignments,
+    headers: {
+      'X-CSRF-Header': cookie,
+      Accept: 'application/json',
+      'Content-Type': 'application/json;charset=UTF-8'
+    }
+  });
+};
+
+export const updateReviewAssignment = async (assignment, cookie) => {
+  return resolve({
+    method: assignment.id === null ? 'POST' : 'PUT',
+    url: reviewAssignmentsUrl,
+    data: assignment,
+    headers: {
+      'X-CSRF-Header': cookie,
+      Accept: 'application/json',
+      'Content-Type': 'application/json;charset=UTF-8'
+    }
+  });
+};
+
+export const removeReviewAssignment = async (id, cookie) => {
+  return resolve({
+    method: 'DELETE',
+    url: `${reviewAssignmentsUrl}/${id}`,
+    headers: { 'X-CSRF-Header': cookie }
+  });
+};