Prevent overwriting completed manager scores

Author: Eric-Guo

app/controllers/staff/mark_scores_controller.rb

@@ -58,7 +58,9 @@ def show
 
     def update
       manager = authorize(policy_scope(User).find(params[:id]), :mark_scores?)
-      need_update_evaluations = policy_scope(EvaluationUserCapability).where(id: save_params[:mark_score].collect { |ms| ms[:id_euc] })
+      need_update_evaluations = policy_scope(EvaluationUserCapability)
+        .where(id: save_params[:mark_score].collect { |ms| ms[:id_euc] })
+        .where(form_status: "self_assessment_done")
       need_update_evaluations.each do |euc|
         p = save_params[:mark_score].find { |ms| ms[:id_euc] == euc.id }.to_h
         update_h = p.select { |key, value| !key.start_with?("p_") && value != "none" && key != "id_cet" && key != "id_euc" && key != "id_user" }
@@ -104,7 +106,9 @@ def update
 
     def score_confirm
       authorize(policy_scope(User).find(params[:id]), :score_confirm?)
-      evaluation_user_capabilities = policy_scope(EvaluationUserCapability).where(id: params[:euc_ids])
+      evaluation_user_capabilities = policy_scope(EvaluationUserCapability)
+        .where(id: params[:euc_ids])
+        .where(form_status: "self_assessment_done")
       evaluation_user_capabilities.each do |evaluation_user_capability|
         evaluation_user_capability.update_form_status_to("manager_scored", current_user)
         evaluation_user_capability.update_columns(

app/javascript/react/modal_dialog/MarkScoreConfirmDialog.jsx

@@ -5,6 +5,8 @@ import {scoreConfirmPath} from "../utils/url";
 
 export function MarkScoreConfirmDialog({accepted, message, euc_ids, onClose}) {
   const modalRef = React.useRef();
+  const submittingRef = React.useRef(false);
+  const [submitting, setSubmitting] = React.useState(false);
   React.useEffect(() => {
     const modal = new coreui.Modal('#coreuiModal');
     modal.show();
@@ -15,13 +17,19 @@ export function MarkScoreConfirmDialog({accepted, message, euc_ids, onClose}) {
 
   const handleConfirm = (event) => {
     event.preventDefault();
+    if (submittingRef.current) return;
+
+    submittingRef.current = true;
+    setSubmitting(true);
     put(scoreConfirmPath(userId()), {body: {euc_ids}}).then((response) => {
-      if (response.ok) {
-        const result_json = response.json;
-        result_json.then(result => {
-          window.location.href = result.go_path;
-        });
-      }
+      if (!response.ok) throw new Error("Failed to confirm mark scores");
+
+      return response.json;
+    }).then(result => {
+      window.location.href = result.go_path;
+    }).catch(() => {
+      submittingRef.current = false;
+      setSubmitting(false);
     });
   }
 
@@ -36,7 +44,7 @@ export function MarkScoreConfirmDialog({accepted, message, euc_ids, onClose}) {
           {message}
         </div>
         <div className="modal-footer">
-          {accepted ? <button onClick={handleConfirm} className="btn btn-primary">{reviewLabels().submit}</button> : null}
+          {accepted ? <button onClick={handleConfirm} className="btn btn-primary" disabled={submitting}>{reviewLabels().submit}</button> : null}
           <button className="btn btn-secondary" type="button" data-coreui-dismiss="modal">{reviewLabels().close}</button>
         </div>
       </div>

test/controllers/staff/mark_scores_controller_test.rb

@@ -137,6 +137,33 @@ class Staff::MarkScoresControllerTest < ActionDispatch::IntegrationTest
     assert_equal [4], response_body.fetch("need_review_evaluations").pluck("mark_score_group").uniq
   end
 
+  test "save does not overwrite a review after manager scoring is complete" do
+    euc = evaluation_user_capabilities(:euc_supervisor_high)
+    euc.update!(
+      form_status: "manager_scored",
+      work_quality: 3,
+      work_load: 3,
+      work_attitude: 3
+    )
+
+    put staff_mark_score_path(@manager, format: :json), params: {
+      company_evaluation_ids: [company_evaluations(:ce_one).id],
+      group_level: "supervisor",
+      mark_score_group: "4",
+      mark_score: [{
+        id_user: euc.user_id,
+        id_cet: euc.company_evaluation_template_id,
+        id_euc: euc.id,
+        work_quality: 0,
+        work_load: 0,
+        work_attitude: 0
+      }]
+    }, as: :json
+
+    assert_response :success
+    assert_equal [3, 3, 3], euc.reload.values_at(:work_quality, :work_load, :work_attitude).map(&:to_i)
+  end
+
   test "score confirm returns mark scores path when reviews remain" do
     euc = evaluation_user_capabilities(:euc_supervisor_high)
     company_evaluation_ids = [company_evaluations(:ce_one).id]
@@ -168,4 +195,31 @@ class Staff::MarkScoresControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_equal staff_root_path, JSON.parse(response.body).fetch("go_path")
   end
+
+  test "score confirm is idempotent after manager scoring is complete" do
+    euc = evaluation_user_capabilities(:euc_supervisor_high)
+    company_evaluation_ids = [company_evaluations(:ce_one).id]
+
+    put score_confirm_staff_mark_score_path(@manager, format: :json), params: {
+      euc_ids: [euc.id],
+      company_evaluation_ids: company_evaluation_ids
+    }, as: :json
+
+    assert_response :success
+    assert_equal 1, euc.euc_form_status_histories.where(
+      previous_form_status: "self_assessment_done",
+      form_status: "manager_scored"
+    ).count
+
+    put score_confirm_staff_mark_score_path(@manager, format: :json), params: {
+      euc_ids: [euc.id],
+      company_evaluation_ids: company_evaluation_ids
+    }, as: :json
+
+    assert_response :success
+    assert_equal 1, euc.euc_form_status_histories.where(
+      previous_form_status: "self_assessment_done",
+      form_status: "manager_scored"
+    ).count
+  end
 end