Merge pull request #2662 from objectcomputing/bugfix-2653/self-review…

…-accessibility Bugfix 2653/self review accessibility
objectcomputing · Oct 25, 2024 · bad46b9 · bad46b9
2 parents 113f74b + 02d8037
commit bad46b9
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 29 deletions.
diff --git a/server/src/main/resources/db/dev/R__Load_testing_data.sql b/server/src/main/resources/db/dev/R__Load_testing_data.sql
@@ -1298,22 +1298,32 @@ INSERT INTO review_periods
 VALUES
     ('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');
 
+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-6baa09405c59', 'Review Period 3', 'CLOSED', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-10-01 06:00:00', '2024-10-02 06:00:00', '2024-10-03 06:00:00', '2024-01-01 06:00:00', '2024-09-30 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-6baa09405c5a', 'Review Period 4', 'CLOSED', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-10-01 06:00:00', '2024-10-02 06:00:00', '2024-10-03 06:00:00', '2024-01-01 06:00:00', '2024-09-30 06:00:00');
+
 -- CAE - Self-Review feedback request, Creator: Big Boss
 INSERT INTO feedback_requests
 (id, creator_id, requestee_id, recipient_id, template_id, send_date, due_date, submit_date, status, review_period_id)
 VALUES
-('98390c09-7121-110a-bfee-9380a470a7ef', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'c7406157-a38f-4d48-aaed-04018d846727', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-09-04', '2024-09-30', '2024-09-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c57');
+('98390c09-7121-110a-bfee-9380a470a7ef', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'c7406157-a38f-4d48-aaed-04018d846727', '926a37a4-4ded-4633-8900-715b0383aecc', '2024-10-04', '2024-10-30', '2024-10-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c59');
 
 -- CAE - Review feedback request, Creator: Big Boss
 INSERT INTO feedback_requests
 (id, creator_id, requestee_id, recipient_id, template_id, send_date, due_date, submit_date, status, review_period_id)
 VALUES
-('98390c09-7121-110a-bfee-9380a470a7f0', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', '6207b3fd-042d-49aa-9e28-dcc04f537c2d', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '2024-09-04', '2024-09-30', '2024-09-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c57');
+('98390c09-7121-110a-bfee-9380a470a7f0', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', '6207b3fd-042d-49aa-9e28-dcc04f537c2d', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '2024-10-04', '2024-10-30', '2024-10-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c59');
 
 INSERT INTO feedback_requests
 (id, creator_id, requestee_id, recipient_id, template_id, send_date, due_date, submit_date, status, review_period_id)
 VALUES
-('98390c09-7121-110a-bfee-9380a470a7f3', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'dfe2f986-fac0-11eb-9a03-0242ac130003', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '2024-09-04', '2024-09-30', '2024-09-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c57');
+('98390c09-7121-110a-bfee-9380a470a7f3', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'dfe2f986-fac0-11eb-9a03-0242ac130003', 'd1e94b60-47c4-4945-87d1-4dc88f088e57', '2024-10-04', '2024-10-30', '2024-10-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c59');
 
 INSERT INTO feedback_requests
 (id, creator_id, requestee_id, recipient_id, template_id, send_date, due_date, submit_date, status, review_period_id)
@@ -1344,7 +1354,7 @@ VALUES
 INSERT INTO feedback_requests
 (id, creator_id, requestee_id, recipient_id, template_id, send_date, due_date, submit_date, status, review_period_id)
 VALUES
-('98390c09-7121-110a-bfee-9380a470a7f2', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'dfe2f986-fac0-11eb-9a03-0242ac130003', '1c8bc142-c447-4889-986e-42ab177da683', '2024-09-04', '2024-09-30', '2024-09-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c57');
+('98390c09-7121-110a-bfee-9380a470a7f2', '72655c4f-1fb8-4514-b31e-7f7e19fa9bd7', 'c7406157-a38f-4d48-aaed-04018d846727', 'dfe2f986-fac0-11eb-9a03-0242ac130003', '1c8bc142-c447-4889-986e-42ab177da683', '2024-10-04', '2024-10-30', '2024-10-05', 'submitted', '12345678-e29c-4cf4-9ea4-6baa09405c59');
 
 ---- Creator: Big Boss
 INSERT INTO feedback_requests

diff --git a/web-ui/src/components/reviews/periods/ReviewPeriods.jsx b/web-ui/src/components/reviews/periods/ReviewPeriods.jsx
@@ -99,11 +99,15 @@ const ReviewStatus = {
   UNKNOWN: 'UNKNOWN'
 };
 
+// mode will be either "self" or undefined.
+const selfReviewMode = "self";
+
 const ReviewPeriods = ({ onPeriodSelected, mode }) => {
   const { state, dispatch } = useContext(AppContext);
 
   const [canSave, setCanSave] = useState(false);
   const [loading, setLoading] = useState(false);
+  const [periods, setPeriods] = useState([]);
   const [periodToAdd, setPeriodToAdd] = useState({
     name: '',
     reviewStatus: ReviewStatus.PLANNING,
@@ -119,9 +123,15 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
 
   const currentUserId = selectCurrentUserId(state);
   const csrf = selectCsrfToken(state);
-  const periods = selectReviewPeriods(state);
   const userProfile = selectUserProfile(state);
 
+  useEffect(() => {
+    setPeriods(selectReviewPeriods(state)
+                 .filter((r) => mode !== selfReviewMode ||
+                                r.reviewStatus === ReviewStatus.OPEN ||
+                                r.reviewStatus === ReviewStatus.CLOSED));
+  }, [state, mode]);
+
   useQueryParameters([
     {
       name: 'add',
@@ -255,6 +265,7 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
 
   useEffect(() => {
     const getSelfReviews = async () => {
+      setLoading(true);
       let reviews = {};
       Promise.all(
         periods.map(async period => {
@@ -280,7 +291,17 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
             });
           }
         })
-      ).then(() => setSelfReviews(reviews));
+      ).then(() => {
+               // Now that we have the reviews loaded, filter out closed
+               // self-review periods in which the current user is not involved.
+               if (mode == selfReviewMode) {
+                 setPeriods(periods.filter((r) =>
+                              r.reviewStatus !== ReviewStatus.CLOSED ||
+                              !!reviews[r.id]));
+               }
+               setSelfReviews(reviews);
+               setLoading(false);
+             });
     };
     if (
       csrf &&
@@ -291,18 +312,17 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
     ) {
       getSelfReviews();
     }
-  }, [csrf, periods, currentUserId, selfReviews]);
+  }, [csrf, periods, currentUserId, selfReviews ]);
 
   const onPeriodClick = useCallback(
     id => {
       const status = selectReviewPeriod(state, id)?.reviewStatus;
       switch (status) {
         case ReviewStatus.PLANNING:
-          onPeriodSelected(id);
-          break;
         case ReviewStatus.AWAITING_APPROVAL:
-          // TODO: Check for the required permission.
-          onPeriodSelected(id);
+          if (mode !== selfReviewMode) {
+            onPeriodSelected(id);
+          }
           break;
         case ReviewStatus.OPEN:
           onPeriodSelected(id);

diff --git a/web-ui/src/components/settings/types/boolean.jsx b/web-ui/src/components/settings/types/boolean.jsx
@@ -15,7 +15,8 @@ import { createLabelId } from '../../../helpers/strings.js';
  */
 const SettingsBoolean = ({ name, description, value, handleChange }) => {
   const labelId = createLabelId(name);
-
+  const checked =
+    typeof(value) === 'boolean' ? value : value.toLowerCase() == "true";
   return (
     <div className="settings-type">
       <label htmlFor={labelId}>
@@ -28,7 +29,7 @@ const SettingsBoolean = ({ name, description, value, handleChange }) => {
         id={labelId}
         className="settings-control"
         type="checkbox"
-        checked={value}
+        checked={checked}
         onChange={handleChange}
       />
     </div>

diff --git a/web-ui/src/pages/SettingsPage.jsx b/web-ui/src/pages/SettingsPage.jsx
@@ -46,7 +46,14 @@ const SettingsPage = () => {
       if (allOptions) {
         // Sort the options by category, store them, and upate the state.
         setSettingsControls(
-          allOptions.sort((l, r) => l.category.localeCompare(r.category)));
+          allOptions.sort((l, r) => {
+            if (l.category === r.category) {
+              return l.name.localeCompare(r.name);
+            } else {
+              return l.category.localeCompare(r.category);
+            }
+          })
+        );
       }
     };
     if (csrf) {
@@ -124,7 +131,7 @@ const SettingsPage = () => {
     for(let key of Object.keys(handlers)) {
       const setting = handlers[key].setting;
       // The settings controller does not allow blank values.
-      if (setting && setting.value) {
+      if (setting?.name && `${setting.value}` != "") {
         let res;
         if (setting.id) {
           res = await putOption({ name: setting.name,
@@ -133,7 +140,7 @@ const SettingsPage = () => {
           res = await postOption({ name: setting.name,
                                    value: setting.value }, csrf);
           if (res?.payload?.data) {
-            setting.exists = true;
+            setting.id = res.payload.data.id;
           }
         }
         if (res?.error) {
@@ -147,6 +154,8 @@ const SettingsPage = () => {
         if (res?.payload?.data) {
           saved++;
         }
+      } else {
+        console.warn(`WARNING: ${setting.name} not sent to the server`);
       }
     }
 

diff --git a/web-ui/src/pages/__snapshots__/SettingsPage.test.jsx.snap b/web-ui/src/pages/__snapshots__/SettingsPage.test.jsx.snap
@@ -38,10 +38,9 @@ exports[`SettingsPage > renders correctly 1`] = `
         class="MuiSwitch-root MuiSwitch-sizeMedium settings-control css-julti5-MuiSwitch-root"
       >
         <span
-          class="MuiButtonBase-root MuiSwitch-switchBase MuiSwitch-colorPrimary Mui-checked PrivateSwitchBase-root MuiSwitch-switchBase MuiSwitch-colorPrimary Mui-checked Mui-checked css-byenzh-MuiButtonBase-root-MuiSwitch-switchBase"
+          class="MuiButtonBase-root MuiSwitch-switchBase MuiSwitch-colorPrimary PrivateSwitchBase-root MuiSwitch-switchBase MuiSwitch-colorPrimary css-byenzh-MuiButtonBase-root-MuiSwitch-switchBase"
         >
           <input
-            checked=""
             class="PrivateSwitchBase-input MuiSwitch-input css-1m9pwf3"
             id="another-setting"
             type="checkbox"
@@ -68,12 +67,12 @@ exports[`SettingsPage > renders correctly 1`] = `
       class="settings-type"
     >
       <label
-        for="string-setting"
+        for="other-setting"
       >
         <h5
           class="MuiTypography-root MuiTypography-h5 MuiTypography-gutterBottom css-h93ljk-MuiTypography-root"
         >
-          String Setting
+          Other Setting
         </h5>
       </label>
       <p>
@@ -84,23 +83,22 @@ exports[`SettingsPage > renders correctly 1`] = `
       >
         <input
           class="MuiInputBase-input MuiInput-input css-1x51dt5-MuiInputBase-input-MuiInput-input"
-          id="string-setting"
-          placeholder="Enter String Setting"
-          type="text"
-          value="The value"
+          id="other-setting"
+          type="number"
+          value="42"
         />
       </div>
     </div>
     <div
       class="settings-type"
     >
       <label
-        for="other-setting"
+        for="string-setting"
       >
         <h5
           class="MuiTypography-root MuiTypography-h5 MuiTypography-gutterBottom css-h93ljk-MuiTypography-root"
         >
-          Other Setting
+          String Setting
         </h5>
       </label>
       <p>
@@ -111,9 +109,10 @@ exports[`SettingsPage > renders correctly 1`] = `
       >
         <input
           class="MuiInputBase-input MuiInput-input css-1x51dt5-MuiInputBase-input-MuiInput-input"
-          id="other-setting"
-          type="number"
-          value="42"
+          id="string-setting"
+          placeholder="Enter String Setting"
+          type="text"
+          value="The value"
         />
       </div>
     </div>