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>
This commit is contained in:
Spencer Olson 2023-08-11 15:33:53 -05:00
parent 5480e07ffa
commit 035ced1e17
2 changed files with 20 additions and 8 deletions

View File

@ -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,

View File

@ -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