ISSUE-7755: Prevent adding duplicate graders on CSV upload

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)

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)

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

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