feat: instructors can appear as graders by YheChen · Pull Request #8060 · MarkUsProject/Markus

YheChen · 2026-07-15T22:46:07Z

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.)

This PR allows course instructors to be assigned as graders for assignments, alongside TAs.

Displays instructors in the assignment Graders tab.
Supports assigning and unassigning instructors for groups and criteria.
Supports instructor assignment through random and section-based assignment actions.
Allows instructor usernames in group and criterion grader-mapping CSV imports.
Shares grader statistics behavior between instructors and TAs so assignment summaries continue to work.
Prevents unsupported roles and cross-assignment groups from being included in bulk grader assignments.
Adds controller and model test coverage for the new behavior.

This is the first incremental part of #1221.
Ref #1221

Screenshots of your changes (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)	X
🐛 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)	X
🚦 Test update (change that only adds or modifies tests)
📦 Dependency update (change that updates a dependency)
📖 Documentation update (change that updates documentation)
🔧 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.)

coveralls · 2026-07-15T23:01:15Z

Coverage Report for CI Build 29963024176

Coverage increased (+0.02%) to 90.429%

Details

Coverage increased (+0.02%) from the base build.
Patch coverage: 119 of 119 lines across 15 files are fully covered (100%).
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	51372
Covered Lines:	47471
Line Coverage:	92.41%
Relevant Branches:	2470
Covered Branches:	1218
Branch Coverage:	49.31%
Branches in Coverage %:	Yes
Coverage Strength:	127.86 hits per line

💛 - Coveralls

david-yz-liu · 2026-07-17T14:37:59Z

  # is a list of grouping IDs involved in the unassignment. The memberships
  # and groupings must belong to the given assignment +assignment+.
  def self.unassign_tas(ta_membership_ids, grouping_ids, assignment)
+    grouping_ids = assignment.groupings.where(id: grouping_ids).ids


This is a separate bug unrelated to the current task. It's good to fix, but please do so in a separate PR.

david-yz-liu · 2026-07-17T14:38:46Z

-    ta_ids = Ta.where(id: ta_ids).ids
-    grouping_ids = Grouping.where(id: grouping_ids).ids
+    ta_ids = assignment.course.graders.where(id: ta_ids).ids
+    grouping_ids = assignment.groupings.where(id: grouping_ids).ids


This is a separate bug unrelated to the current task. It's good to fix, but please do so in a separate PR.

david-yz-liu · 2026-07-17T14:52:13Z

  has_many :roles
  has_many :instructors
  has_many :tas
+  has_many :graders, -> { where(type: %w[Instructor Ta]) }, class_name: 'Role', inverse_of: :course


Naming here is quite challenging. The current application uses "TAs" and "Graders" as somewhat synonymous, both internally and externally. That's not ideal, but some historical baggage we have to live with now.

Please rename this association to course_staff. It's in some sense more generic (not specifically about grading) but has a fairly intuitive meaning.

Carry this renaming throughout; for example, rename GraderRole to CourseStaffRole

david-yz-liu · 2026-07-17T14:54:47Z

-    ta_ids.each_with_index do |group, index|
-      weightings[group] = (weightings_arr[index].to_f / total * grouping_ids.length).round
-    end
+    weightings = Array(ta_ids).map(&:to_i).zip(weightings_arr).to_h


The changes in this method are unrelated to the current task, and (unlike the ones below) I do not see what problem this is trying to solve. Revert these changes and message me separately about this method.

david-yz-liu · 2026-07-21T00:00:52Z

-    if role && !role.is_a?(Ta)
-      errors.add(:base, :not_ta)
+  def must_be_a_grader
+    if role && !role.is_a?(Ta) && !role.instance_of?(Instructor)


Similar to my above comment, use is_a?

david-yz-liu · 2026-07-21T00:01:08Z

 # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective
 class TaMembership < Membership
-  validate :must_be_a_ta
+  validate :must_be_a_grader


Rename to must_be_course_staff

david-yz-liu · 2026-07-21T00:05:07Z

      models:
        ta_membership:
-          not_ta: User must be a TA
+          not_grader: User must be a TA or instructor


rename this key to not_course_staff

david-yz-liu · 2026-07-21T00:05:34Z

 ## Graders Tab

-The graders tab can be used to assign graders to specific students. Once you assign a grader to a student, they will be able to perform all the same actions as the instructor account (including releasing marks). However, they may only perform these actions on students that have been assigned to them.
+The Graders tab can be used to assign graders to specific student groups and marking criteria. For assignments, both course graders and course instructors are available for assignment. Assigning an instructor can be useful for distributing and tracking marking work, but does not restrict the access they already have as an instructor.


"course graders" -> "course TAs"

david-yz-liu · 2026-07-21T00:06:01Z

-The graders tab can be used to assign graders to specific students. Once you assign a grader to a student, they will be able to perform all the same actions as the instructor account (including releasing marks). However, they may only perform these actions on students that have been assigned to them.
+The Graders tab can be used to assign graders to specific student groups and marking criteria. For assignments, both course graders and course instructors are available for assignment. Assigning an instructor can be useful for distributing and tracking marking work, but does not restrict the access they already have as an instructor.
+
+For marks spreadsheets, graders can be assigned to individual students, but instructors are not included as assignable graders.


Please make this an inset "NOTE" (see how we do this elsewhere in the docs). Also, replace "graders" with "TAs"

david-yz-liu · 2026-07-21T00:06:42Z

 ### Table Info

-When you arrive at the "Manage Graders" page you will see a table of graders:
+When you arrive at the "Manage Graders" page you will see a table of assignable graders. For assignments, this table contains both graders and instructors:


"graders and instructors" -> "TAs and instructors"

david-yz-liu · 2026-07-21T00:07:40Z

 ## Setting up a Grader Account

-If anyone other than the course instructor wants to be able to grade a student (or group's) work, they must have a "Grader" account. This means if a course has 4 TAs and everyone wants to be able to grade work, you must set up 4 separate "Grader" accounts - one for each TA. The course instructor does not require a separate account for grading as all marking can also be done from the admin account.
+Anyone other than a course instructor who needs to grade student work must have a "Grader" account. For example, if a course has 4 TAs who will all grade work, you must set up 4 separate Grader accounts—one for each TA.


"Grader" account -> "TA" account (and same for "Grader accounts")

david-yz-liu · 2026-07-21T00:11:55Z

  has_one :course, through: :assignment

  validate :courses_should_match
+  validate :must_be_a_grader


rename to must_be_course_staff

david-yz-liu · 2026-07-21T00:12:54Z

-      course_tas = assignment.course.tas
-      unless ta_user_names.all? { |g| course_tas.joins(:user).exists?('users.user_name': g) }
+      course_staff = assignment.course.course_staff
+      unless ta_user_names.all? { |g| course_staff.joins(:user).exists?('users.user_name': g) }


please rename ta_user_names to staff_user_names

david-yz-liu · 2026-07-22T16:46:19Z

  private

+  def must_be_course_staff
+    errors.add(:ta, :invalid) if ta && !ta.is_a?(Ta) && !ta.is_a?(Instructor)


I'm sorry, I forgot that AdminRole is a subclass of Instructor. We don't want "admins" to be assigned to be graders. Please revert to instance_of? and the additional tests. But also leave a comment explaining that we use instance_of? to exclude AdminRole.

Oh good point

…nt be graders

david-yz-liu

Nice work, @YheChen!

YheChen added 4 commits July 15, 2026 18:45

instructors can appear as graders

367b87a

changelog

3f04953

Merge branch 'master' into feat/assign-instructors-as-graders

3c02528

docs

c2e7913

YheChen changed the title ~~WIP: feat: instructors can appear as graders~~ feat: instructors can appear as graders Jul 16, 2026

YheChen requested a review from david-yz-liu July 16, 2026 00:57

david-yz-liu reviewed Jul 17, 2026

View reviewed changes

YheChen added 5 commits July 17, 2026 15:11

Merge branch 'master' into feat/assign-instructors-as-graders

60f428e

renamed graderrole to coursestaffrole

6a7c3fd

out of scope for this PR

ea001fa

removed out of scope change

8f035c6

change out of scope of PR

bd3145b

YheChen requested a review from david-yz-liu July 18, 2026 17:48

david-yz-liu reviewed Jul 21, 2026

View reviewed changes

YheChen added 2 commits July 20, 2026 21:13

david changes

e32047b

Merge branch 'master' into feat/assign-instructors-as-graders

e3a8040

YheChen requested a review from david-yz-liu July 21, 2026 01:30

This was referenced Jul 21, 2026

bugfix: cross-assignment grader membership changes #8072

Merged

bugfix: random assignment after grader filtering #8073

Merged

david-yz-liu reviewed Jul 22, 2026

View reviewed changes

YheChen added 2 commits July 22, 2026 18:26

Merge branch 'master' into feat/assign-instructors-as-graders

904ecc4

restore instance of instructor in course staff validations, admins ca…

f1db383

…nt be graders

YheChen requested a review from david-yz-liu July 22, 2026 22:42

david-yz-liu approved these changes Jul 23, 2026

View reviewed changes

david-yz-liu merged commit 22f49a7 into MarkUsProject:master Jul 23, 2026
8 checks passed

YheChen deleted the feat/assign-instructors-as-graders branch July 23, 2026 21:13

Uh oh!

Conversation

YheChen commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Type of Change

Checklist

Questions and Comments

Uh oh!

coveralls commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 29963024176

Coverage increased (+0.02%) to 90.429%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

YheChen commented Jul 15, 2026 •

edited

Loading

coveralls commented Jul 15, 2026 •

edited

Loading