Skip to content

Redesign Avenger dashboard #2437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Apr 22, 2023
Merged

Conversation

zhyuhan
Copy link
Contributor

@zhyuhan zhyuhan commented Apr 11, 2023

Description

This is a major redesign of the dashboard at /grading to improve performance and speedup the workflow of Avengers. This is down by migrating the existing submissions table from AG Grid to the headless TanStack Table and components from Tremor.

The demo video recorded for the final Show-and-Tell can be found here.

Changes

The changes in this PR include:

  • Rewrite submissions table using TanStack Table and Tremor
  • Improve column and global filtering
  • Add grading statistics (number of graded/ungraded submission and ungraded assessments)
  • Refactor pages/academy/grading/Grading.tsx to a functional component
  • Implement custom exportCSV function (previously part of AG Grid API)
    • A few columns that don't exist in the data but exported to CSV with empty values (initialGrade, gradeAdjustment, currentGrade, maxGrade`) are removed, is this okay @martin-henz?

Future improvements

There are still a few existing issues that can be improved:

  • The submission/grading statuses cannot be used to tell whether a submission has been unsubmitted by the Avenger (see Visibility of grading icons for resubmissions #2189)
    • Currently when a submission is unsubmitted, the submission status stays as submitted and the grading status stays as graded, while the XP adjustments get reverted. There should be a better way to tell whether a submission has been unsubmitted or not.
  • Allow filtering by assessments through clicking items in the "Ungraded assessments" list
    • The new grading stats allow an Avenger to see which assessments still have ungraded submission. It will be good if they can click on one of them to add a filter.
  • Migrate Tremor to v2 (see changelog)
    • Tremor v1 uses internal Tailwind stylesheets for styling. In v2, the recommended styling solution is to install our own Tailwind. We will have to resolve a lot of style conflicts if we were to upgrade to v2.
  • This redesign is also a first step to migrating away from ag-grid-community and ag-grid-react, which have a combined bundle size of 33MB.

@zhyuhan zhyuhan self-assigned this Apr 11, 2023
@coveralls
Copy link

coveralls commented Apr 12, 2023

Pull Request Test Coverage Report for Build 4673565577

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 16 of 131 (12.21%) changed or added relevant lines in 11 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 34.637%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/pages/academy/Academy.tsx 0 1 0.0%
src/pages/academy/grading/subcomponents/GradingSubmissionFilters.tsx 1 3 33.33%
src/pages/academy/grading/subcomponents/GradingBadges.tsx 3 11 27.27%
src/pages/academy/grading/subcomponents/GradingDashboard.tsx 1 11 9.09%
src/pages/academy/grading/subcomponents/GradingActions.tsx 1 15 6.67%
src/pages/academy/grading/subcomponents/GradingSummary.tsx 1 17 5.88%
src/pages/academy/grading/Grading.tsx 1 26 3.85%
src/pages/academy/grading/subcomponents/GradingSubmissionsTable.tsx 4 43 9.3%
Files with Coverage Reduction New Missed Lines %
src/commons/workspace/WorkspaceReducer.ts 1 94.9%
src/pages/academy/grading/Grading.tsx 3 2.17%
Totals Coverage Status
Change from base Build 4672994200: 0.02%
Covered Lines: 4834
Relevant Lines: 13013

💛 - Coveralls

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hard work! I really like the abstractions and the organization of your code, and of course, the UI looks amazing 🎉🎊! There's just a few minor comments as below.

Well done and keep up the great work :)

@shenyih0ng shenyih0ng requested a review from martin-henz April 20, 2023 04:53
@zhyuhan zhyuhan marked this pull request as ready for review April 20, 2023 07:11
@zhyuhan
Copy link
Contributor Author

zhyuhan commented Apr 20, 2023

  • Implement custom exportCSV function (previously part of AG Grid API)
    • A few columns that don't exist in the data but exported to CSV with empty values (initialGrade, gradeAdjustment, currentGrade, maxGrade`) are removed, is this okay @martin-henz?

Here are the changes to the csv columns.

Before:

"Assessment Name","Student Name","Group","Status","Grading","Question Count","Questions Graded","Initial Grade","Grade Adjustment","Current Grade","Max Grade","Initial XP","XP Adjustment","Current XP (excl. bonus)","Max XP","Bonus XP"
"Mission 1","Dee Sign","1F","submitted","grading","6","2","","","","","0","-2","-2","400","12"
"Mission 0","May Trix","1F","submitted","none","6","0","","","","","996","4","1000","1000","12"
"Mission 0 ","Al Gorithm","1D","submitted","graded","6","6","","","","","69","0","69","100","10"

After:

"Assessment Name","Student Name","Group","Status","Grading","Question Count","Questions Graded","Initial XP","XP Adjustment","Current XP (excl. bonus)","Max XP","Bonus XP"
"Mission 1","Dee Sign","1F","submitted","grading","6","2","0","-2","-2","400","12"
"Mission 0","May Trix","1F","submitted","none","6","0","996","4","1000","1000","12"
"Mission 0 ","Al Gorithm","1D","submitted","graded","6","6","69","0","69","100","10"

@martin-henz
Copy link
Member

  • Implement custom exportCSV function (previously part of AG Grid API)

    • A few columns that don't exist in the data but exported to CSV with empty values (initialGrade, gradeAdjustment, currentGrade, maxGrade`) are removed, is this okay @martin-henz?

Here are the changes to the csv columns.

Before:

"Assessment Name","Student Name","Group","Status","Grading","Question Count","Questions Graded","Initial Grade","Grade Adjustment","Current Grade","Max Grade","Initial XP","XP Adjustment","Current XP (excl. bonus)","Max XP","Bonus XP"
"Mission 1","Dee Sign","1F","submitted","grading","6","2","","","","","0","-2","-2","400","12"
"Mission 0","May Trix","1F","submitted","none","6","0","","","","","996","4","1000","1000","12"
"Mission 0 ","Al Gorithm","1D","submitted","graded","6","6","","","","","69","0","69","100","10"

After:

"Assessment Name","Student Name","Group","Status","Grading","Question Count","Questions Graded","Initial XP","XP Adjustment","Current XP (excl. bonus)","Max XP","Bonus XP"
"Mission 1","Dee Sign","1F","submitted","grading","6","2","0","-2","-2","400","12"
"Mission 0","May Trix","1F","submitted","none","6","0","996","4","1000","1000","12"
"Mission 0 ","Al Gorithm","1D","submitted","graded","6","6","69","0","69","100","10"

I vaguely remember that we need to distinguish between "initial grade" and "adjusted grade" so that we don't lose the result of autograding. Could you check that @zhyuhan ?

Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vaguely remember that we need to distinguish between "initial grade" and "adjusted grade" so that we don't lose the result of autograding. Could you check that @zhyuhan ?

@zhyuhan
Copy link
Contributor Author

zhyuhan commented Apr 22, 2023

I vaguely remember that we need to distinguish between "initial grade" and "adjusted grade" so that we don't lose the result of autograding. Could you check that @zhyuhan ?

@martin-henz @shenyih0ng Looks like Grade was deprecated and replaced with XP in #1878 , so it should be safe to remove the Grade fields as long as we still have the submission's XP.

@martin-henz
Copy link
Member

I vaguely remember that we need to distinguish between "initial grade" and "adjusted grade" so that we don't lose the result of autograding. Could you check that @zhyuhan ?

@martin-henz @shenyih0ng Looks like Grade was deprecated and replaced with XP in #1878 , so it should be safe to remove the Grade fields as long as we still have the submission's XP.

OK, got it.

@martin-henz martin-henz merged commit 05cad2b into master Apr 22, 2023
@martin-henz martin-henz deleted the feat/avenger-dashboard-redesign branch April 22, 2023 11:57
RichDom2185 added a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Jul 15, 2023
* feat: redesign submissions table with basic column filtering

* feat: make new submission action buttons functional

* feat: improving submissions table filtering

* feat: add submissions table filter state to Redux store

* fix: use React Router's `Link` instead of `a` tag

* feat: add grading summary to show progress

* refactor: make `loadContentDispatch` optional

* feat: use new grading dashboard

* feat: move `Export to CSV` button

* fix: sort imports

* fix: skip missing assessments instead of throwing an error

* refactor: add helper to get badge color

* refactor: use nullish coallescing

Co-authored-by: Richard Dominick <[email protected]>

* refactor: remove unnecessary check for Paths

* refactor: reorganize components and logic for badges

* fix: wrap csv fields in quotes

* fix: wraps column headers in quotes too

---------

Co-authored-by: Richard Dominick <[email protected]>
@zhyuhan zhyuhan mentioned this pull request Aug 21, 2023
7 tasks
@RichDom2185 RichDom2185 mentioned this pull request Feb 18, 2024
7 tasks
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.

Grading table: remember state Grading table should remember and run last filter setting
5 participants