From 29f84b89701ca167090f2610502f884cbf24f31c Mon Sep 17 00:00:00 2001 From: Mateo Date: Tue, 13 Jan 2026 15:49:55 -0500 Subject: [PATCH 1/2] ISSUE-7755: Update changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index e096873ab6..e531418820 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ - Update autotest settings form UI (#7777) - Store start and end date for courses (#7783) - Update batch test runs table UI (#7790) +- Prevent adding duplicate grader group mappings on CSV upload (#7786) ### 🐛 Bug fixes - Fixed the editing form of marking schemes to include newly added assessments (#7788) From f5ef49e757912b69eeec4dccdb743f1ea9aa5e2e Mon Sep 17 00:00:00 2001 From: Naragod Date: Thu, 22 Jan 2026 22:39:16 -0500 Subject: [PATCH 2/2] ISSUE-7755: Prevent duplicates from being added --- app/models/ta_membership.rb | 11 ++++++ spec/controllers/graders_controller_spec.rb | 40 +++++++++++++++++++++ spec/models/ta_memberships_spec.rb | 21 +++++++++++ 3 files changed, 72 insertions(+) diff --git a/app/models/ta_membership.rb b/app/models/ta_membership.rb index 98ecf5ed28..6befb9ce64 100644 --- a/app/models/ta_membership.rb +++ b/app/models/ta_membership.rb @@ -37,6 +37,17 @@ def self.from_csv(assignment, csv_data, remove_existing) end end + # Dedupe within the CSV and remove already-existing memberships + new_ta_memberships.uniq! + unless new_ta_memberships.empty? + existing_pairs = TaMembership + .where(grouping_id: new_ta_memberships.pluck(:grouping_id), + role_id: new_ta_memberships.pluck(:role_id)) + .pluck(:grouping_id, :role_id) + .to_set + new_ta_memberships.reject! { |m| existing_pairs.include?([m[:grouping_id], m[:role_id]]) } + end + Repository.get_class.update_permissions_after do unless new_ta_memberships.empty? TaMembership.insert_all(new_ta_memberships) diff --git a/spec/controllers/graders_controller_spec.rb b/spec/controllers/graders_controller_spec.rb index 26b8b7877d..9028c991c2 100644 --- a/spec/controllers/graders_controller_spec.rb +++ b/spec/controllers/graders_controller_spec.rb @@ -183,6 +183,46 @@ expect(response).to be_redirect expect(@grouping4.tas.count).to eq 0 end + + it 'and uploading the same mappings twice does not create duplicates' do + @ta1 = create(:ta, user: create(:end_user, user_name: 'g9browni')) + @ta2 = create(:ta, user: create(:end_user, user_name: 'g9younas')) + @ta3 = create(:ta, user: create(:end_user, user_name: 'c7benjam')) + @grouping1 = create(:grouping, + assignment: @assignment, + group: create(:group, course: @assignment.course, group_name: 'test_group')) + @grouping2 = create(:grouping, + assignment: @assignment, + group: create(:group, course: @assignment.course, group_name: 'second_test_group')) + @grouping3 = create(:grouping, + assignment: @assignment, + group: create(:group, course: @assignment.course, group_name: 'Group 3')) + + # First upload + post_as @instructor, + :upload, + params: { course_id: course.id, assignment_id: @assignment.id, + upload_file: @group_grader_map_file, groupings: true } + + expect(response).to be_redirect + expect(@grouping1.tas.count).to eq 2 + + # Second upload of the same file without removing existing mappings + post_as @instructor, + :upload, + params: { course_id: course.id, assignment_id: @assignment.id, + upload_file: @group_grader_map_file, groupings: true } + + expect(response).to be_redirect + # Should still have the same count, no duplicates + expect(@grouping1.tas.count).to eq 2 + expect(@grouping1.tas).to include(@ta1) + expect(@grouping1.tas).to include(@ta2) + expect(@grouping2.tas.count).to eq 1 + expect(@grouping2.tas).to include(@ta1) + expect(@grouping3.tas.count).to eq 1 + expect(@grouping3.tas).to include(@ta3) + end end context 'doing a POST on :upload (assigning to criteria)' do diff --git a/spec/models/ta_memberships_spec.rb b/spec/models/ta_memberships_spec.rb index 3922dca95b..7e66b46144 100644 --- a/spec/models/ta_memberships_spec.rb +++ b/spec/models/ta_memberships_spec.rb @@ -29,4 +29,25 @@ membership.destroy end end + + describe '.from_csv' do + let(:assignment) { create(:assignment) } + let(:ta) { create(:ta, course: assignment.course) } + let(:grouping) { create(:grouping, assignment: assignment) } + + it 'does not create duplicate memberships when same grader-group pair exists' do + create(:ta_membership, role: ta, grouping: grouping) + csv_data = "#{grouping.group.group_name},#{ta.user.user_name}" + + expect { TaMembership.from_csv(assignment, csv_data, false) } + .not_to(change { TaMembership.count }) + end + + it 'handles duplicate entries within the same CSV' do + csv_data = "#{grouping.group.group_name},#{ta.user.user_name}\n#{grouping.group.group_name},#{ta.user.user_name}" + + expect { TaMembership.from_csv(assignment, csv_data, false) } + .to change { TaMembership.count }.by(1) + end + end end