Implement Graders Column in Assignment Grades tab

Changelog.md

@@ -7,6 +7,7 @@
 ### 🚨 Breaking changes
 
 ### ✨ New features and improvements
+- Removed Graders Subcomponent and added a Graders column in the Assignment Grades tab (#7967)
 - Added GET and PATCH /overall_comment API routes (#7963)
 - Add case-sensitive search toggle to group name filters in graders, groups, submissions, and annotation usage tables (#7938)
 

app/javascript/Components/__tests__/assignment_summary.test.jsx

@@ -200,7 +200,10 @@ describe("For the AssignmentSummaryTable's search filter", () => {
             section: null,
             members: [["c8holstg", "Holst", "Gustav", false]],
             tags: [],
-            graders: [["c9varoqu", "Nelle", "Varoquaux"]],
+            graders: [
+              ["c6gehwol", "Severin", "Gehwolf"],
+              ["c9rada", "Mark", "Rada"],
+            ],
             marking_state: "complete",
             final_grade: 6.0,
             criteria: {},
@@ -230,122 +233,75 @@ describe("For the AssignmentSummaryTable's search filter", () => {
     await screen.findByText("group_0001", {exact: false});
   });
 
-  it("filters rows as the user types in the search box", () => {
-    fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0], {
-      target: {value: "0001"},
+  describe("For the Group Column", () => {
+    it("filters rows as the user types in the Group search box", () => {
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0], {
+        target: {value: "0001"},
+      });
+
+      expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
     });
 
-    expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
-    expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
-  });
+    it("restores all rows when the Group search query is cleared", () => {
+      const searchInput = screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0];
+      fireEvent.change(searchInput, {target: {value: "0001"}});
+      fireEvent.change(searchInput, {target: {value: ""}});
 
-  it("restores all rows when the search query is cleared", () => {
-    const searchInput = screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0];
-    fireEvent.change(searchInput, {target: {value: "0001"}});
-    fireEvent.change(searchInput, {target: {value: ""}});
+      expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
+    });
 
-    expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
-    expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
-  });
+    it("shows no rows when the Group search query matches nothing", () => {
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0], {
+        target: {value: "zzznomatch"},
+      });
 
-  it("shows no rows when the search query matches nothing", () => {
-    fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[0], {
-      target: {value: "zzznomatch"},
+      expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
     });
-
-    expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
-    expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
   });
-});
 
-describe("For the AssignmentSummaryTable's subcomponent behavior", () => {
-  let groups_sample;
-  beforeEach(async () => {
-    groups_sample = [
-      {
-        group_name: "group_0001",
-        section: null,
-        members: [
-          ["c9nielse", "Nielsen", "Carl", true],
-          ["c8szyman", "Szymanowski", "Karol", true],
-        ],
-        tags: [],
-        graders: [["c9varoqu", "Nelle", "Varoquaux"]],
-        marking_state: "released",
-        final_grade: 9.0,
-        criteria: {1: 0.0},
-        max_mark: "21.0",
-        result_id: 15,
-        submission_id: 15,
-        total_extra_marks: null,
-      },
-      {
-        group_name: "group_0002",
-        section: "LEC0101",
-        members: [
-          ["c8debuss", "Debussy", "Claude", false],
-          ["c8holstg", "Holst", "Gustav", false],
-        ],
-        tags: [],
-        graders: [["c9varoqu", "Nelle", "Varoquaux"]],
-        marking_state: "released",
-        final_grade: 6.0,
-        criteria: {1: 2.0},
-        max_mark: "21.0",
-        result_id: 5,
-        submission_id: 5,
-        total_extra_marks: null,
-      },
-    ];
-    fetch.mockReset();
-    fetch.mockResolvedValueOnce({
-      ok: true,
-      json: jest.fn().mockResolvedValueOnce({
-        data: groups_sample,
-        criteriaColumns: [
-          {
-            Header: "dolores",
-            accessor: "criteria.1",
-            className: "number",
-            headerClassName: "",
-          },
-        ],
-        numAssigned: 2,
-        numMarked: 2,
-        ltiDeployments: [],
-      }),
-    });
+  describe("For the Graders Column", () => {
+    it("filters rows as the user types in the Graders search box", () => {
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[2], {
+        target: {value: "Mark"},
+      });
 
-    await act(async () => {
-      render(
-        <AssignmentSummaryTable
-          assignment_id={1}
-          course_id={1}
-          is_instructor={false}
-          lti_deployments={[]}
-        />
-      );
+      expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
+
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[2], {
+        target: {value: "Varoquaux"},
+      });
+
+      expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
+
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[2], {
+        target: {value: "c6gehwol"},
+      });
+
+      expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
     });
-  });
 
-  it("should initially hide grader subcomponent content", () => {
-    expect(
-      screen.queryByText(I18n.t("activerecord.models.ta", {count: groups_sample[0].graders.length}))
-    ).not.toBeInTheDocument();
-    expect(
-      screen.queryByText(I18n.t("activerecord.models.ta", {count: groups_sample[1].graders.length}))
-    ).not.toBeInTheDocument();
-  });
+    it("restores all rows when the Graders search query is cleared", () => {
+      const searchInput = screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[2];
+      fireEvent.change(searchInput, {target: {value: "Rada"}});
+      fireEvent.change(searchInput, {target: {value: ""}});
 
-  it("should show grader subcomponent when expanded", async () => {
-    const expanders = await screen.findAllByTestId("expander-button");
+      expect(screen.queryByText(/group_0001/)).toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
+    });
 
-    fireEvent.click(expanders[0]);
+    it("shows no rows when the Graders search query matches nothing", () => {
+      fireEvent.change(screen.getAllByPlaceholderText(defaultSearchPlaceholderText())[2], {
+        target: {value: "zzznomatch"},
+      });
 
-    await waitFor(() => {
-      expect(
-        screen.getByText(I18n.t("activerecord.models.ta", {count: groups_sample[0].graders.length}))
-      ).toBeInTheDocument();
+      expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
+      expect(screen.queryByText(/group_0002/)).not.toBeInTheDocument();
     });
   });
 });

app/javascript/Components/assignment_summary_table.jsx

@@ -78,14 +78,7 @@ export class AssignmentSummaryTable extends React.Component {
               member.slice(0, 3).some(name => name.toLowerCase().includes(filterValue))
             );
 
-            if (member_matches) {
-              return true;
-            }
-
-            // Check grader user names
-            return row.original.graders.some(grader =>
-              grader.some(name => name.toLowerCase().includes(filterValue))
-            );
+            return member_matches;
           } else {
             return true;
           }
@@ -223,7 +216,29 @@ export class AssignmentSummaryTable extends React.Component {
       sortDescFirst: true,
     });
 
-    return [...fixedColumns, ...criteriaColumnDefs, bonusColumn];
+    const gradersColumn = columnHelper.accessor("graders", {
+      id: "graders",
+      header: () => I18n.t("activerecord.models.ta.other"),
+      size: 200,
+      enableResizing: true,
+      cell: props => {
+        const graders = props.row.original.graders;
+        return this.graderDisplay(graders);
+      },
+      filterFn: (row, columnId, filterValue) => {
+        if (filterValue) {
+          filterValue = filterValue.toLowerCase();
+          // Check grader usernames, first names, or last names
+          return row.original.graders.some(grader =>
+            grader.some(name => name.toLowerCase().includes(filterValue))
+          );
+        } else {
+          return true;
+        }
+      },
+    });
+
+    return [...fixedColumns, ...criteriaColumnDefs, bonusColumn, gradersColumn];
   };
 
   toggleShowInactiveGroups = showInactiveGroups => {
@@ -250,6 +265,18 @@ export class AssignmentSummaryTable extends React.Component {
     }
   };
 
+  graderDisplay = graders => {
+    return graders.map((grader, index) => (
+      <span key={index}>
+        {grader[0]}{" "}
+        <span title={grader[1] + " " + grader[2]}>
+          <i className="fa-solid fa-circle-info" />
+        </span>
+        {index < graders.length - 1 ? ", " : ""}
+      </span>
+    ));
+  };
+
   fetchData = () => {
     return fetch(
       Routes.summary_course_assignment_path(this.props.course_id, this.props.assignment_id),
@@ -330,31 +357,6 @@ export class AssignmentSummaryTable extends React.Component {
     return data;
   }
 
-  onFilteredChange = (filtered, column) => {
-    const summaryTable = this.wrappedInstance;
-    if (column.id != "marking_state") {
-      const markingStates = getMarkingStates(summaryTable.state.sortedData);
-      this.setState({
-        marking_states: markingStates,
-        columns: this.getColumns(
-          this.state.criteriaColumns,
-          markingStates,
-          this.state.markingStateFilter
-        ),
-      });
-    } else {
-      const markingStateFilter = filtered.find(filter => filter.id == "marking_state").value;
-      this.setState({
-        markingStateFilter,
-        columns: this.getColumns(
-          this.state.criteriaColumns,
-          this.state.marking_states,
-          markingStateFilter
-        ),
-      });
-    }
-  };
-
   onDownloadTestsModal = () => {
     this.setState({showDownloadTestsModal: true});
   };
@@ -458,8 +460,6 @@ export class AssignmentSummaryTable extends React.Component {
               return {columnFilters: newFilters};
             });
           }}
-          getRowCanExpand={() => true}
-          renderSubComponent={renderSubComponent}
           loading={this.state.loading}
         />
         <DownloadTestResultsModal
@@ -480,20 +480,3 @@ export class AssignmentSummaryTable extends React.Component {
     );
   }
 }
-
-const renderSubComponent = ({row}) => {
-  return (
-    <div>
-      <h4>{I18n.t("activerecord.models.ta", {count: row.original.graders.length})}</h4>
-      <ul>
-        {row.original.graders.map(grader => {
-          return (
-            <li key={grader[0]}>
-              ({grader[0]}) {grader[1]} {grader[2]}
-            </li>
-          );
-        })}
-      </ul>
-    </div>
-  );
-};