From 035ced1e17d12a9ca5d8e023461c3f75ebedc67a Mon Sep 17 00:00:00 2001
From: Spencer Olson <solson@instructure.com>
Date: Fri, 11 Aug 2023 15:33:53 -0500
Subject: [PATCH] clear sis_source_id when duplicating an assignment

closes EVAL-3308
flag=none

Sets the unique-by-root-account sis_source_id to nil when creating a
duplicate assignment in order to prevent the unique index from throwing
an error and to allow the duplicate to be saved.

In addition, adds a model-level uniqueness validation for sis_source_id
so that these types of validation errors are caught at the application
level, not at the DB.

Test Plan:
1. Create an assignment.

2. Through API give a sis_assignment_id to the assignment. Using
   https://canvas.instructure.com
      /doc/api/assignments.html#method.assignments_api.update

   assignment[sis_assignment_id]=value

3. Navigate back to the assignments index page and attempt to duplicate
   the assignment. Verify the duplication completes and no errors are
   shown. The duplicate assignment should have a null
   sis_source_id/sis_assignment_id.

Change-Id: Ic3ede54b1d915c6df64c4e4ca40b72b59e53eebd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325022
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Knapp <eknapp@instructure.com>
Reviewed-by: Christopher Soto <christopher.soto@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
Product-Review: Cameron Ray <cameron.ray@instructure.com>
---
 app/models/assignment.rb       | 19 ++++++++++++-------
 spec/models/assignment_spec.rb |  9 ++++++++-
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/app/models/assignment.rb b/app/models/assignment.rb
index 608942b2b6d..17888511687 100644
--- a/app/models/assignment.rb
+++ b/app/models/assignment.rb
@@ -209,6 +209,7 @@ class Assignment < ActiveRecord::Base
   validates :lti_context_id, presence: true, uniqueness: true
   validates :grader_count, numericality: true
   validates :allowed_attempts, numericality: { greater_than: 0 }, unless: proc { |a| a.allowed_attempts == -1 }, allow_nil: true
+  validates :sis_source_id, uniqueness: { scope: :root_account_id }, allow_nil: true
 
   with_options unless: :moderated_grading? do
     validates :graders_anonymous_to_graders, absence: true
@@ -330,12 +331,13 @@ class Assignment < ActiveRecord::Base
     result.ignores.clear
     result.moderated_grading_selections.clear
     result.grades_published_at = nil
-    %i[migration_id
-       lti_context_id
-       turnitin_id
-       discussion_topic
+    %i[discussion_topic
+       integration_data
        integration_id
-       integration_data].each do |attr|
+       lti_context_id
+       migration_id
+       sis_source_id
+       turnitin_id].each do |attr|
       result.send(:"#{attr}=", nil)
     end
     result.peer_review_count = 0
@@ -599,6 +601,11 @@ class Assignment < ActiveRecord::Base
     write_attribute(:allowed_extensions, new_value)
   end
 
+  # ensure a root_account_id is set before validation so that we can
+  # properly validate sis_source_id uniqueness by root_account_id
+  before_validation :set_root_account_id, on: :create
+  before_create :set_muted
+
   before_save :ensure_post_to_sis_valid,
               :process_if_quiz,
               :default_values,
@@ -613,8 +620,6 @@ class Assignment < ActiveRecord::Base
     observer_alerts.in_batches(of: 10_000).delete_all
   end
 
-  before_create :set_root_account_id, :set_muted
-
   after_save  :update_submissions_and_grades_if_details_changed,
               :update_grading_period_grades,
               :touch_assignment_group,
diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb
index 8d9b994cb5f..fbcd8af137e 100644
--- a/spec/models/assignment_spec.rb
+++ b/spec/models/assignment_spec.rb
@@ -1554,6 +1554,13 @@ describe Assignment do
       expect(new_assignment3.title).to eq "Wiki Assignment Copy 3"
     end
 
+    it "does not duplicate the sis_source_id" do
+      assignment = @course.assignments.create!(sis_source_id: "abc")
+      new_assignment = assignment.duplicate
+      expect(new_assignment).to be_valid
+      expect(new_assignment.sis_source_id).to be_nil
+    end
+
     it "does not duplicate grades_published_at" do
       assignment = @course.assignments.create!(title: "whee", points_possible: 10)
       assignment.grades_published_at = Time.zone.now
@@ -10837,7 +10844,7 @@ describe Assignment do
       Assignment.create!(course: @course, name: "some assignment", sis_source_id: "BLAH")
       expect do
         Assignment.create!(course: @course, name: "some assignment", sis_source_id: "BLAH")
-      end.to raise_error(ActiveRecord::RecordNotUnique)
+      end.to raise_error(ActiveRecord::RecordInvalid)
     end
   end