Skip to content

Implement Graders Column in Assignment Grades tab#7967

Draft
mrafie1 wants to merge 6 commits into
MarkUsProject:masterfrom
mrafie1:implement-graders-column
Draft

Implement Graders Column in Assignment Grades tab#7967
mrafie1 wants to merge 6 commits into
MarkUsProject:masterfrom
mrafie1:implement-graders-column

Conversation

@mrafie1
Copy link
Copy Markdown
Contributor

@mrafie1 mrafie1 commented May 28, 2026

Proposed Changes

(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, use a keyword to link this pull request to the issue.)

  • Changes to assignment_summary_table.jsx
    • Removed all related Graders subcomponent code in the file
    • Added new columnHelper.accessor column for Graders column
    • Graders can be filtered by first name, last name, or username
    • Added function graderDisplay to help display Grader information in the column
  • Changes to assignment_summary_table.test.jsx
    • Removed all related Graders subcomponent tests in the file
    • Added two Describe blocks to separate Group and Graders searching
    • Added Graders searching tests analogous to Group tests
Screenshots of your changes (if applicable)

Example of Assignment Grades tab with new column:

Screenshot 2026-05-26 at 14 24 26
Associated documentation repository pull request (if applicable)

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots) X
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

  • I have performed a self-review of my changes.
    • Check that all changed files included in this pull request are intentional changes.
    • Check that all changes are relevant to the purpose of this pull request, as described above.
  • I have added tests for my changes, if applicable.
    • This is required for all bug fixes and new features.
  • I have updated the project documentation, if applicable.
    • This is required for new features.
  • If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

  • I have updated the project Changelog (this is required for all changes).
  • I have verified that the pre-commit.ci checks have passed.
  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported by Coveralls.
  • I have requested a review from a project maintainer.

Questions and Comments

(Include any questions or comments you have regarding your changes.)

  • Is the format for display graders okay? I am able to change it so that it is in the same format as the previous subcomponent: (username) first name last name
  • Is there a need to write tests that test what happens when filtering by both graders and group?
  • In the assignment_summary_table.jsx file, there is a unused function onFilteredChange. Is it alright if I delete it?

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented May 28, 2026

Coverage Report for CI Build 26616535799

Coverage decreased (-0.02%) to 90.257%

Details

  • Coverage decreased (-0.02%) from the base build.
  • Patch coverage: 1 uncovered change across 1 file (9 of 10 lines covered, 90.0%).
  • 5 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
app/javascript/Components/assignment_summary_table.jsx 10 9 90.0%

Coverage Regressions

5 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
app/javascript/Components/table/table.jsx 5 79.52%

Coverage Stats

Coverage Status
Relevant Lines: 49905
Covered Lines: 46019
Line Coverage: 92.21%
Relevant Branches: 2151
Covered Branches: 965
Branch Coverage: 44.86%
Branches in Coverage %: Yes
Coverage Strength: 122.0 hits per line

💛 - Coveralls

@mrafie1
Copy link
Copy Markdown
Contributor Author

mrafie1 commented May 28, 2026

Working on missing coverage..

@mrafie1 mrafie1 marked this pull request as draft May 29, 2026 04:10
@david-yz-liu
Copy link
Copy Markdown
Collaborator

Hi @mrafie1, thanks for making this draft PR. In response to your questions:

  1. The format is okay but please make the "Graders" column the final column rather than adding it to the front.
  2. No, it's fine to just write tests for the "Graders" column filtering separate from other filtering.
  3. Okay, yes you can delete that function.

@mrafie1
Copy link
Copy Markdown
Contributor Author

mrafie1 commented May 29, 2026

Hi David, thanks for the feedback. Will implement soon.

Quick follow-up questions (similar to my question on slack):

In the file app/javascript/Components/table/table.jsx there are some lines that become uncovered because I removed the tests in app/javascript/Components/__tests__/assignment_summary.test.jsx related to expanderColumns for the subcomponent. I'm not too sure how to proceed with this.

Also, there is a line on line 97 in the assignment_summary_table.jsx file that is not covered. It is the same code as another line of code on line 69 that is uncovered from past PR's. Should I write tests for both lines?

@david-yz-liu
Copy link
Copy Markdown
Collaborator

Hi @mrafie1, don't worry about the loss of test coverage for the table subcomponents code; also don't worry about the missed branch on Line 97.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants