From 4a83fb289eec6117f01e057ca20ea82e43580d4a Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:08:24 -0400 Subject: [PATCH 01/13] corner cases for add_members --- spec/controllers/groups_controller_spec.rb | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index ce27a90f89..7280c0a916 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -844,6 +844,7 @@ describe '#add_members' do let(:grouping) { create(:grouping_with_inviter) } + let(:grouping2) { create(:grouping_with_inviter, assignment: grouping.assignment) } let(:student1) { create(:student) } let(:student2) { create(:student) } @@ -856,6 +857,34 @@ expect(grouping.students.size).to eq 3 end + + it 'should return bad request when more than one grouping is selected' do + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping, grouping2], + students: [student1.id], + global_actions: 'assign' } + expect(response).to have_http_status(:bad_request) + end + + it 'should return bad request when no students are selected' do + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [], + global_actions: 'assign' } + expect(response).to have_http_status(:bad_request) + end + + it 'should return bad request when assigning would exceed group_max' do + grouping.assignment.update!(group_max: 1, student_form_groups: true) + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } + expect(response).to have_http_status(:bad_request) + end end describe '#remove_members' do From 8b319446ad60a732edd6a2eb97c546050d717ed4 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:26:32 -0400 Subject: [PATCH 02/13] corner cases for add_member --- spec/controllers/groups_controller_spec.rb | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 7280c0a916..0b57d06ceb 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -887,6 +887,52 @@ end end + describe '#add_member' do + let(:grouping) { create(:grouping) } + let(:student1) { create(:student) } + + it 'should assign inviter status when grouping has no members' do + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } + expect(grouping.student_memberships.reload.find_by(role: student1).membership_status) + .to eq(StudentMembership::STATUSES[:inviter]) + end + + it 'should return bad request when student is already in a group for this assignment' do + create(:grouping_with_inviter, assignment: grouping.assignment, inviter: student1) + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } + expect(response).to have_http_status(:bad_request) + end + + it 'should return bad request when student cannot be invited' do + diff_course_student = create(:student) + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [diff_course_student.id], + global_actions: 'assign' } + expect(response).to have_http_status(:bad_request) + end + + it 'should set warning_grace_day when student has insufficient grace credits' do + student1.update!(grace_credits: 0) + allow_any_instance_of(Grouping).to receive(:grace_period_deduction_single).and_return(1) + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } + expect(assigns(:warning_grace_day)).to be_present + end + end + describe '#remove_members' do let(:grouping) { create(:grouping_with_inviter) } let(:student1) { create(:student) } From 6f1ba73ccce96d88f711dd930dd37d857aee2194 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:31:31 -0400 Subject: [PATCH 03/13] coverage for check_for_groupings --- spec/controllers/groups_controller_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 0b57d06ceb..c09cd4472e 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -842,6 +842,18 @@ end end + describe '#check_for_groupings' do + let(:grouping) { create(:grouping_with_inviter) } + + it 'should return a bad request when no grouping is selected.' do + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [], + global_actions: 'valid' } + expect(response).to have_http_status(:bad_request) + end + end + describe '#add_members' do let(:grouping) { create(:grouping_with_inviter) } let(:grouping2) { create(:grouping_with_inviter, assignment: grouping.assignment) } From 057e73e907bc1ffd0a7689dabe53609fa1150db0 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:37:46 -0400 Subject: [PATCH 04/13] updated changelog # Conflicts: # Changelog.md --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 3967191066..29649414e4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -28,6 +28,7 @@ - Fix: include original total mark in JSON response for remark requests (#7945) ### 🔧 Internal changes +- Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) - Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) - Added tests for `GradersController` to fully cover `grader_groupers_mapping` (#7946) From 8483d958c89c064aeac238c66ab3c9e09b44c039 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:37:53 -0400 Subject: [PATCH 05/13] added self to list of contributors --- doc/markus-contributors.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/markus-contributors.txt b/doc/markus-contributors.txt index 3915d2ac4e..58c39f6d41 100644 --- a/doc/markus-contributors.txt +++ b/doc/markus-contributors.txt @@ -1,4 +1,5 @@ Aaron Lee +Aayush Karki Abdelhamid Benmouffok Adam Goucher Aimen Khan From 506f95223e23011f94d06c680c6a41b6a97e6364 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 17 May 2026 04:46:16 +0000 Subject: [PATCH 06/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- spec/controllers/groups_controller_spec.rb | 62 +++++++++++----------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index c09cd4472e..671971242d 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -847,9 +847,9 @@ it 'should return a bad request when no grouping is selected.' do post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [], - global_actions: 'valid' } + assignment_id: grouping.assignment.id, + groupings: [], + global_actions: 'valid' } expect(response).to have_http_status(:bad_request) end end @@ -872,29 +872,29 @@ it 'should return bad request when more than one grouping is selected' do post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping, grouping2], - students: [student1.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping, grouping2], + students: [student1.id], + global_actions: 'assign' } expect(response).to have_http_status(:bad_request) end it 'should return bad request when no students are selected' do post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [], + global_actions: 'assign' } expect(response).to have_http_status(:bad_request) end it 'should return bad request when assigning would exceed group_max' do grouping.assignment.update!(group_max: 1, student_form_groups: true) post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [student1.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } expect(response).to have_http_status(:bad_request) end end @@ -905,10 +905,10 @@ it 'should assign inviter status when grouping has no members' do post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [student1.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } expect(grouping.student_memberships.reload.find_by(role: student1).membership_status) .to eq(StudentMembership::STATUSES[:inviter]) end @@ -916,20 +916,20 @@ it 'should return bad request when student is already in a group for this assignment' do create(:grouping_with_inviter, assignment: grouping.assignment, inviter: student1) post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [student1.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } expect(response).to have_http_status(:bad_request) end it 'should return bad request when student cannot be invited' do diff_course_student = create(:student) post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [diff_course_student.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [diff_course_student.id], + global_actions: 'assign' } expect(response).to have_http_status(:bad_request) end @@ -937,10 +937,10 @@ student1.update!(grace_credits: 0) allow_any_instance_of(Grouping).to receive(:grace_period_deduction_single).and_return(1) post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], - students: [student1.id], - global_actions: 'assign' } + assignment_id: grouping.assignment.id, + groupings: [grouping], + students: [student1.id], + global_actions: 'assign' } expect(assigns(:warning_grace_day)).to be_present end end From ea50660fca56441d7b4f8e38f22b0074ce164b0b Mon Sep 17 00:00:00 2001 From: Muhammad Date: Sun, 17 May 2026 10:26:21 -0400 Subject: [PATCH 07/13] Added tests to fully cover GradersController#grader_criteria_mapping (#7949) # Conflicts: # Changelog.md --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 29649414e4..45d4571718 100644 --- a/Changelog.md +++ b/Changelog.md @@ -28,6 +28,7 @@ - Fix: include original total mark in JSON response for remark requests (#7945) ### 🔧 Internal changes +- Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) - Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) - Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) From 3d0e9b21a2d2c5a7382b1fb93c997242a1ba3f53 Mon Sep 17 00:00:00 2001 From: Daniel Rafailov <74218740+danielrafailov1@users.noreply.github.com> Date: Mon, 18 May 2026 17:43:31 -0400 Subject: [PATCH 08/13] Fixed flaky test in grade_entry_student_spec.rb (#7958) --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 45d4571718..3db7340237 100644 --- a/Changelog.md +++ b/Changelog.md @@ -28,6 +28,7 @@ - Fix: include original total mark in JSON response for remark requests (#7945) ### 🔧 Internal changes +- Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) - Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) - Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) From 720a58c61c4ceb59339a81e3395ea8bca1091ad4 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Sun, 17 May 2026 00:37:46 -0400 Subject: [PATCH 09/13] updated changelog # Conflicts: # Changelog.md --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 3db7340237..a17809ab4a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -29,6 +29,7 @@ ### 🔧 Internal changes - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) +- Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) - Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) From b687180ca90ede1072b003a281e05b43c2b9993b Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Mon, 18 May 2026 18:15:40 -0400 Subject: [PATCH 10/13] address review comments --- app/controllers/groups_controller.rb | 2 +- spec/controllers/groups_controller_spec.rb | 43 +++++++++++----------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 78ce50cb87..b73b3165a2 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -713,7 +713,7 @@ def add_member(student, grouping, assignment) # Generate a warning if a member is added to a group and they # have fewer grace days credits than already used by that group if student.remaining_grace_credits < grouping.grace_period_deduction_single - @warning_grace_day = I18n.t('groups.grace_day_over_limit', group: grouping.group.group_name) + flash_message(:warning, I18n.t('groups.grace_day_over_limit', group: grouping.group.group_name)) end grouping.reload diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 671971242d..0f5775eebf 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -797,6 +797,15 @@ describe '#validate_groupings' do let(:grouping) { create(:grouping_with_inviter) } + it 'should return a bad request when no grouping is selected.' do + post_as instructor, :global_actions, params: { course_id: course.id, + assignment_id: grouping.assignment.id, + groupings: [], + global_actions: 'valid' } + expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank + end + it 'should validate groupings' do post_as instructor, :global_actions, params: { course_id: course.id, assignment_id: grouping.assignment.id, @@ -842,18 +851,6 @@ end end - describe '#check_for_groupings' do - let(:grouping) { create(:grouping_with_inviter) } - - it 'should return a bad request when no grouping is selected.' do - post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [], - global_actions: 'valid' } - expect(response).to have_http_status(:bad_request) - end - end - describe '#add_members' do let(:grouping) { create(:grouping_with_inviter) } let(:grouping2) { create(:grouping_with_inviter, assignment: grouping.assignment) } @@ -877,6 +874,7 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank end it 'should return bad request when no students are selected' do @@ -886,6 +884,7 @@ students: [], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank end it 'should return bad request when assigning would exceed group_max' do @@ -896,20 +895,17 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank end - end - - describe '#add_member' do - let(:grouping) { create(:grouping) } - let(:student1) { create(:student) } it 'should assign inviter status when grouping has no members' do + empty_grouping = create(:grouping) post_as instructor, :global_actions, params: { course_id: course.id, - assignment_id: grouping.assignment.id, - groupings: [grouping], + assignment_id: empty_grouping.assignment.id, + groupings: [empty_grouping], students: [student1.id], global_actions: 'assign' } - expect(grouping.student_memberships.reload.find_by(role: student1).membership_status) + expect(empty_grouping.student_memberships.reload.find_by(role: student1).membership_status) .to eq(StudentMembership::STATUSES[:inviter]) end @@ -921,6 +917,7 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank end it 'should return bad request when student cannot be invited' do @@ -931,9 +928,10 @@ students: [diff_course_student.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) + expect(flash[:error]).not_to be_blank end - it 'should set warning_grace_day when student has insufficient grace credits' do + it 'should flash a warning when student has insufficient grace credits' do student1.update!(grace_credits: 0) allow_any_instance_of(Grouping).to receive(:grace_period_deduction_single).and_return(1) post_as instructor, :global_actions, params: { course_id: course.id, @@ -941,7 +939,8 @@ groupings: [grouping], students: [student1.id], global_actions: 'assign' } - expect(assigns(:warning_grace_day)).to be_present + expect(flash[:warning]).to be_present + expect(grouping.student_memberships.reload.find_by(role: student1)).to be_present end end From 4eb7fd0df9cca98e5fc356af4c8d3d62300d092c Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Mon, 18 May 2026 20:05:04 -0400 Subject: [PATCH 11/13] fixed failing test --- spec/controllers/groups_controller_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 0f5775eebf..b5dbab40a5 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -922,6 +922,7 @@ it 'should return bad request when student cannot be invited' do diff_course_student = create(:student) + diff_course_student.update!(course: create(:course)) post_as instructor, :global_actions, params: { course_id: course.id, assignment_id: grouping.assignment.id, groupings: [grouping], From 4e84a8587ceb36dba791d3475a7291f8a5c80c91 Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Mon, 18 May 2026 21:11:02 -0400 Subject: [PATCH 12/13] removed duplicated entries --- Changelog.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index a17809ab4a..3d980d3176 100644 --- a/Changelog.md +++ b/Changelog.md @@ -31,9 +31,6 @@ - Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) - Added tests for `GroupsController` to fully cover `global_actions` (#7955) - Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) -- Added tests for `GroupsController` to fully cover `global_actions` (#7955) -- Fixed flaky test `can bulk assign duplicated TAs to grade entry students` in `/spec/models/grade_entry_student_spec.rb` (#7958) -- Added tests for `graders_controller` to fully cover `grader_criteria_mapping` function (#7949) - Added tests for `GradersController` to fully cover `grader_groupers_mapping` (#7946) - Added seed task to assign TAs to A1 groupings and criteria (#7867) - Updated autotest seed files to ensure settings follow tester JSON schema (#7775) From d2180264db1fc154ee10f1a80bbacf7cab8a315d Mon Sep 17 00:00:00 2001 From: Aayush Karki Date: Wed, 20 May 2026 15:16:20 -0400 Subject: [PATCH 13/13] made test assertions more specific --- spec/controllers/groups_controller_spec.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index b5dbab40a5..0ffea5db2c 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -803,7 +803,7 @@ groupings: [], global_actions: 'valid' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.select_a_group')) end it 'should validate groupings' do @@ -874,7 +874,7 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.select_only_one_group')) end it 'should return bad request when no students are selected' do @@ -884,7 +884,7 @@ students: [], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.select_a_student')) end it 'should return bad request when assigning would exceed group_max' do @@ -895,7 +895,7 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.assign_over_limit', group: grouping.group.group_name)) end it 'should assign inviter status when grouping has no members' do @@ -917,7 +917,8 @@ students: [student1.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.invite_member.errors.already_grouped', + user_name: student1.user_name)) end it 'should return bad request when student cannot be invited' do @@ -929,7 +930,8 @@ students: [diff_course_student.id], global_actions: 'assign' } expect(response).to have_http_status(:bad_request) - expect(flash[:error]).not_to be_blank + expect(flash[:error]).to have_message(I18n.t('groups.invite_member.errors.not_found', + user_name: diff_course_student.user_name)) end it 'should flash a warning when student has insufficient grace credits' do @@ -940,7 +942,7 @@ groupings: [grouping], students: [student1.id], global_actions: 'assign' } - expect(flash[:warning]).to be_present + expect(flash[:warning]).to have_message(I18n.t('groups.grace_day_over_limit', group: grouping.group.group_name)) expect(grouping.student_memberships.reload.find_by(role: student1)).to be_present end end