diff --git a/Changelog.md b/Changelog.md index 0ffa5625a2..5110be5a8c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -26,6 +26,7 @@ - Ensure Jupyter notebook HTML rendering does not require external CDNs (#6656) - Prevent Jupyter notebook from reloading when an annotation is added (#6656) - Added a button allowing graders to view a random incomplete submission (#6641) +- Added a filter modal allowing graders to specify filters and order when navigating submissions (#6642) - Add icons to submission and result grading action buttons (#6666) - Remove group name maximum length constraint (#6668) - Fix bug where in some cases flash messages were not being rendered correctly (#6670) diff --git a/app/assets/javascripts/Components/Modals/filter_modal.jsx b/app/assets/javascripts/Components/Modals/filter_modal.jsx new file mode 100644 index 0000000000..60957a8b84 --- /dev/null +++ b/app/assets/javascripts/Components/Modals/filter_modal.jsx @@ -0,0 +1,307 @@ +import React from "react"; +import Modal from "react-modal"; +import {MultiSelectDropdown} from "../../DropDownMenu/MultiSelectDropDown"; +import {SingleSelectDropDown} from "../../DropDownMenu/SingleSelectDropDown"; +import {FontAwesomeIcon} from "@fortawesome/react-fontawesome"; + +export class FilterModal extends React.Component { + onToggleOptionTas = user_name => { + const newArray = [...this.props.filterData.tas]; + if (newArray.includes(user_name)) { + this.props.updateFilterData({ + tas: newArray.filter(item => item !== user_name), + }); + } else { + newArray.push(user_name); + this.props.updateFilterData({ + tas: newArray, + }); + } + }; + + onClearSelectionTAs = () => { + this.props.updateFilterData({ + tas: [], + }); + }; + + onClearSelectionTags = () => { + this.props.updateFilterData({ + tags: [], + }); + }; + + onToggleOptionTags = tag => { + const newArray = [...this.props.filterData.tags]; + if (newArray.includes(tag)) { + this.props.updateFilterData({ + tags: newArray.filter(item => item !== tag), + }); + } else { + newArray.push(tag); + this.props.updateFilterData({ + tags: newArray, + }); + } + }; + + renderTasDropdown = () => { + if (this.props.role === "Instructor") { + let tas = this.props.tas.map(option => { + return {key: option[0], display: option[0] + " - " + option[1]}; + }); + return ( +
+

{I18n.t("activerecord.models.ta.other")}

+ +
+ ); + } + }; + + renderTagsDropdown = () => { + let options = []; + if (this.props.available_tags.length !== 0) { + options = options.concat( + this.props.available_tags.map(item => { + return {key: item.name, display: item.name}; + }) + ); + } + if (this.props.current_tags.length !== 0) { + options = options.concat( + this.props.current_tags.map(item => { + return {key: item.name, display: item.name}; + }) + ); + } + return ( + + ); + }; + + rangeFilter = (min, max, title, onMinChange, onMaxChange) => { + return ( +
+

{title}

+
+ onMinChange(e)} + /> + {I18n.t("to")} + onMaxChange(e)} + /> +
+ + {I18n.t("results.filters.invalid_range")} +
+
+
+ ); + }; + + onTotalMarkMinChange = e => { + this.props.updateFilterData({ + totalMarkRange: {...this.props.filterData.totalMarkRange, min: e.target.value}, + }); + }; + + onTotalMarkMaxChange = e => { + this.props.updateFilterData({ + totalMarkRange: {...this.props.filterData.totalMarkRange, max: e.target.value}, + }); + }; + + onTotalExtraMarkMinChange = e => { + this.props.updateFilterData({ + totalExtraMarkRange: {...this.props.filterData.totalExtraMarkRange, min: e.target.value}, + }); + }; + + onTotalExtraMarkMaxChange = e => { + this.props.updateFilterData({ + totalExtraMarkRange: {...this.props.filterData.totalExtraMarkRange, max: e.target.value}, + }); + }; + + componentDidMount() { + Modal.setAppElement("body"); + } + + onClearFilters = event => { + event.preventDefault(); + this.props.clearAllFilters(); + }; + + render() { + if (this.props.loading) { + return ""; + } + return ( + { + this.props.onRequestClose(); + }} + > +

+ + {I18n.t("results.filter_submissions")} +

+
+
+
+
+
+

{I18n.t("results.filters.order_by")}

+ { + this.props.updateFilterData({ + orderBy: selection, + }); + }} + defaultValue={I18n.t("activerecord.attributes.group.group_name")} + /> +
+ { + this.props.updateFilterData({ascending: true}); + }} + id={"Asc"} + data-testid={"ascending"} + /> + + { + this.props.updateFilterData({ascending: false}); + }} + id={"Desc"} + data-testid={"descending"} + /> + +
+
+
+

{I18n.t("activerecord.attributes.result.marking_state")}

+ { + this.props.updateFilterData({ + markingState: selection, + }); + }} + /> +
+
+
+
+

{I18n.t("activerecord.models.tag.other")}

+ {this.renderTagsDropdown()} +
+
+

{I18n.t("activerecord.models.section.one")}

+ { + this.props.updateFilterData({ + section: selection, + }); + }} + defaultValue={""} + /> +
+
+
+ {this.renderTasDropdown()} +
+

{I18n.t("activerecord.models.annotation.one")}

+ + this.props.updateFilterData({ + annotationText: e.target.value, + }) + } + placeholder={I18n.t("results.filters.text_box_placeholder")} + /> +
+
+ +
+ {this.rangeFilter( + this.props.filterData.totalMarkRange.min, + this.props.filterData.totalMarkRange.max, + I18n.t("results.total_mark"), + this.onTotalMarkMinChange, + this.onTotalMarkMaxChange + )} + {this.rangeFilter( + this.props.filterData.totalExtraMarkRange.min, + this.props.filterData.totalExtraMarkRange.max, + I18n.t("results.total_extra_marks"), + this.onTotalExtraMarkMinChange, + this.onTotalExtraMarkMaxChange + )} +
+
+
+
+
+ + +
+
+
+ + ); + } +} diff --git a/app/assets/javascripts/Components/Result/result.jsx b/app/assets/javascripts/Components/Result/result.jsx index e55f7066b6..98c8b09c4c 100644 --- a/app/assets/javascripts/Components/Result/result.jsx +++ b/app/assets/javascripts/Components/Result/result.jsx @@ -17,6 +17,24 @@ const INITIAL_ANNOTATION_MODAL_STATE = { changeOneOption: false, }; +const INITIAL_FILTER_MODAL_STATE = { + ascending: true, + orderBy: "group_name", + annotationText: "", + tas: [], + tags: [], + section: "", + markingState: "", + totalMarkRange: { + min: "", + max: "", + }, + totalExtraMarkRange: { + min: "", + max: "", + }, +}; + class Result extends React.Component { constructor(props) { super(props); @@ -33,6 +51,7 @@ class Result extends React.Component { result_id: props.result_id, grouping_id: props.grouping_id, can_release: false, + filterData: INITIAL_FILTER_MODAL_STATE, }; this.leftPane = React.createRef(); @@ -659,10 +678,15 @@ class Result extends React.Component { nextSubmission = direction => { return () => { + let data = {direction: direction}; + if (this.props.role !== "Student") { + data["filterData"] = this.state.filterData; + } + const url = Routes.next_grouping_course_result_path( this.props.course_id, this.state.result_id, - {direction: direction} + data ); this.setState({loading: true}, () => { @@ -675,6 +699,7 @@ class Result extends React.Component { .then(result => { if (!result.next_result || !result.next_grouping) { alert(I18n.t("results.no_results_in_direction")); + this.setState({loading: false}); return; } @@ -743,6 +768,15 @@ class Result extends React.Component { }); }; + updateFilterData = new_filters => { + const filters = {...this.state.filterData, ...new_filters}; + this.setState({filterData: filters}); + }; + + resetFilterData = () => { + this.setState({filterData: INITIAL_FILTER_MODAL_STATE}); + }; + render() { return ( @@ -783,6 +817,14 @@ class Result extends React.Component { randomIncompleteSubmission={this.randomIncompleteSubmission} previousSubmission={this.nextSubmission(-1)} course_id={this.props.course_id} + filterData={this.state.filterData} + updateFilterData={this.updateFilterData} + clearAllFilters={this.resetFilterData} + sections={this.state.sections} + tas={this.state.tas} + available_tags={this.state.available_tags} + current_tags={this.state.current_tags} + loading={this.state.loading} />
diff --git a/app/assets/javascripts/Components/Result/submission_selector.jsx b/app/assets/javascripts/Components/Result/submission_selector.jsx index 0814cf1a13..7466b6bc74 100644 --- a/app/assets/javascripts/Components/Result/submission_selector.jsx +++ b/app/assets/javascripts/Components/Result/submission_selector.jsx @@ -1,9 +1,16 @@ import React from "react"; -import {render} from "react-dom"; import PropTypes from "prop-types"; import {FontAwesomeIcon} from "@fortawesome/react-fontawesome"; +import {FilterModal} from "../Modals/filter_modal"; export class SubmissionSelector extends React.Component { + constructor(props) { + super(props); + this.state = { + showFilterModal: false, + }; + } + renderToggleMarkingStateButton = () => { let buttonText, className, disabled, icon; if (this.props.marking_state === "complete") { @@ -108,6 +115,24 @@ export class SubmissionSelector extends React.Component { ); } + onOpenFilterModal = () => { + this.setState({showFilterModal: true}); + }; + + renderFilterButton() { + if (this.props.role !== "Student") { + return ( + + ); + } + } + renderRandomIncompleteSubmissionButton() { if (this.props.role !== "Student") { return ( @@ -127,6 +152,28 @@ export class SubmissionSelector extends React.Component { } } + renderFilterModal() { + if (this.props.role !== "Student") { + return ( +
+ this.setState({showFilterModal: false})} + filterData={this.props.filterData} + updateFilterData={this.props.updateFilterData} + clearAllFilters={this.props.clearAllFilters} + sections={this.props.sections} + tas={this.props.tas} + available_tags={this.props.available_tags} + current_tags={this.props.current_tags} + loading={this.props.loading} + role={this.props.role} + /> +
+ ); + } + } + render() { if (this.props.role === "Student" && !this.props.is_reviewer) { return ""; @@ -171,6 +218,7 @@ export class SubmissionSelector extends React.Component { {this.renderRandomIncompleteSubmissionButton()} + {this.renderFilterButton()}
+ {this.renderFilterModal()}
); } diff --git a/app/assets/javascripts/Components/__tests__/filter_modal.test.jsx b/app/assets/javascripts/Components/__tests__/filter_modal.test.jsx new file mode 100644 index 0000000000..01097c41f1 --- /dev/null +++ b/app/assets/javascripts/Components/__tests__/filter_modal.test.jsx @@ -0,0 +1,354 @@ +import * as React from "react"; +import {render, screen, fireEvent, within} from "@testing-library/react"; +import {FilterModal} from "../Modals/filter_modal"; +import Modal from "react-modal"; + +jest.mock("@fortawesome/react-fontawesome", () => ({ + FontAwesomeIcon: () => { + return null; + }, +})); + +describe("FilterModal", () => { + let props; + let component; + + let sharedExamplesTaAndInstructor = role => { + beforeEach(() => { + props = { + filterData: { + ascending: true, + orderBy: "group_name", + annotationText: "", + tas: ["a", "b"], + tags: ["a", "b"], + section: "", + markingState: "", + totalMarkRange: { + min: "", + max: "", + }, + totalExtraMarkRange: { + min: "", + max: "", + }, + }, + available_tags: [{name: "a"}, {name: "b"}], + current_tags: [{name: "c"}, {name: "d"}], + sections: ["LEC0101", "LEC0202"], + tas: [ + ["a", "A"], + ["b", "B"], + ["c", "C"], + ["d", "D"], + ], + isOpen: true, + onRequestClose: jest.fn().mockImplementation(() => (props.isOpen = false)), + updateFilterData: jest.fn().mockImplementation(() => null), + clearAllFilters: jest.fn().mockImplementation(() => null), + role: role, + }; + + // Set the app element for React Modal + Modal.setAppElement("body"); + component = render(); + }); + + it("should close on submit", () => { + fireEvent.click(screen.getByText(/Close/i)); + expect(props.onRequestClose).toHaveBeenCalled(); + }); + + it("should clear all filters when clicked on Clear All button", () => { + fireEvent.click(screen.getByText(/Clear All/i)); + expect(props.clearAllFilters).toHaveBeenCalled(); + }); + + it("should render the modal", () => { + expect(screen.getByText(/Filter Submissions/i)).toBeInTheDocument(); + expect(screen.getByText(/Close/i)).toBeInTheDocument(); + expect(screen.getByText(/Clear All/i)).toBeInTheDocument(); + }); + + describe("Filter By Annotation", () => { + it("should render the annotation textbox", () => { + expect(screen.getByText(/Annotation/i)).toBeInTheDocument(); + expect(screen.getByPlaceholderText(/Search Text/i)).toBeInTheDocument(); + }); + + it("should save annotation text on input", () => { + fireEvent.change(screen.getByPlaceholderText(/Search Text/i), { + target: {value: "JavaScript"}, + }); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }); + + describe("MultiSelectDropdown filters", () => { + let multiSelectDropdownClear = test_id => { + it("should reset tags selection when clicked on xmark icon", () => { + const dropdown = screen.getByTestId(test_id); + const icon = within(dropdown).getByTestId("reset"); + fireEvent.click(icon); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + let multiSelectDropdownRender = test_id => { + it("should render all selected tags", () => { + const dropdown = screen.getByTestId(test_id); + const tags = dropdown.getElementsByClassName("tag"); + expect(tags).toHaveLength(2); + }); + }; + + let multiSelectDropdownMakeSelection = (test_id, selection) => { + it("should select option when clicked on an option", () => { + const dropdown = screen.getByTestId(test_id); + fireEvent.click(dropdown); + + //select + const option = within(dropdown).getByText(selection); + fireEvent.click(option); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + let multiSelectDropdownDeselect = (test_id, option) => { + it("should deselect option when clicked on a tag", () => { + const dropdown = screen.getByTestId(test_id); + fireEvent.click(within(dropdown).getByText(option)); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + + it("should deselect option when clicked on a selected option", () => { + const dropdown = screen.getByTestId(test_id); + fireEvent.click(dropdown); + const list = within(dropdown).getByRole("list", {hidden: true}); + + //select + const selected_option = within(list).getByText(option); + fireEvent.click(selected_option); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + describe("Filter By Tags", () => { + const test_id = "Tags"; + multiSelectDropdownRender(test_id); + multiSelectDropdownClear(test_id); + multiSelectDropdownMakeSelection(test_id, "d"); + multiSelectDropdownDeselect(test_id, "a"); + }); + }); + + describe("Range filters", () => { + let rangeRender = test_id => { + it("should render 2 input fields of type number", () => { + const filter = screen.getByTestId(test_id); + const minInput = within(filter).getByPlaceholderText(/Min/i); + const maxInput = within(filter).getByPlaceholderText(/Max/i); + expect(minInput).toHaveAttribute("type", "number"); + expect(maxInput).toHaveAttribute("type", "number"); + }); + }; + + let rangeOnInput = test_id => { + it("inputs should be valid when passed valid range", () => { + const filter = screen.getByTestId(test_id); + const minInput = within(filter).getByPlaceholderText(/Min/i); + const maxInput = within(filter).getByPlaceholderText(/Max/i); + fireEvent.change(minInput, { + target: {value: 0}, + }); + fireEvent.change(maxInput, { + target: {value: 10}, + }); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + let rangeValidInput = test_id => { + it("inputs should be valid when passed valid range", () => { + props.filterData.totalMarkRange = {min: 0, max: 10}; + props.filterData.totalExtraMarkRange = {min: 0, max: 10}; + component.rerender(); + + const filter = screen.getByTestId(test_id); + const minInput = within(filter).getByPlaceholderText(/Min/i); + const maxInput = within(filter).getByPlaceholderText(/Max/i); + + expect(minInput).toHaveValue(0); + expect(maxInput).toHaveValue(10); + + expect(minInput.checkValidity()).toBe(true); + expect(maxInput.checkValidity()).toBe(true); + }); + }; + + let rangeInvalidInput = test_id => { + it("inputs should be invalid when passed invalid range", () => { + props.filterData.totalMarkRange = {min: 0, max: -1}; + props.filterData.totalExtraMarkRange = {min: 0, max: -1}; + component.rerender(); + + const filter = screen.getByTestId(test_id); + const minInput = within(filter).getByPlaceholderText(/Min/i); + const maxInput = within(filter).getByPlaceholderText(/Max/i); + + expect(minInput).toHaveValue(0); + expect(maxInput).toHaveValue(-1); + + expect(minInput.checkValidity()).toBe(false); + expect(maxInput.checkValidity()).toBe(false); + }); + }; + + describe("Total Mark Range", () => { + const test_id = "Total Mark"; + rangeRender(test_id); + rangeOnInput(test_id); + rangeValidInput(test_id); + rangeInvalidInput(test_id); + }); + + describe("Total Extra Mark Range", () => { + const test_id = "Total Extra Marks"; + rangeRender(test_id); + rangeOnInput(test_id); + rangeValidInput(test_id); + rangeInvalidInput(test_id); + }); + }); + + describe("Single Select Dropdown Filters", () => { + let singleSelectDropdownMakeSelection = (filterTestId, selection) => { + it("should change the selection", () => { + let dropdownDiv = screen.getByTestId(filterTestId); + fireEvent.click(within(dropdownDiv).getByTestId("dropdown")); + fireEvent.click(within(dropdownDiv).getByText(selection)); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + let singleSelectDropdownClear = (filterTestId, selection, defaultValue) => { + it("should reset the selection and close the dropdown when the x button is clicked", () => { + let dropdownDiv = screen.getByTestId(filterTestId); + + // setting the dropdown value to some random value + fireEvent.click(within(dropdownDiv).getByTestId("dropdown")); + fireEvent.click(within(dropdownDiv).getByText(selection)); + + // resetting the dropdown value + fireEvent.click(within(dropdownDiv).getByTestId("reset-dropdown-selection")); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }; + + describe("Order By", () => { + describe("selecting order subject", () => { + singleSelectDropdownMakeSelection("order-by", "Submission Date"); + singleSelectDropdownClear("order-by", "Submission Date", "Group Name"); + + it("should show the user specific options", () => { + let dropdownDiv = screen.getByTestId("order-by"); + fireEvent.click(within(dropdownDiv).getByTestId("dropdown")); + const options = within(dropdownDiv).getByTestId("options"); + expect(within(options).getByText("Group name")).toBeInTheDocument(); + expect(within(options).getByText("Submission Date")).toBeInTheDocument(); + }); + }); + + describe("selecting between ascending and descending order", () => { + it("should save the selection on change", () => { + // setting the ordering to descending + fireEvent.click(within(screen.getByTestId("order-by")).getByTestId("descending")); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }); + }); + + describe("Filter By Section", () => { + singleSelectDropdownMakeSelection("section", "LEC0101"); + singleSelectDropdownClear("section", "LEC0101", ""); + }); + + describe("Filter By Marking State", () => { + singleSelectDropdownMakeSelection("marking-state", "Complete"); + singleSelectDropdownClear("marking-state", "Complete", ""); + + it("should show the user specific options", () => { + let dropdownDiv = screen.getByTestId("marking-state"); + fireEvent.click(within(dropdownDiv).getByTestId("dropdown")); + dropdownDiv = screen.getByTestId("marking-state"); + expect(within(dropdownDiv).getByText("In Progress")).toBeInTheDocument(); + expect(within(dropdownDiv).getByText("Complete")).toBeInTheDocument(); + expect(within(dropdownDiv).getByText("Released")).toBeInTheDocument(); + expect(within(dropdownDiv).getByText("Remark Requested")).toBeInTheDocument(); + }); + }); + }); + }; + + describe("An Instructor", () => { + sharedExamplesTaAndInstructor("Instructor"); + + describe("Filter by Tas", () => { + const test_id = "Tas"; + + it("should reset tags selection when clicked on xmark icon", () => { + const dropdown = screen.getByTestId(test_id); + const icon = within(dropdown).getByTestId("reset"); + fireEvent.click(icon); + expect(props.updateFilterData).toHaveBeenCalled(); + }); + + it("should render all selected tags", () => { + const dropdown = screen.getByTestId(test_id); + const tags = dropdown.getElementsByClassName("tag"); + expect(tags).toHaveLength(2); + }); + + it("should select option when clicked on an option", () => { + const dropdown = screen.getByTestId(test_id); + fireEvent.click(dropdown); + const option = within(dropdown).getByText("d - D"); + fireEvent.click(option); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + + it("should deselect option when clicked on a tag", () => { + const dropdown = screen.getByTestId(test_id); + const tag = within(dropdown).getByText("a"); + fireEvent.click(tag); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + + it("should deselect option when clicked on a selected option", () => { + const dropdown = screen.getByTestId(test_id); + fireEvent.click(dropdown); + const selected_option = within(dropdown).getByText("a - A"); + fireEvent.click(selected_option); + + expect(props.updateFilterData).toHaveBeenCalled(); + }); + }); + }); + + describe("A Ta", () => { + sharedExamplesTaAndInstructor("Ta"); + + describe("Filter by Tas", () => { + it("should not render filter by tas", () => { + const dropdown = screen.queryByTestId("Tas"); + expect(dropdown).not.toBeInTheDocument(); + }); + }); + }); +}); diff --git a/app/assets/javascripts/Components/__tests__/multi_select_dropdown.test.jsx b/app/assets/javascripts/Components/__tests__/multi_select_dropdown.test.jsx new file mode 100644 index 0000000000..a0a5799339 --- /dev/null +++ b/app/assets/javascripts/Components/__tests__/multi_select_dropdown.test.jsx @@ -0,0 +1,109 @@ +/*** + * Tests for MultiSelectDropdown Component + */ + +import * as React from "react"; +import {render, screen, fireEvent, within} from "@testing-library/react"; +import {MultiSelectDropdown} from "../../DropDownMenu/MultiSelectDropDown"; + +jest.mock("@fortawesome/react-fontawesome", () => ({ + FontAwesomeIcon: () => { + return null; + }, +})); + +describe("MultiSelectDropdown", () => { + let props = { + id: "Test", + options: [ + {key: "a", display: "a"}, + {key: "b", display: "b"}, + {key: "c", display: "c"}, + {key: "d", display: "d"}, + ], + selected: ["a"], + onToggleOption: jest.fn().mockImplementation(() => null), + onClearSelection: jest.fn().mockImplementation(() => null), + }; + + it("should render the closed dropdown by default", () => { + render(); + + const tags_box = screen.getByTestId("tags-box"); + expect(tags_box).toBeInTheDocument(); + expect(tags_box.getElementsByClassName("tag")).toHaveLength(1); + expect(screen.getByText("a")).toBeInTheDocument(); //test for arrow + expect(screen.getByTestId("reset")).toBeInTheDocument(); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); + }); + + it("should expand dropdown on click", () => { + render(); + + const tags_box = screen.getByTestId("tags-box"); + fireEvent.click(tags_box); + const list = screen.queryByRole("list"); + expect(list).toBeInTheDocument(); + expect(screen.queryAllByRole("listitem")).toHaveLength(4); + expect(within(list).getAllByTestId("checked")).toHaveLength(1); + expect(within(list).getAllByTestId("unchecked")).toHaveLength(3); + }); + + it("should close expanded dropdown on click", () => { + render(); + + const tags_box = screen.getByTestId("tags-box"); + fireEvent.click(tags_box); + fireEvent.click(tags_box); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); + }); + + it("should deselect option when clicked on tag", () => { + render(); + + const tag = screen.getByText("a"); + fireEvent.click(tag); + expect(props.onToggleOption).toHaveBeenCalledWith("a"); + }); + + it("should deselect option when clicked on a select list item", () => { + render(); + + const tags_box = screen.getByTestId("tags-box"); + fireEvent.click(tags_box); + const selected_option = within(screen.queryByRole("list")).getByText("a"); + fireEvent.click(selected_option); + expect(props.onToggleOption).toHaveBeenCalledWith("a"); + expect(screen.queryByRole("list")).toBeInTheDocument(); + }); + + it("should select option when clicked on a list item", () => { + render(); + + const tags_box = screen.getByTestId("tags-box"); + fireEvent.click(tags_box); + const option = within(screen.queryByRole("list")).getByText("b"); + fireEvent.click(option); + expect(props.onToggleOption).toHaveBeenCalledWith("b"); + }); + + it("should clear all selections when clicked on reset xmark icon", () => { + render(); + + const icon = screen.getByTestId("reset"); + fireEvent.click(icon); + expect(props.onClearSelection).toHaveBeenCalled(); + }); + + it("should show no options available when options passed down from props is empty", () => { + props.options = []; + props.selected = []; + render(); + + const tags_box = screen.getByTestId("tags-box"); + fireEvent.click(tags_box); + expect(screen.queryByRole("list")).toBeInTheDocument(); + expect(screen.queryAllByRole("listitem")).toHaveLength(1); + expect(screen.getByText("No options available")).toBeInTheDocument(); + }); +}); diff --git a/app/assets/javascripts/Components/__tests__/single_select_dropdown.test.jsx b/app/assets/javascripts/Components/__tests__/single_select_dropdown.test.jsx new file mode 100644 index 0000000000..a168c8ada5 --- /dev/null +++ b/app/assets/javascripts/Components/__tests__/single_select_dropdown.test.jsx @@ -0,0 +1,77 @@ +import * as React from "react"; +import {render, screen, fireEvent} from "@testing-library/react"; +import {SingleSelectDropDown} from "../../DropDownMenu/SingleSelectDropDown.js"; + +describe("SingleSelectDropdown", () => { + let props; + let onChange = jest.fn(); + let options = ["a", "b", "c"]; + + beforeEach(() => { + props = { + options: options, + selected: "", + onSelect: onChange, + defaultValue: "", + }; + + render(); + }); + + describe("when the dropdown is closed", () => { + describe("when the dropdown is first rendered", () => { + it("should load the selected value as the currently selected option", () => { + expect(screen.getByTestId("selection")).toHaveTextContent(""); + }); + }); + + it("should not load the dropdown options", () => { + expect(screen.queryByText("a")).toBeNull(); + expect(screen.queryByText("b")).toBeNull(); + expect(screen.queryByText("c")).toBeNull(); + }); + + it("should render a down arrow within the dropdown", () => { + expect(screen.getByTestId("arrow-down")).toBeInTheDocument(); + }); + }); + + describe("when the dropdown is opened", () => { + describe("when there are options available", () => { + it("should open a dropdown only with the options specified", () => { + fireEvent.click(screen.getByTestId("dropdown")); + expect(screen.getByText("a")).toBeInTheDocument(); + expect(screen.getByText("b")).toBeInTheDocument(); + expect(screen.getByText("c")).toBeInTheDocument(); + expect(screen.getAllByRole("listitem").length).toBe(3); + }); + }); + + describe("No options available", () => { + beforeAll(() => { + options = []; + }); + + it("should show no options available when there aren't any options to choose from", () => { + fireEvent.click(screen.getByTestId("dropdown")); + expect(screen.getByText("No options available")).toBeInTheDocument(); + expect(screen.queryByText("a")).toBeNull(); + expect(screen.queryByText("b")).toBeNull(); + expect(screen.queryByText("c")).toBeNull(); + }); + }); + + it("should render a upward arrow within the dropdown", () => { + fireEvent.click(screen.getByTestId("dropdown")); + expect(screen.getByTestId("arrow-up")).toBeInTheDocument(); + }); + + it("should close the dropdown when clicked again", () => { + fireEvent.click(screen.getByTestId("dropdown")); + fireEvent.click(screen.getByTestId("dropdown")); + expect(screen.queryByText("a")).toBeNull(); + expect(screen.queryByText("b")).toBeNull(); + expect(screen.queryByText("c")).toBeNull(); + }); + }); +}); diff --git a/app/assets/javascripts/Components/__tests__/submission_selector.test.jsx b/app/assets/javascripts/Components/__tests__/submission_selector.test.jsx index cabbeb8c9d..d71031f92d 100644 --- a/app/assets/javascripts/Components/__tests__/submission_selector.test.jsx +++ b/app/assets/javascripts/Components/__tests__/submission_selector.test.jsx @@ -63,6 +63,15 @@ describe("SubmissionSelector", () => { expect(wrapper.find(".group-name").text()).toBe(props.group_name); }); + it("should show filter modal when filter-button is pressed", () => { + wrapper = getWrapper(props); + expect(wrapper.find(".filter").exists()).toBeTruthy(); + wrapper.find(".filter").simulate("click"); + const modal = wrapper.find("FilterModal"); + + expect(modal.exists()).toBeTruthy(); + }); + it("should pass correct values to the progress meter", () => { props.num_marked = 50; props.num_collected = 100; diff --git a/app/assets/javascripts/DropDownMenu/MultiSelectDropDown.js b/app/assets/javascripts/DropDownMenu/MultiSelectDropDown.js new file mode 100644 index 0000000000..08285b28fd --- /dev/null +++ b/app/assets/javascripts/DropDownMenu/MultiSelectDropDown.js @@ -0,0 +1,110 @@ +import React from "react"; +import {FontAwesomeIcon} from "@fortawesome/react-fontawesome"; + +export class MultiSelectDropdown extends React.Component { + constructor(props) { + super(props); + this.state = {expanded: false, tags: []}; + } + + onSelect = (e, option) => { + e.stopPropagation(); + this.props.onToggleOption(option); + }; + + renderDropdown = (options, selected, expanded) => { + let isSelected; + options.sort((a, b) => a.key.localeCompare(b.key)); + if (expanded) { + if (options.length === 0) { + return ( +
    +
  • + {I18n.t("results.filters.no_options")} +
    • +
+ ); + } else { + return ( +
    + {options.map(option => { + isSelected = selected.includes(option.key); + return ( +
  • this.onSelect(e, option.key)}> + {this.renderCheckBox(isSelected)} + {option.display} +
    • + ); + })} +
+ ); + } + } + }; + + renderCheckBox = checked => { + if (checked) { + return ( +
+ +
+ ); + } else { + return ( +
+ +
+ ); + } + }; + + render() { + let selected = this.props.selected; + let options = this.props.options; + let expanded = this.state.expanded; + let arrow; + if (expanded !== false) { + arrow = ; + } else { + arrow = ; + } + + return ( +
this.setState({expanded: !this.state.expanded})} + data-testid={this.props.title} + tabIndex={-1} + onBlur={() => this.setState({expanded: false})} + > +
+ {selected.map(tag => ( +
{ + this.onSelect(e, tag); + }} + > + {tag} + +
+ ))} +
+
+
{ + e.preventDefault(); + e.stopPropagation(); + this.props.onClearSelection(); + }} + > + +
+ {arrow} +
+ {expanded && this.renderDropdown(options, selected, expanded)} +
+ ); + } +} diff --git a/app/assets/javascripts/DropDownMenu/SingleSelectDropDown.js b/app/assets/javascripts/DropDownMenu/SingleSelectDropDown.js new file mode 100644 index 0000000000..eb342a4711 --- /dev/null +++ b/app/assets/javascripts/DropDownMenu/SingleSelectDropDown.js @@ -0,0 +1,88 @@ +import React from "react"; +import {FontAwesomeIcon} from "@fortawesome/react-fontawesome"; + +export class SingleSelectDropDown extends React.Component { + constructor(props) { + super(props); + this.state = {expanded: false}; + } + + onSelect = (e, selection) => { + e.stopPropagation(); + this.props.onSelect(selection); + this.setState({expanded: false}); + }; + + renderDropdown = (options, selected, expanded) => { + if (expanded) { + if (options.length === 0) { + return ( +
    +
  • + {I18n.t("results.filters.no_options")} +
    • +
+ ); + } else { + return ( +
    + {options.map(option => { + return ( +
  • this.onSelect(e, option)}> + + {this.props.valueToDisplayName != null + ? this.props.valueToDisplayName[option] + : option} + +
    • + ); + })} +
+ ); + } + } + }; + + renderArrow = () => { + if (this.state.expanded !== false) { + return ; + } else { + return ; + } + }; + + render() { + let selected = this.props.selected; + let options = this.props.options; + let expanded = this.state.expanded; + + return ( +
this.setState({expanded: !this.state.expanded})} + onBlur={() => this.setState({expanded: false})} + tabIndex={-1} + data-testid={"dropdown"} + > + + {this.props.valueToDisplayName != null + ? this.props.valueToDisplayName[this.props.selected] + : this.props.selected} + + {this.renderArrow()} +
{ + e.preventDefault(); + this.onSelect(e, this.props.defaultValue); + }} + data-testid={"reset-dropdown-selection"} + > + +
+ + {expanded && this.renderDropdown(options, selected, expanded)} +
+ ); + } +} diff --git a/app/assets/javascripts/fontawesome_config.js b/app/assets/javascripts/fontawesome_config.js index aee577c46a..ba622ad2a3 100644 --- a/app/assets/javascripts/fontawesome_config.js +++ b/app/assets/javascripts/fontawesome_config.js @@ -22,6 +22,7 @@ import { faFileImage, faFileImport, faFilePdf, + faFilter, faFolder, faFolderOpen, faFolderPlus, @@ -49,6 +50,8 @@ import { import {faGithub} from "@fortawesome/free-brands-svg-icons"; +import {faSquare} from "@fortawesome/free-regular-svg-icons"; + config.autoAddCss = false; library.add( @@ -74,10 +77,12 @@ library.add( faFileImage, faFileImport, faFilePdf, + faFilter, faFolder, faFolderOpen, faFolderPlus, faGear, + faGithub, faGripVertical, faLink, faMinus, @@ -89,6 +94,7 @@ library.add( faRocket, faSignOut, faSlash, + faSquare, faSquareCheck, faTable, faTrash, @@ -99,6 +105,4 @@ library.add( faWarning ); -library.add(faGithub); - dom.watch(); diff --git a/app/assets/stylesheets/common/_filter_modal.scss b/app/assets/stylesheets/common/_filter_modal.scss new file mode 100644 index 0000000000..1033d6a051 --- /dev/null +++ b/app/assets/stylesheets/common/_filter_modal.scss @@ -0,0 +1,58 @@ +@import 'constants'; + +.filter-modal { + .filter-modal-title { + padding-left: 30px; + } + + .filter-icon-title { + padding-right: 10px; + } + + .annotation-input { + margin: 5px; + + input[type='text'] { + height: 30px; + width: $dropdown-horizontal; + } + } + + .filter { + margin: 5px; + } + + .order { + padding-top: 10px; + } + + .range { + background: $background-main; + max-width: $dropdown-horizontal; + min-height: 30px; + + > span { + margin-left: 10px; + margin-right: 10px; + } + + input[type='number'] { + width: 80px; + } + + > input:valid ~ .hidden { + display: none; + } + + .hidden { + display: inline-flex; + padding-top: 5px; + width: $dropdown-horizontal; + + .validity { + color: $severe-error; + margin-left: 2px; + } + } + } +} diff --git a/app/assets/stylesheets/common/_filter_modal_dropdown.scss b/app/assets/stylesheets/common/_filter_modal_dropdown.scss new file mode 100644 index 0000000000..8e5672d6ac --- /dev/null +++ b/app/assets/stylesheets/common/_filter_modal_dropdown.scss @@ -0,0 +1,16 @@ +@import 'constants'; + +@mixin filter-modal-dropdown { + height: 30px; + width: $dropdown-horizontal; + + ul { + display: block; + max-height: 120px; + overflow-y: auto; + } + + .fa-xmark { + color: $primary-one; + } +} diff --git a/app/assets/stylesheets/common/_markus.scss b/app/assets/stylesheets/common/_markus.scss index 02c858135a..2c895c7be1 100644 --- a/app/assets/stylesheets/common/_markus.scss +++ b/app/assets/stylesheets/common/_markus.scss @@ -15,6 +15,9 @@ @import 'courses'; @import 'url_viewer'; @import 'statistics'; +@import 'multi_select_dropdown'; +@import 'single_select_dropdown'; +@import 'filter_modal'; /** Main */ diff --git a/app/assets/stylesheets/common/_modals.scss b/app/assets/stylesheets/common/_modals.scss index c095eba63b..93781005c7 100644 --- a/app/assets/stylesheets/common/_modals.scss +++ b/app/assets/stylesheets/common/_modals.scss @@ -21,6 +21,18 @@ } } +.modal-container-scrollable { + align-items: flex-start; + display: flex; + max-height: 300px; + overflow-y: auto; + + > * { + margin: 5px; + padding: 5px; + } +} + .modal-container-vertical { display: flex; flex-direction: column; diff --git a/app/assets/stylesheets/common/_multi_select_dropdown.scss b/app/assets/stylesheets/common/_multi_select_dropdown.scss new file mode 100644 index 0000000000..c021a0198c --- /dev/null +++ b/app/assets/stylesheets/common/_multi_select_dropdown.scss @@ -0,0 +1,57 @@ +@import 'constants'; +@import 'filter_modal_dropdown'; + +.dropdown.multi-select-dropdown { + @include filter-modal-dropdown; + padding: 0.25em; + + .tags-box { + display: inline-block; + height: 22px; + overflow-y: auto; + width: $dropdown-horizontal - 50px; + + .tag { + border: 1px solid $primary-three; + border-radius: $radius; + display: inline-block; + font-size: x-small; + margin: 1pt; + padding: 0.1em 0.2em; + + span { + margin-right: 0.25em; + } + } + } + + ul { + li { + > div { + display: inline-block; + } + + > span { + margin-left: 2pt; + } + + .fa-square, + .fa-square-check { + color: $primary-one; + } + + &:hover, + &.active { + .fa-square-check, + .fa-square { + color: $background-main; + } + } + } + } + + .options { + display: inline-flex; + padding: 0.25em; + } +} diff --git a/app/assets/stylesheets/common/_single_select_dropdown.scss b/app/assets/stylesheets/common/_single_select_dropdown.scss new file mode 100644 index 0000000000..90422e23d4 --- /dev/null +++ b/app/assets/stylesheets/common/_single_select_dropdown.scss @@ -0,0 +1,15 @@ +@import 'constants'; +@import 'filter_modal_dropdown'; + +.dropdown.single-select-dropdown { + @include filter-modal-dropdown; + + a { + display: inline-block; + height: 17px; + overflow-x: hidden; + overflow-y: auto; + width: 140px; + word-wrap: break-word; + } +} diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index 85429d01b0..44da2ac25d 100644 --- a/app/controllers/results_controller.rb +++ b/app/controllers/results_controller.rb @@ -21,6 +21,7 @@ def show result = record submission = result.submission assignment = submission.assignment + course = assignment.course remark_submitted = submission.remark_submitted? original_result = remark_submitted ? submission.get_original_result : nil is_review = result.is_a_review? @@ -117,6 +118,7 @@ def show end if current_role.instructor? || current_role.ta? + data[:sections] = course.sections.pluck(:name) data[:notes_count] = submission.grouping.notes.count data[:num_marked] = assignment.get_num_marked(current_role.instructor? ? nil : current_role.id) data[:num_collected] = assignment.get_num_collected(current_role.instructor? ? nil : current_role.id) @@ -135,6 +137,10 @@ def show data[:members] = [] end + if current_role.instructor? + data[:tas] = assignment.ta_memberships.joins(:user).distinct.pluck('users.user_name', 'users.display_name') + end + # Marks fields = [:id, :name, :description, :position, :max_mark] criteria_query = { assessment_id: is_review ? assignment.pr_assignment.id : assignment.id } @@ -298,6 +304,7 @@ def remove_tag end def next_grouping + filter_data = params[:filterData] result = record grouping = result.submission.grouping assignment = grouping.assignment @@ -313,7 +320,7 @@ def next_grouping next_result = Result.find_by(id: next_grouping&.result_id) else reversed = params[:direction] != '1' - next_grouping = grouping.get_next_grouping(current_role, reversed) + next_grouping = grouping.get_next_grouping(current_role, reversed, filter_data) next_result = next_grouping&.current_result end diff --git a/app/models/assignment.rb b/app/models/assignment.rb index bf15b4460b..0c6e97a96d 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -879,6 +879,10 @@ def current_results 'results.created_at = sub.results_created_at') end + def current_remark_results + self.current_results.where.not('results.remark_request_submitted_at' => nil) + end + # Query for all non-peer review results for this assignment (for the current submissions) def non_pr_results Result.joins(:grouping) diff --git a/app/models/grouping.rb b/app/models/grouping.rb index 4ba92b859c..b54272bc06 100644 --- a/app/models/grouping.rb +++ b/app/models/grouping.rb @@ -707,37 +707,31 @@ def access_repo end end - def get_next_grouping(current_role, reversed) + def get_next_grouping(current_role, reversed, filter_data = nil) if current_role.ta? - # Get relevant groupings for a TA - groupings = current_role.groupings - .where(assignment: assignment) - .joins(:group) - .order('group_name') + results = self.assignment.current_results.joins(grouping: :tas).where( + 'roles.id': current_role.id + ) + else - # Get all groupings in an assignment -- typically for an instructor - groupings = assignment.groupings.joins(:group).order('group_name') + results = self.assignment.current_results end - if !reversed - # get next grouping with a result - next_grouping = groupings.where('group_name > ?', self.group.group_name).where(is_collected: true).first - else - # get previous grouping with a result - next_grouping = groupings.where('group_name < ?', self.group.group_name).where(is_collected: true).last + results = results.joins(grouping: :group) + if filter_data.nil? + filter_data = {} end - next_grouping + results = filter_results(current_role, results, filter_data) + order_and_get_next_grouping(results, filter_data, reversed) end def get_random_incomplete(current_role) if current_role.ta? - # Get relevant groupings for a TA results = self.assignment.current_results.joins(grouping: :tas).where( marking_state: Result::MARKING_STATES[:incomplete], 'roles.id': current_role.id ) else - # Get all groupings in an assignment -- typically for an instructor results = self.assignment.current_results.where( marking_state: Result::MARKING_STATES[:incomplete] ) @@ -747,6 +741,138 @@ def get_random_incomplete(current_role) private + # Takes in a collection of results specified by +results+, and filters them using +filter_data+. Assumes + # +filter_data+ is not nil. + # +filter_data['annotationText']+ is a string specifying some annotation text to filter by. + # +filter_data['section']+ is a string specifying the name of the section to filter by. + # +filter_data['markingState']+ is a string specifying the marking state to filter by; valid strings + # include "remark_requested", "released", "complete", "in_progress" and "". + # +filter_data['tas']+ is a list of strings corresponding to ta user names specifying the tas to filter by. + # +filter_data['tags']+ is a list of strings corresponding to tag names specifying the tags to filter by. + # +filter_data['totalMarkRange']+ is a hash with the keys 'min' and 'max' each mapping to a string representing a + # float. 'max' is the maximum and 'min' is the minimum total mark a result should have. + # +filter_data['totalExtraMarkRange']+ is a hash with the keys 'min' and 'max' each mapping to a string representing + # a float. 'max' is the maximum and 'min' is the minimum total extra mark a result should have. + # To avoid filtering by any of the specified filters, don't set values for the corresponding key in +filter_data+ + # or set it to nil. If the value for a key is blank (false, empty, or a whitespace string, as determined by + # `.blank?`), no filtering will occur for the corresponding option. + def filter_results(current_role, results, filter_data) + if filter_data['annotationText'].present? + results = results.joins(annotations: :annotation_text) + .where('lower(annotation_texts.content) LIKE ?', + "%#{AnnotationText.sanitize_sql_like(filter_data['annotationText'].downcase)}%") + end + if filter_data['section'].present? + results = results.joins(grouping: :section).where('section.name': filter_data['section']) + end + if filter_data['markingState'].present? + remark_results = results.where.not('results.remark_request_submitted_at': nil) + .where('results.marking_state': Result::MARKING_STATES[:incomplete]) + released_results = results.where.not('results.id': remark_results).where('results.released_to_students': true) + case filter_data['markingState'] + when 'remark_requested' + results = remark_results + when 'released' + results = released_results + when 'complete' + results = results.where.not('results.id': released_results) + .where('results.marking_state': Result::MARKING_STATES[:complete]) + when 'in_progress' + results = results.where.not('results.id': remark_results).where.not('results.id': released_results) + .where('results.marking_state': Result::MARKING_STATES[:incomplete]) + end + end + + unless current_role.ta? || filter_data['tas'].blank? + results = results.joins(grouping: { tas: :user }).where('user.user_name': filter_data['tas']) + end + if filter_data['tags'].present? + results = results.joins(grouping: :tags).where('tags.name': filter_data['tags']) + end + unless filter_data.dig('totalMarkRange', 'max').blank? && filter_data.dig('totalMarkRange', 'min').blank? + result_ids = results.ids + total_marks_hash = Result.get_total_marks(result_ids) + if filter_data.dig('totalMarkRange', 'max').present? + total_marks_hash.select! { |_, value| value <= filter_data['totalMarkRange']['max'].to_f } + end + if filter_data.dig('totalMarkRange', 'min').present? + total_marks_hash.select! { |_, value| value >= filter_data['totalMarkRange']['min'].to_f } + end + results = Result.where('results.id': total_marks_hash.keys) + end + unless filter_data.dig('totalExtraMarkRange', 'max').blank? && filter_data.dig('totalExtraMarkRange', 'min').blank? + result_ids = results.ids + total_marks_hash = Result.get_total_extra_marks(result_ids) + if filter_data.dig('totalExtraMarkRange', 'max').present? + total_marks_hash.select! do |_, value| + value <= filter_data['totalExtraMarkRange']['max'].to_f + end + end + if filter_data.dig('totalExtraMarkRange', 'min').present? + total_marks_hash.select! do |_, value| + value >= filter_data['totalExtraMarkRange']['min'].to_f + end + end + results = Result.where('results.id': total_marks_hash.keys) + end + results.joins(grouping: :group) + end + + # Orders the results, specified as +results+ by using +filter_data+ and returns the next grouping using +reversed+. + # +reversed+ is a boolean value, true to return the next grouping and false to return the previous one. + # +filter_data['orderBy']+ specifies how the results should be ordered, with valid values being "group_name" and + # "submission_date". When this value is not specified (or nil), default ordering is applied. + # +filter_data['ascending']+ specifies whether results should be ordered in ascending or descending order. Valid + # options include "true" (corresponding to ascending order) or "false" (corresponding to descending order). When + # this value is not specified (or nil), the results are ordered in ascending order. + def order_and_get_next_grouping(results, filter_data, reversed) + asc_temp = filter_data['ascending'].nil? || filter_data['ascending'] == 'true' ? 'ASC' : 'DESC' + ascending = (asc_temp == 'ASC' && !reversed) || (asc_temp == 'DESC' && reversed) ? true : false + case filter_data['orderBy'] + when 'submission_date' + next_grouping_ordered_submission_date(results, ascending) + else # group name/otherwise + next_grouping_ordered_group_name(results, ascending) + end + end + + # Gets the next grouping by first ordering +results+ by group name in either ascending + # (+ascending+ = true) or descending (+ascending+ = false) order and then extracting the next grouping. + # If there is no next grouping, nil is returned. + def next_grouping_ordered_group_name(results, ascending) + results = results.group([:id, 'groups.group_name']).order('groups.group_name ASC') + if ascending + next_result = results.where('groups.group_name > ?', self.group.group_name).first + else + next_result = results.where('groups.group_name < ?', self.group.group_name).last + end + next_result&.grouping + end + + # Gets the next grouping by first ordering +results+ by submission date and then by group name in either ascending + # (+ascending+ = true) or descending (+ascending+ = false) order and then extracting the next grouping. + # If there is no next grouping, nil is returned. + def next_grouping_ordered_submission_date(results, ascending) + results = results.joins(:submission).group([:id, 'groups.group_name', 'submissions.revision_timestamp']) + .order('submissions.revision_timestamp ASC', + 'groups.group_name ASC') + if ascending + next_result = results + .where('submissions.revision_timestamp > ?', self.current_submission_used.revision_timestamp) + .or(results.where('groups.group_name > ? AND submissions.revision_timestamp = ?', + self.group.group_name, + self.current_submission_used.revision_timestamp)).first + + else + next_result = results + .where('submissions.revision_timestamp < ?', self.current_submission_used.revision_timestamp) + .or(results.where('groups.group_name < ? AND submissions.revision_timestamp = ?', + self.group.group_name, + self.current_submission_used.revision_timestamp)).last + end + next_result&.grouping + end + def add_assignment_folder(group_repo) assignment_folder = self.assignment.repository_folder diff --git a/config/locales/common/en.yml b/config/locales/common/en.yml index 7f8e7cb8ed..b4471bf61c 100644 --- a/config/locales/common/en.yml +++ b/config/locales/common/en.yml @@ -5,6 +5,7 @@ en: all: All apply: Apply cancel: Cancel + clear_all: Clear All close: Close continue: Continue copy: Copy @@ -51,6 +52,8 @@ en: unlink_confirm: This will unlink the course and any assessments from this LMS. Continue? unlink_courses: Unlink this course markus: MarkUs + max: Max + min: Min not_applicable: N/A or: Or other_info: Other Info @@ -63,5 +66,6 @@ en: select_filename: Select Filename skip: Skip this: This + to: to working: Loading write: Write diff --git a/config/locales/views/results/en.yml b/config/locales/views/results/en.yml index 9a02e189ca..59dd2dd036 100644 --- a/config/locales/views/results/en.yml +++ b/config/locales/views/results/en.yml @@ -29,6 +29,15 @@ en: delete_extra_mark_confirm: Are you sure you want to remove this extra mark? expand_all: Expand All expand_unmarked: Expand Unmarked + filter_submissions: Filter Submissions + filters: + invalid_range: Invalid Range + no_options: No options available + order_by: Order By + ordering: + ascending: Ascending + descending: Descending + text_box_placeholder: Search text fullscreen_enter: Fullscreen fullscreen_exit: Leave fullscreen keybinding: diff --git a/spec/controllers/results_controller_spec.rb b/spec/controllers/results_controller_spec.rb index 6e53d126f0..bf51bccd76 100644 --- a/spec/controllers/results_controller_spec.rb +++ b/spec/controllers/results_controller_spec.rb @@ -44,6 +44,639 @@ def self.test_unauthorized(route_name) end end + shared_examples 'ta and instructor #next_grouping with filters' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping4) { create :grouping, assignment: grouping1.assignment } + let(:groupings) { [grouping1, grouping2, grouping3, grouping4] } + + context 'when annotation text filter is applied' do + let(:annotation_text) { create :annotation_text, content: 'aa_' } + before(:each) do + create :text_annotation, annotation_text: annotation_text, result: grouping1.current_result + create :text_annotation, annotation_text: annotation_text, result: grouping3.current_result + end + + context 'when there are no more filtered submissions in the specified direction' do + it 'should return a response with next_grouping and next_result set to nil' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping3.id, + id: grouping3.current_result.id, + direction: 1, filterData: { annotationText: 'aa_' } } + expect(response.parsed_body['next_grouping']).to be_nil + expect(response.parsed_body['next_result']).to be_nil + end + end + + context 'when there is another filtered result after the current one' do + it 'should return a response with the next filtered group' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { annotationText: 'aa_' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + expect(response.parsed_body['next_result']['id']).to eq(grouping3.current_result.id) + end + + it 'shouldn\'t return the next non-filtered group' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { annotationText: 'aa_' } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + expect(response.parsed_body['next_result']['id']).not_to eq(grouping2.current_result.id) + end + end + + context 'when annotationText contains special characters (in the context of a like clause)' do + it 'should sanitize the string and return the next relevant result' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { annotationText: 'aa_' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + expect(response.parsed_body['next_result']['id']).to eq(grouping3.current_result.id) + end + end + + context 'when we filter by a substring of the desired annotation text' do + it 'should return the next result containing the substring in one of its annotations' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { annotationText: 'a' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + expect(response.parsed_body['next_result']['id']).to eq(grouping3.current_result.id) + end + end + end + + context 'section filter' do + let(:section) { create :section } + before(:each) do + groupings[0].inviter.update(section: section) + groupings[1].inviter.update(section: nil) + groupings[2].inviter.update(section: section) + end + + context 'when a section has been picked' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { section: 'Section 1' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { section: 'Section 1' } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when section is left blank' do + it 'should return the next grouping without constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { section: '' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + end + + context 'marking state filter' do + context 'when remark request is selected' do + let(:grouping2) do + result = create :incomplete_result + result.submission.update(submission_version_used: true) + result.grouping.update(assignment: grouping1.assignment) + result.grouping + end + let(:grouping3) do + remark_result = create :remark_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + let(:grouping4) do + remark_result = create :remark_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + before(:each) do + grouping3.current_result.update(marking_state: Result::MARKING_STATES[:complete]) + end + + it 'should respond with the next grouping with a remark requested and who has a marking state of incomplete' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'remark_requested' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not respond with a grouping whose current result is a remark result but is complete' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'remark_requested' } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping3.id) + end + + it 'should not respond with a grouping whose current result is not a remark result' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'remark_requested' } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when released is selected' do + let(:grouping2) do + result = create :incomplete_result + result.submission.update(submission_version_used: true) + result.grouping.update(assignment: grouping1.assignment) + result.grouping + end + let(:grouping3) do + remark_result = create :complete_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + let(:grouping4) do + remark_result = create :released_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + + it 'should respond with the next grouping whose submission has been released' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'released' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + end + + context 'when complete is selected' do + let(:grouping2) do + result = create :released_result + result.submission.update(submission_version_used: true) + result.grouping.update(assignment: grouping1.assignment) + result.grouping + end + let(:grouping3) do + remark_result = create :remark_result, marking_state: Result::MARKING_STATES[:complete] + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + + it 'should respond with the next grouping whose result is complete regardless of remark request status' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'complete' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not respond with a released result regardless of the result\'s marking status' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'complete' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + context 'when in progress is selected' do + let(:grouping2) do + result = create :remark_result + result.submission.update(submission_version_used: true) + result.grouping.update(assignment: grouping1.assignment) + result.grouping + end + let(:grouping3) do + remark_result = create :released_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + let(:grouping4) do + remark_result = create :incomplete_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + + it 'should respond with the next grouping whose result is incomplete' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'in_progress' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not respond with a released or remark result' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: 'in_progress' } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping3.id) + end + end + + context 'when markingState is left blank' do + let(:grouping2) do + result = create :incomplete_result + result.submission.update(submission_version_used: true) + result.grouping.update(assignment: grouping1.assignment) + result.grouping + end + let(:grouping3) do + remark_result = create :remark_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + let(:grouping4) do + remark_result = create :remark_result + remark_result.submission.update(submission_version_used: true) + remark_result.grouping.update(assignment: grouping1.assignment) + remark_result.grouping + end + + it 'should return the next group regardless of marking state' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { markingState: '' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + end + + context 'when filtering by tags' do + let(:tag1) { create :tag, groupings: [grouping1, grouping3], name: 'tag1' } + let(:tag2) { create :tag, groupings: [grouping2, grouping3], name: 'tag2' } + + context 'when a tag has been picked' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tags: [tag1.name] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tags: [tag1.name] } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when multiple tags have been picked' do + it 'should return the next group with a larger group name that has at least one of the tags' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tags: [tag1.name, tag2.name] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + + context 'when no tag has been picked' do + it 'should return the next grouping without constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tags: [] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + end + + context 'when filtering by total mark' do + let(:grouping4) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:assignment) { grouping1.assignment } + let(:criterion) { create :flexible_criterion, assignment: assignment, max_mark: 10 } + let!(:mark2) do + create :flexible_mark, criterion: criterion, result: grouping2.current_result, assignment: assignment, mark: 6 + end + let!(:mark3) do + create :flexible_mark, criterion: criterion, result: grouping3.current_result, assignment: assignment, mark: 10 + end + let!(:mark4) do + create :flexible_mark, criterion: criterion, result: grouping4.current_result, assignment: assignment, mark: 5 + end + + context 'when no range is provided' do + it 'should return the next grouping without constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: {} } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + + context 'when minimum value is provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { min: 7.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { min: 7.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when maximum value is provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when minimum and maximum values are provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { min: 4.00, max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalMarkRange: { min: 4.00, max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + end + + context 'when filtering by total extra mark' do + let(:grouping4) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:assignment) { grouping1.assignment } + let!(:mark2) { create :extra_mark_points, result: grouping2.current_result, extra_mark: 6 } + let!(:mark3) { create :extra_mark_points, result: grouping3.current_result, extra_mark: 10 } + let!(:mark4) { create :extra_mark_points, result: grouping4.current_result, extra_mark: 5 } + + context 'when no range is provided' do + it 'should return the next grouping without constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: {} } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + + context 'when minimum value is provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { min: 7.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { min: 7.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when maximum value is provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when minimum and maximum values are provided' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { min: 4.00, max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping4.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { totalExtraMarkRange: { min: 4.00, max: 5.00 } } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + end + end + + shared_examples 'instructor and ta #next_grouping with different orderings' do + context 'with 3 groupings' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:groupings) { [grouping1, grouping2, grouping3] } + + context 'order by group name' do + context 'Descending Order' do + context 'direction = 1' do + it 'should return the next grouping in descending order of group name' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: 1, filterData: { ascending: 'false', orderBy: 'group_name' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping1.id) + end + end + + context 'direction = -1' do + it 'should return the previous grouping in descending order of group name' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: -1, filterData: { ascending: 'false', orderBy: 'group_name' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + end + end + + context 'order by submission date' do + context 'Ascending Order' do + context 'when direction = 1' do + context 'when the ordered submission has a different submission date from the current one' do + it 'should return the grouping with the next latest submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: 1, filterData: + { ascending: 'true', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + + context 'when the next ordered submission shares has the same submission date as the current one' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + + before(:each) do + 3.times do |i| + groupings[i].current_submission_used.update(revision_timestamp: Date.current) + end + end + + it 'should return the grouping with the next largest group name with the same submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: 1, filterData: + { ascending: 'true', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + end + + context 'direction = -1' do + context 'when the previous ordered submission has a different submission date from the current one' do + it 'should return the grouping with the next earliest submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: -1, filterData: + { ascending: 'true', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping1.id) + end + end + + context 'when the previous ordered submission shares has the same submission date as the current one' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + before(:each) do + 3.times do |i| + groupings[i].current_submission_used.update(revision_timestamp: Date.current) + end + end + + it 'should return the grouping with the next smallest group name with the same submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: -1, filterData: + { ascending: 'true', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping1.id) + end + end + end + end + + context 'Descending Order' do + context 'direction = 1' do + context 'when the next ordered submission has a different submission date from the current one' do + it 'should return the grouping with the next earliest submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: 1, filterData: + { ascending: 'false', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping1.id) + end + end + + context 'when the next ordered submission shares has the same submission date as the current one' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + before(:each) do + 3.times do |i| + groupings[i].current_submission_used.update(revision_timestamp: Date.current) + end + end + + it 'should return the grouping with the next smallest group name with the same submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: 1, filterData: + { ascending: 'false', orderBy: 'submission_date' } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping1.id) + end + end + end + + context 'direction = -1' do + context 'when the previous ordered submission has a different submission date from the current one' do + it 'should return the grouping with the next latest submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: -1, filterData: { + ascending: 'false', orderBy: 'submission_date' + } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + + context 'when the previous ordered submission shares has the same submission date as the current one' do + let(:grouping1) { create :grouping_with_inviter_and_submission, is_collected: true } + let(:grouping2) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + let(:grouping3) do + create :grouping_with_inviter_and_submission, assignment: grouping1.assignment, is_collected: true + end + before(:each) do + 3.times do |i| + groupings[i].current_submission_used.update(revision_timestamp: Date.current) + end + end + + it 'should return the grouping with the next largest group name with the same submission date' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping2.id, + id: grouping2.current_result.id, + direction: -1, filterData: { + ascending: 'false', orderBy: 'submission_date' + } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + end + end + end + end + end + end + shared_examples 'shared ta and instructor tests' do context 'accessing next_grouping' do it 'should receive 200 when current grouping has a submission' do @@ -934,6 +1567,60 @@ def self.test_unauthorized(route_name) end end + context 'accessing next_grouping' do + let(:grouping1) { create :grouping_with_inviter_and_submission } + let(:grouping2) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:grouping3) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:grouping4) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:groupings) { [grouping1, grouping2, grouping3, grouping4] } + before(:each) { groupings } + include_examples 'ta and instructor #next_grouping with filters' + include_examples 'instructor and ta #next_grouping with different orderings' + + context 'filter by tas' do + let(:ta1) { create :ta } + let(:ta2) { create :ta } + let!(:ta_membership1) { create :ta_membership, role: ta1, grouping: grouping1 } + let!(:ta_membership2) { create :ta_membership, role: ta1, grouping: grouping3 } + let!(:ta_membership3) { create :ta_membership, role: ta2, grouping: grouping3 } + let!(:ta_membership4) { create :ta_membership, role: ta2, grouping: grouping2 } + + context 'when a ta has been picked' do + it 'should return the next group with a larger group name that satisfies the constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tas: [ta1.user.user_name] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping3.id) + end + + it 'should not return the next group that doesn\'t satisfy the constraint' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tas: [ta1.user.user_name] } } + expect(response.parsed_body['next_grouping']['id']).not_to eq(grouping2.id) + end + end + + context 'when multiple tas have been picked' do + it 'should return the next group with a larger group name that has atleast one of the tas' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tas: [ta1.user.user_name, ta2.user.user_name] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + + context 'when no Ta is picked' do + it 'should return the next grouping without constraints' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tas: [] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + end + end + describe '#random_incomplete_submission' do it 'should receive 200 when current grouping has a submission' do allow_any_instance_of(Grouping).to receive(:has_submission).and_return true @@ -967,6 +1654,7 @@ def self.test_unauthorized(route_name) end end end + context 'A TA' do before(:each) { sign_in ta } [:set_released_to_students].each { |route_name| test_unauthorized(route_name) } @@ -1229,5 +1917,42 @@ def self.test_unauthorized(route_name) test_unauthorized(:get_test_runs_instructors) end end + context 'accessing next_grouping with valid permissions' do + let(:grouping1) { create :grouping_with_inviter_and_submission } + let(:grouping2) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:grouping3) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:grouping4) { create :grouping_with_inviter_and_submission, assignment: grouping1.assignment } + let(:groupings) { [grouping1, grouping2, grouping3, grouping4] } + before(:each) do + 3.times do |i| + create :ta_membership, role: ta, grouping: groupings[i] + end + end + context 'ta and instructor #next_grouping with filters' do + before(:each) do + create :ta_membership, role: ta, grouping: groupings[3] + end + include_examples 'ta and instructor #next_grouping with filters' + end + + include_examples 'instructor and ta #next_grouping with different orderings' + context 'filter by tas' do + let(:ta1) { create :ta } + let(:ta2) { create :ta } + let!(:ta_membership1) { create :ta_membership, role: ta1, grouping: grouping1 } + let!(:ta_membership2) { create :ta_membership, role: ta1, grouping: grouping3 } + let!(:ta_membership3) { create :ta_membership, role: ta2, grouping: grouping3 } + let!(:ta_membership4) { create :ta_membership, role: ta2, grouping: grouping2 } + + context 'when a ta has been picked' do + it 'should return the next group with a larger group name and NOT filter by selected ta' do + get :next_grouping, params: { course_id: course.id, grouping_id: grouping1.id, + id: grouping1.current_result.id, + direction: 1, filterData: { tas: [ta1.user.user_name] } } + expect(response.parsed_body['next_grouping']['id']).to eq(grouping2.id) + end + end + end + end end end diff --git a/spec/factories/submissions.rb b/spec/factories/submissions.rb index 54e7aafcfa..85d5320ac4 100644 --- a/spec/factories/submissions.rb +++ b/spec/factories/submissions.rb @@ -3,7 +3,7 @@ association :grouping submission_version { 1 } revision_identifier { 1 } - revision_timestamp { Date.current } + revision_timestamp { Time.current } is_empty { false } factory :version_used_submission do diff --git a/spec/models/grouping_spec.rb b/spec/models/grouping_spec.rb index 9b1402bb67..a3c1c707f7 100644 --- a/spec/models/grouping_spec.rb +++ b/spec/models/grouping_spec.rb @@ -1534,8 +1534,8 @@ def expect_updated_criteria_coverage_count_eq(expected_count) describe '#get_next_group as instructor' do let(:role) { create :instructor } let(:assignment) { create :assignment } - let!(:grouping1) { create :grouping, assignment: assignment, is_collected: true } - let!(:grouping2) { create :grouping, assignment: assignment, is_collected: true } + let!(:grouping1) { create :grouping_with_inviter_and_submission, assignment: assignment } + let!(:grouping2) { create :grouping_with_inviter_and_submission, assignment: assignment } it 'should let one navigate right if there is a result directly to the right' do groupings = assignment.groupings.joins(:group).order('group_name') new_grouping = groupings.first.get_next_grouping(role, false) @@ -1558,7 +1558,7 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end describe 'with collected results separated by an uncollected results' do let!(:grouping2) { create :grouping, assignment: assignment, is_collected: false } - let!(:grouping3) { create :grouping, assignment: assignment, is_collected: true } + let!(:grouping3) { create :grouping_with_inviter_and_submission, assignment: assignment, is_collected: true } it 'should let me navigate to the right if any result exists towards the right' do groupings = assignment.groupings.joins(:group).order('group_name') new_grouping = groupings.first.get_next_grouping(role, false) @@ -1573,9 +1573,15 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end describe '#get_next_group as ta' do let(:assignment) { create :assignment } - let(:role) { create :ta, groupings: assignment.groupings } - let!(:grouping1) { create :grouping, assignment: assignment, is_collected: true } - let!(:grouping2) { create :grouping, assignment: assignment, is_collected: true } + let(:role) { create :ta } + let!(:grouping1) { create :grouping_with_inviter_and_submission, assignment: assignment } + let!(:grouping2) { create :grouping_with_inviter_and_submission, assignment: assignment } + let(:groupings) { [grouping1, grouping2] } + before(:each) do + 2.times do |i| + create :ta_membership, role: role, grouping: groupings[i] + end + end it 'should let one navigate right if there is a result directly to the right' do groupings = assignment.groupings.joins(:group).order('group_name') new_grouping = groupings.first.get_next_grouping(role, false) @@ -1598,7 +1604,10 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end describe 'with collected results separated by an uncollected results' do let!(:grouping2) { create :grouping, assignment: assignment, is_collected: false } - let!(:grouping3) { create :grouping, assignment: assignment, is_collected: true } + let!(:grouping3) { create :grouping_with_inviter_and_submission, assignment: assignment } + before(:each) do + create :ta_membership, role: role, grouping: grouping3 + end it 'should let me navigate to the right if any result exists towards the right' do groupings = assignment.groupings.joins(:group).order('group_name') new_grouping = groupings.first.get_next_grouping(role, false)