Feature: Add option to retain old grading data on recollection (#7256)

Changelog.md

@@ -10,6 +10,7 @@
 - Add visual indicator on a per-assignment basis for used grace credits (#7226)
 - Change default disabled text area colour to ligher grey on light mode (#7269)
 - Implement a limit on the file size rendered by the submission viewer (#7273)
+- Add an option to retain old grading data when recollecting graded submissions (#7256)
 
 ### 🐛 Bug fixes
 

app/assets/javascripts/Components/Modals/collect_submissions_modal.jsx

@@ -12,6 +12,7 @@ class CollectSubmissionsModal extends React.Component {
       override: this.props.override,
       collect_time: this.props.isScannedExam ? "collect_current" : "collect_due_date",
       apply_late_penalty: !this.props.isScannedExam,
+      retain_existing_grading: true,
     };
   }
 
@@ -25,14 +26,19 @@ class CollectSubmissionsModal extends React.Component {
       this.state.override,
       this.state.collect_time === "collect_current",
       // Always apply late penalty when collecting based on due date
-      this.state.apply_late_penalty || this.state.collect_time === "collect_due_date"
+      this.state.apply_late_penalty || this.state.collect_time === "collect_due_date",
+      this.state.override && this.state.retain_existing_grading
     );
   };
 
   handleOverrideChange = event => {
     this.setState({override: event.target.checked});
   };
 
+  handleRetainExistingGradingChange = event => {
+    this.setState({retain_existing_grading: event.target.checked});
+  };
+
   handleCollectTimeChange = event => {
     this.setState({collect_time: event.target.value});
   };
@@ -96,15 +102,47 @@ class CollectSubmissionsModal extends React.Component {
               <legend>{I18n.t("submissions.collect.collection_options")}</legend>
               <p>
                 <label>
-                  <input type="checkbox" name="override" onChange={this.handleOverrideChange} />
-                  &nbsp;
-                  <span
-                    dangerouslySetInnerHTML={{
-                      __html: I18n.t("submissions.collect.override_existing_html"),
-                    }}
+                  <input
+                    type="checkbox"
+                    defaultChecked={this.state.override}
+                    name="override"
+                    data-testid="chk_recollect_existing_submissions"
+                    onChange={this.handleOverrideChange}
                   />
+                  &nbsp;
+                  <span data-testid="lbl_recollect_existing_submissions">
+                    {I18n.t("submissions.collect.override_existing")}
+                  </span>
                 </label>
               </p>
+              {this.state.override && (
+                <p style={{marginLeft: "15px"}}>
+                  <input
+                    type="checkbox"
+                    defaultChecked={this.state.retain_existing_grading}
+                    name="retain_existing_grading"
+                    id="retain_existing_grading"
+                    data-testid="chk_retain_existing_grading"
+                    onChange={this.handleRetainExistingGradingChange}
+                  />
+                  &nbsp;
+                  <label
+                    htmlFor="retain_existing_grading"
+                    data-testid="lbl_retain_existing_grading"
+                  >
+                    {I18n.t("submissions.collect.retain_existing_grading")}
+                  </label>
+                  {!this.state.retain_existing_grading && (
+                    <div
+                      data-testid="div_grading_data_will_be_lost"
+                      className="warning"
+                      style={{marginTop: "4px"}}
+                    >
+                      {I18n.t("submissions.collect.grading_data_will_be_lost")}
+                    </div>
+                  )}
+                </p>
+              )}
               {this.state.collect_time === "collect_current" && !this.props.isScannedExam && (
                 <p>
                   <label>
@@ -121,7 +159,11 @@ class CollectSubmissionsModal extends React.Component {
               )}
             </fieldset>
             <section className={"modal-container dialog-actions"}>
-              <input type="submit" value={I18n.t("submissions.collect.submit")} />
+              <input
+                type="submit"
+                data-testid="btn_collect_submissions"
+                value={I18n.t("submissions.collect.submit")}
+              />
             </section>
           </div>
         </form>

app/assets/javascripts/Components/__tests__/collect_submissions_modal.test.jsx

@@ -0,0 +1,72 @@
+import {render, screen, fireEvent, waitFor} from "@testing-library/react";
+import CollectSubmissionsModal from "../Modals/collect_submissions_modal";
+import Modal from "react-modal";
+
+describe("CollectSubmissionsModal", () => {
+  let props;
+
+  beforeEach(() => {
+    props = {
+      isOpen: true,
+      isScannedExam: false,
+      onRequestClose: jest.fn(),
+      onSubmit: jest.fn(),
+    };
+
+    // Set the app element for React Modal
+    Modal.setAppElement("body");
+    render(<CollectSubmissionsModal {...props} />);
+  });
+
+  it("should display the option to recollect old submissions unchecked by default", () => {
+    const lblRecollectExistingSubmissions = screen.getByTestId(
+      "lbl_recollect_existing_submissions"
+    );
+    const chkRecollectExistingSubmissions = screen.getByTestId(
+      "chk_recollect_existing_submissions"
+    );
+
+    expect(lblRecollectExistingSubmissions).toBeInTheDocument();
+    expect(chkRecollectExistingSubmissions).toBeInTheDocument();
+    expect(chkRecollectExistingSubmissions.checked).toBe(false);
+  });
+
+  describe("when the option to recollect checkbox is checked", () => {
+    beforeEach(() => {
+      fireEvent.click(screen.getByTestId("chk_recollect_existing_submissions"));
+    });
+
+    it("should display the option to retain existing grading checked by default", () => {
+      const lblRetainExistingGrading = screen.getByTestId("lbl_retain_existing_grading");
+      const chkRetainExistingGrading = screen.getByTestId("chk_retain_existing_grading");
+
+      expect(lblRetainExistingGrading).toBeInTheDocument();
+      expect(chkRetainExistingGrading).toBeInTheDocument();
+      expect(chkRetainExistingGrading.checked).toBe(true);
+    });
+
+    it("should display a warning when the retain existing grading option is unchecked", () => {
+      const chkRetainExistingGrading = screen.getByTestId("chk_retain_existing_grading");
+
+      fireEvent.click(chkRetainExistingGrading);
+
+      const divGradingDataWillBeLost = screen.getByTestId("div_grading_data_will_be_lost");
+      const lblRetainExistingGrading = screen.queryByTestId("lbl_retain_existing_grading");
+
+      expect(chkRetainExistingGrading.checked).toBe(false);
+      expect(divGradingDataWillBeLost).toBeInTheDocument();
+      expect(lblRetainExistingGrading).toBeInTheDocument();
+    });
+
+    it("should call onSubmit with the correct parameters when the form is submitted", async () => {
+      const btnCollectSubmissions = screen.getByTestId("btn_collect_submissions");
+
+      fireEvent.click(btnCollectSubmissions);
+
+      await waitFor(() => {
+        expect(props.onSubmit).toHaveBeenCalledTimes(1);
+        expect(props.onSubmit).toHaveBeenCalledWith(true, false, true, true);
+      });
+    });
+  });
+});

app/assets/javascripts/Components/__tests__/repo_browser_manual_collection_form.test.jsx

@@ -0,0 +1,66 @@
+import {render, screen, fireEvent} from "@testing-library/react";
+import {ManualCollectionForm} from "../repo_browser";
+
+jest.mock("@fortawesome/react-fontawesome", () => ({
+  FontAwesomeIcon: () => {
+    return null;
+  },
+}));
+
+describe("RepoBrowser's ManualCollectionForm", () => {
+  let props, component;
+
+  beforeEach(() => {
+    props = {
+      course_id: 1,
+      assignment_id: 2,
+      late_penalty: false,
+      grouping_id: 1,
+      revision_identifier: "test",
+      collected_revision_id: "test",
+    };
+
+    component = render(<ManualCollectionForm {...props} />);
+  });
+
+  it("shows the option to retain existing grading when there is a collected revision present", () => {
+    const lblRecollectExistingSubmissions = screen.getByTestId("lbl_retain_existing_grading");
+    const chkRecollectExistingSubmissions = screen.getByTestId("chk_retain_existing_grading");
+
+    expect(lblRecollectExistingSubmissions).toBeVisible();
+    expect(chkRecollectExistingSubmissions).toBeVisible();
+  });
+
+  it("does not show the option to retain existing grading when there is a not collected revision present", () => {
+    props.collected_revision_id = "";
+    component.rerender(<ManualCollectionForm {...props} />);
+
+    const lblRecollectExistingSubmissions = screen.queryByTestId("lbl_retain_existing_grading");
+    const chkRecollectExistingSubmissions = screen.queryByTestId("chk_retain_existing_grading");
+
+    expect(lblRecollectExistingSubmissions).not.toBeVisible();
+    expect(chkRecollectExistingSubmissions).not.toBeVisible();
+  });
+
+  it("should confirm with a full overwrite warning when retain existing grading option is checked", () => {
+    const confirmSpy = jest.spyOn(window, "confirm").mockImplementation(() => false);
+    const manualCollectionForm = component.getByTestId("form_manual_collection");
+
+    fireEvent.submit(manualCollectionForm);
+
+    expect(confirmSpy).toHaveBeenCalledWith(
+      I18n.t("submissions.collect.confirm_recollect_retain_data")
+    );
+  });
+
+  it("should confirm with a full overwrite warning when retain existing grading option is not checked", () => {
+    const confirmSpy = jest.spyOn(window, "confirm").mockImplementation(() => false);
+    const chkRecollectExistingSubmissions = screen.queryByTestId("chk_retain_existing_grading");
+    const manualCollectionForm = component.getByTestId("form_manual_collection");
+
+    fireEvent.click(chkRecollectExistingSubmissions);
+    fireEvent.submit(manualCollectionForm);
+
+    expect(confirmSpy).toHaveBeenCalledWith(I18n.t("submissions.collect.full_overwrite_warning"));
+  });
+});

app/assets/javascripts/Components/__tests__/starter_file_manager.test.jsx

@@ -9,12 +9,6 @@ jest.mock("@fortawesome/react-fontawesome", () => ({
   },
 }));
 
-// workaround needed for using i18n in jest tests, see
-// https://github.com/fnando/i18n/issues/26#issuecomment-1235751777
-jest.mock("i18n-js", () => {
-  return jest.requireActual("i18n-js/dist/require/index");
-});
-
 [true, false].forEach(readOnly => {
   let container;
   describe(`When a user ${

app/assets/javascripts/Components/repo_browser.jsx

@@ -87,6 +87,7 @@ class RepoBrowser extends React.Component {
           late_penalty={this.props.late_penalty}
           grouping_id={this.props.grouping_id}
           revision_identifier={this.state.revision_identifier}
+          collected_revision_id={this.props.collected_revision_id}
         />
       );
     }
@@ -138,6 +139,13 @@ class ManualCollectionForm extends React.Component {
     revision_identifier: "", //set initial value so that the input (in render) remains controlled
   };
 
+  constructor(props) {
+    super(props);
+    this.state = {
+      retainExistingGrading: true,
+    };
+  }
+
   render() {
     const action = Routes.manually_collect_and_begin_grading_course_assignment_submissions_path(
       this.props.course_id,
@@ -149,7 +157,22 @@ class ManualCollectionForm extends React.Component {
         <legend>
           <span>{I18n.t("submissions.collect.manual_collection")}</span>
         </legend>
-        <form method="POST" action={action}>
+        <form
+          method="POST"
+          action={action}
+          data-testid="form_manual_collection"
+          onSubmit={event => {
+            if (
+              this.props.collected_revision_id &&
+              ((!this.state.retainExistingGrading &&
+                !confirm(I18n.t("submissions.collect.full_overwrite_warning"))) ||
+                (this.state.retainExistingGrading &&
+                  !confirm(I18n.t("submissions.collect.confirm_recollect_retain_data"))))
+            ) {
+              event.preventDefault();
+            }
+          }}
+        >
           <input
             type="hidden"
             name="current_revision_identifier"
@@ -168,15 +191,28 @@ class ManualCollectionForm extends React.Component {
               {I18n.t("submissions.collect.apply_late_penalty")}
             </label>
           </p>
-          <button
-            type="submit"
-            name="commit"
-            onClick={event => {
-              if (!confirm(I18n.t("submissions.collect.overwrite_warning"))) {
-                event.preventDefault();
-              }
-            }}
-          >
+          <p className="inline-labels">
+            <input
+              type="checkbox"
+              name="retain_existing_grading"
+              id="retain_existing_grading"
+              data-testid="chk_retain_existing_grading"
+              checked={this.state.retainExistingGrading}
+              hidden={!this.props.collected_revision_id}
+              disabled={!this.props.collected_revision_id} // prevent from sending info on submit
+              onChange={e => {
+                this.setState({retainExistingGrading: e.target.checked});
+              }}
+            />
+            <label
+              data-testid="lbl_retain_existing_grading"
+              htmlFor="retain_existing_grading"
+              hidden={!this.props.collected_revision_id}
+            >
+              {I18n.t("submissions.collect.retain_existing_grading")}
+            </label>
+          </p>
+          <button type="submit" name="commit">
             <FontAwesomeIcon icon="fa-solid fa-file-import" />
             {I18n.t("submissions.collect.this_revision")}
           </button>
@@ -189,3 +225,5 @@ class ManualCollectionForm extends React.Component {
 export function makeRepoBrowser(elem, props) {
   render(<RepoBrowser {...props} />, elem);
 }
+
+export {ManualCollectionForm};

app/assets/javascripts/Components/submission_table.jsx

@@ -283,7 +283,7 @@ class RawSubmissionTable extends React.Component {
   };
 
   // Submission table actions
-  collectSubmissions = (override, collect_current, apply_late_penalty) => {
+  collectSubmissions = (override, collect_current, apply_late_penalty, retain_existing_grading) => {
     this.setState({showCollectSubmissionsModal: false});
     $.post({
       url: Routes.collect_submissions_course_assignment_submissions_path(
@@ -295,6 +295,7 @@ class RawSubmissionTable extends React.Component {
         override: override,
         collect_current: collect_current,
         apply_late_penalty: apply_late_penalty,
+        retain_existing_grading: retain_existing_grading,
       },
     });
   };

app/controllers/submissions_controller.rb

@@ -172,9 +172,16 @@ def manually_collect_and_begin_grading
                          else
                            params[:apply_late_penalty]
                          end
+    retain_existing_grading = if params[:retain_existing_grading].nil?
+                                false
+                              else
+                                params[:retain_existing_grading]
+                              end
+
     SubmissionsJob.perform_now([@grouping],
                                apply_late_penalty: apply_late_penalty,
-                               revision_identifier: @revision_identifier)
+                               revision_identifier: @revision_identifier,
+                               retain_existing_grading: retain_existing_grading)
 
     submission = @grouping.reload.current_submission_used
     redirect_to edit_course_result_path(course_id: current_course.id, id: submission.get_latest_result.id)
@@ -198,6 +205,7 @@ def collect_submissions
     end
     collect_current = params[:collect_current] == 'true'
     apply_late_penalty = params[:apply_late_penalty] == 'true'
+    retain_existing_grading = params[:retain_existing_grading] == 'true'
     assignment = Assignment.includes(:groupings).find(params[:assignment_id])
     groupings = assignment.groupings.find(params[:groupings])
     collectable = []
@@ -225,7 +233,8 @@ def collect_submissions
                                                  collection_dates: collection_dates.transform_keys(&:to_s),
                                                  collect_current: collect_current,
                                                  apply_late_penalty: apply_late_penalty,
-                                                 notify_socket: true)
+                                                 notify_socket: true,
+                                                 retain_existing_grading: retain_existing_grading)
       CollectSubmissionsChannel.broadcast_to(@current_user, ActiveJob::Status.get(current_job).to_h)
     end
     if some_before_due