Skip to content

Commit

Permalink
test(retain-old-grading-data-option): further test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
pranavrao145 committed Oct 24, 2024
1 parent 40f488d commit 01e0d63
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 60 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as React from "react";
import {render, screen, fireEvent, waitFor} from "@testing-library/react";
import CollectSubmissionsModal from "../Modals/collect_submissions_modal";
import Modal from "react-modal";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
import * as React from "react";
import {render, screen} from "@testing-library/react";
import {render, screen, fireEvent} from "@testing-library/react";
import {ManualCollectionForm} from "../repo_browser";

// 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");
});

jest.mock("@fortawesome/react-fontawesome", () => ({
FontAwesomeIcon: () => {
return null;
},
}));

describe("RepoBrowser's ManualCollectionForm", () => {
let props, component;

Expand All @@ -15,16 +26,15 @@ describe("RepoBrowser's ManualCollectionForm", () => {
collected_revision_id: "test",
};

// Set the app element for React Modal
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).toBeInTheDocument();
expect(chkRecollectExistingSubmissions).toBeInTheDocument();
expect(lblRecollectExistingSubmissions).toBeVisible();
expect(chkRecollectExistingSubmissions).toBeVisible();
});

it("does not show the option to retain existing grading when there is a not collected revision present", () => {
Expand All @@ -34,7 +44,29 @@ describe("RepoBrowser's ManualCollectionForm", () => {
const lblRecollectExistingSubmissions = screen.queryByTestId("lbl_retain_existing_grading");
const chkRecollectExistingSubmissions = screen.queryByTestId("chk_retain_existing_grading");

expect(lblRecollectExistingSubmissions).not.toBeInTheDocument();
expect(chkRecollectExistingSubmissions).not.toBeInTheDocument();
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.partial_overwrite_warning")
);
});

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"));
});
});
67 changes: 36 additions & 31 deletions app/assets/javascripts/Components/repo_browser.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,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.partial_overwrite_warning"))))
) {
event.preventDefault();
}
}}
>
<input
type="hidden"
name="current_revision_identifier"
Expand All @@ -176,36 +191,26 @@ class ManualCollectionForm extends React.Component {
{I18n.t("submissions.collect.apply_late_penalty")}
</label>
</p>
{this.props.collected_revision_id && (
<div className="inline-labels" style={{marginBottom: "1em"}}>
<input
type="checkbox"
name="retain_existing_grading"
data-testid="chk_retain_existing_grading"
checked={this.state.retainExistingGrading}
onChange={e => {
this.setState({retainExistingGrading: e.target.checked});
}}
/>
<label data-testid="lbl_retain_existing_grading">
{I18n.t("submissions.collect.retain_existing_grading")}
</label>
</div>
)}
<button
type="submit"
name="commit"
onClick={event => {
if (
(!this.state.retainExistingGrading &&
!confirm(I18n.t("submissions.collect.full_overwrite_warning"))) ||
(this.state.retainExistingGrading &&
!confirm(I18n.t("submissions.collect.partial_overwrite_warning")))
) {
event.preventDefault();
}
}}
>
<p className="inline-labels">
<input
type="checkbox"
name="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"
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>
Expand Down
50 changes: 29 additions & 21 deletions spec/jobs/submissions_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,25 +191,25 @@
before do
# explicit storing and freezing here or it may implicitly reload this and
# we can't compare to the old one
@original_submissions = assignment.reload.current_submissions_used.to_a
@original_results = assignment.current_results.to_a
@original_submissions = assignment.reload.current_submissions_used.order('submissions.grouping_id').to_a
@original_results = assignment.current_results.order('submissions.grouping_id').to_a

SubmissionsJob.perform_now(assignment.groupings, retain_existing_grading: true)

@new_submissions = assignment.reload.current_submissions_used
@new_results = assignment.reload.current_results
@new_submissions = assignment.reload.current_submissions_used.order('submissions.grouping_id')
@new_results = assignment.reload.current_results.order('submissions.grouping_id')
end

it 'creates the correct number of new submissions' do
# still the same number of groupings => the same number of submissions
expect(@new_submissions.size).to eq(@original_submissions.size)
expect(@new_submissions.ids).not_to eq(@original_submissions.map(&:id))
expect(@new_submissions.ids.sort).not_to eq(@original_submissions.map(&:id).sort)
end

it 'creates the correct number of new results' do
# still the same number of submissions => the same number of results
expect(@new_results.size).to eq(@original_results.size)
expect(@new_results.ids).not_to eq(@original_results.map(&:id))
expect(@new_results.ids.sort).not_to eq(@original_results.map(&:id).sort)
end

context 'for feedback files on each new submission' do
Expand All @@ -218,13 +218,16 @@
it 'creates the correct number of new feedback files on each submission' do
@new_submissions.zip(@original_submissions).each do |new_submission, old_submission|
expect(new_submission.feedback_files.size).to eq(old_submission.feedback_files.size)
expect(new_submission.feedback_files.ids).not_to eq(old_submission.feedback_files.ids)
expect(new_submission.feedback_files.ids.sort).not_to eq(old_submission.feedback_files.ids.sort)
end
end

it 'retains the file name on each new feedback file' do
@new_submissions.zip(@original_submissions).each do |new_submission, old_submission|
expect(new_submission.feedback_files.map(&:filename)).to eq(old_submission.feedback_files.map(&:filename))
# cannot compare by order here because there is no common field between the two, so
# instead we check the set of filenames is equal
expect(new_submission.feedback_files.map(&:filename).sort).to eq(old_submission
.feedback_files.map(&:filename).sort)
end
end
end
Expand All @@ -235,7 +238,7 @@
it 'creates the correct number of new test runs' do
@new_submissions.zip(@original_submissions).each do |new_submission, old_submission|
expect(new_submission.test_runs.size).to eq(old_submission.test_runs.size)
expect(new_submission.test_runs.ids).not_to eq(old_submission.test_runs.ids)
expect(new_submission.test_runs.ids.sort).not_to eq(old_submission.test_runs.ids.sort)
end
end

Expand All @@ -244,7 +247,7 @@
@new_submissions.zip(@original_submissions).each do |new_submission, old_submission|
new_submission.test_runs.zip(old_submission.test_runs).each do |old_test_run, new_test_run|
expect(new_test_run.test_group_results.size).to eq(old_test_run.test_group_results.size)
expect(new_test_run.test_group_results.ids).not_to eq(old_test_run.test_group_results.ids)
expect(new_test_run.test_group_results.ids.sort).not_to eq(old_test_run.test_group_results.ids.sort)
end
end
end
Expand All @@ -255,7 +258,7 @@
new_submission.test_runs.zip(old_submission.test_runs).each do |old_test_run, new_test_run|
new_test_run.test_group_results.zip(old_test_run.test_group_results).each do |old_tgr, new_tgr|
expect(new_tgr.test_results.size).to eq(old_tgr.test_results.size)
expect(new_tgr.test_results.ids).not_to eq(old_tgr.test_results.ids)
expect(new_tgr.test_results.ids.sort).not_to eq(old_tgr.test_results.ids.sort)
end
end
end
Expand All @@ -266,7 +269,7 @@
new_submission.test_runs.zip(old_submission.test_runs).each do |old_test_run, new_test_run|
new_test_run.test_group_results.zip(old_test_run.test_group_results).each do |old_tgr, new_tgr|
expect(new_tgr.feedback_files.size).to eq(old_tgr.feedback_files.size)
expect(new_tgr.feedback_files.ids).not_to eq(old_tgr.feedback_files.ids)
expect(new_tgr.feedback_files.ids.sort).not_to eq(old_tgr.feedback_files.ids.sort)
end
end
end
Expand All @@ -286,8 +289,8 @@
it 'retains the marks from each original submission\'s original result' do
@new_submissions.zip(@original_submissions).each do |new_submission, old_submission|
# the last result in each submission is the original one in the old submission
expect(new_submission.current_result.marks.map(&:mark)).to eq(old_submission
.get_original_result.marks.map(&:mark))
expect(new_submission.current_result.marks.order(:criterion_id).map(&:mark)).to eq(old_submission
.get_original_result.marks.order(:criterion_id).map(&:mark))
end
end
end
Expand All @@ -296,13 +299,14 @@
it 'creates the correct number of new marks for each result' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.marks.size).to eq(old_result.marks.size)
expect(new_result.marks.ids).not_to eq(old_result.marks.ids)
expect(new_result.marks.ids.sort).not_to eq(old_result.marks.ids.sort)
end
end

it 'retains the correct mark values for each result' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.marks.map(&:mark)).to eq(old_result.marks.map(&:mark))
expect(new_result.marks.order(:criterion_id).map(&:mark)).to eq(old_result
.marks.order(:criterion_id).map(&:mark))
end
end
end
Expand All @@ -313,19 +317,22 @@
it 'creates the correct number of new annotations for each result' do
@new_results.reload.zip(@original_results).each do |new_result, old_result|
expect(new_result.annotations.size).to eq(old_result.annotations.size)
expect(new_result.annotations.ids).not_to eq(old_result.annotations.ids)
expect(new_result.annotations.ids.sort).not_to eq(old_result.annotations.ids.sort)
end
end

it 'retains the mark deductions from deductive annotations' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.marks.map(&:calculate_deduction)).to eq(old_result.marks.map(&:calculate_deduction))
expect(new_result.marks.order(:criterion_id).map(&:calculate_deduction)).to eq(old_result
.marks.order(:criterion_id).map(&:calculate_deduction))
end
end

it 'retains the text from each annotation' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.annotations.map { |a| a.annotation_text.content }).to eq(old_result.annotations.map do |a|
expect(new_result.annotations.order(:annotation_text_id).map do |a|
a.annotation_text.content
end).to eq(old_result.annotations.order(:annotation_text_id).map do |a|
a.annotation_text.content
end)
end
Expand All @@ -348,13 +355,14 @@
it 'creates the correct number of new extra marks for each result' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.extra_marks.size).to eq(old_result.extra_marks.size)
expect(new_result.extra_marks.ids).not_to eq(old_result.extra_marks.ids)
expect(new_result.extra_marks.ids.sort).not_to eq(old_result.extra_marks.ids.sort)
end
end

it 'retains the correct mark values for each result' do
@new_results.zip(@original_results).each do |new_result, old_result|
expect(new_result.extra_marks.map(&:extra_mark)).to eq(old_result.extra_marks.map(&:extra_mark))
# no common field so compare by full array
expect(new_result.extra_marks.map(&:extra_mark).sort).to eq(old_result.extra_marks.map(&:extra_mark).sort)
end
end
end
Expand Down

0 comments on commit 01e0d63

Please sign in to comment.