fix user merge to include placeholder submissions
closes FOO-2586 flag = none Test plan: • Create an assignment and assign it to the one student with a due date (Assignment due date override for individual user) excluding the "everyone" default section scope • Create a second user and merge the first student into the newly created user • Check the submission API for the merged student and assignment and notice that the "cached_due_date" displays the assigned due date used when creating the assignment • See Jira for additional notes and attachments Change-Id: If1ecddc72939fafc373f8aa060ccfd252bd01d8a Change-Id: Idae972fceb4f28d9b1f9a99fb70f3dba0fc0d9e1 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282894 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Ben Rinaca <brinaca@instructure.com> QA-Review: August Thornton <august@instructure.com> Product-Review: August Thornton <august@instructure.com>
This commit is contained in:
parent
c6c76d5156
commit
be7e46587f
|
@ -148,7 +148,6 @@ class DueDateCacher
|
|||
else
|
||||
Set.new
|
||||
end
|
||||
|
||||
@user_ids = Array(user_ids)
|
||||
@update_grades = update_grades
|
||||
@original_caller = original_caller
|
||||
|
|
|
@ -621,6 +621,11 @@ class UserMerge
|
|||
.where.not(unique_id => already_scope.having_submission.select(unique_id))
|
||||
.where.not(id: to_move_ids)
|
||||
.pluck(:id)
|
||||
to_move_ids += scope.without_submission # placeholder submissions
|
||||
.select(unique_id)
|
||||
.where.not(unique_id => already_scope.without_submission.select(unique_id))
|
||||
.where.not(id: to_move_ids)
|
||||
.pluck(:id)
|
||||
to_move = scope.where(id: to_move_ids).to_a
|
||||
move_back = already_scope.where(unique_id => to_move.map(&unique_id)).to_a
|
||||
merge_data.build_more_data(to_move, data: data) unless to_move.empty?
|
||||
|
@ -644,7 +649,7 @@ class UserMerge
|
|||
@data = []
|
||||
end
|
||||
|
||||
def swap_submission(model, move_back, table, to_move, to_move_ids, fk)
|
||||
def swap_submission(model, move_back, table, to_move, to_move_ids, fkey)
|
||||
return if to_move_ids.empty?
|
||||
|
||||
model.transaction do
|
||||
|
@ -653,7 +658,7 @@ class UserMerge
|
|||
# to get around this and to allow us to swap we are setting the
|
||||
# user_id to the negative user_id and then the user_id, after the
|
||||
# conflicting rows have been updated.
|
||||
model.connection.execute("SET CONSTRAINTS #{model.connection.quote_table_name(fk)} DEFERRED")
|
||||
model.connection.execute("SET CONSTRAINTS #{model.connection.quote_table_name(fkey)} DEFERRED")
|
||||
model.where(id: move_back).update_all(user_id: -from_user.id)
|
||||
model.where(id: to_move_ids).update_all(user_id: target_user.id)
|
||||
model.where(id: move_back).update_all(user_id: from_user.id)
|
||||
|
|
|
@ -19,8 +19,8 @@
|
|||
|
||||
describe UserMerge do
|
||||
describe "with simple users" do
|
||||
let!(:user1) { user_model }
|
||||
let!(:user2) { user_model }
|
||||
let!(:user1) { user_model(name: "target_user") }
|
||||
let!(:user2) { user_model(name: "from_user") }
|
||||
let(:course1) { course_factory(active_all: true) }
|
||||
let(:course2) { course_factory(active_all: true) }
|
||||
|
||||
|
@ -161,6 +161,48 @@ describe UserMerge do
|
|||
expect(user1.submissions.map(&:id)).to be_include(s3.id)
|
||||
end
|
||||
|
||||
it "recalculates cached_due_date on unsubmitted placeholder submissions for the new user" do
|
||||
due_date_timestamp = DateTime.now.iso8601
|
||||
course1.enroll_user(user2, "StudentEnrollment", enrollment_state: "active")
|
||||
assignment = course1.assignments.create!(
|
||||
title: "some assignment",
|
||||
grading_type: "points",
|
||||
submission_types: "online_text_entry",
|
||||
due_at: due_date_timestamp
|
||||
)
|
||||
expect(Submission.where(user_id: user2.id, assignment_id: assignment.id).take.cached_due_date)
|
||||
.to eq due_date_timestamp
|
||||
UserMerge.from(user2).into(user1)
|
||||
run_jobs
|
||||
expect(Submission.where(user_id: user1.id, assignment_id: assignment.id).take.cached_due_date)
|
||||
.to eq due_date_timestamp
|
||||
end
|
||||
|
||||
it "recalculates cached_due_date on submissions for assignments with overrides" do
|
||||
due_date_timestamp = DateTime.now.iso8601
|
||||
course1.enroll_user(user2, "StudentEnrollment", enrollment_state: "active")
|
||||
assignment = course1.assignments.create!(
|
||||
title: "Assignment with student due date override",
|
||||
grading_type: "points",
|
||||
submission_types: "online_text_entry"
|
||||
)
|
||||
override = assignment.assignment_overrides.create!(
|
||||
due_at: due_date_timestamp,
|
||||
due_at_overridden: true,
|
||||
all_day: true,
|
||||
unlock_at_overridden: true,
|
||||
lock_at_overridden: true
|
||||
)
|
||||
override.assignment_override_students.create!(user: user2)
|
||||
assignment.update(due_at: nil, only_visible_to_overrides: true)
|
||||
expect(Submission.where(user_id: user2.id, assignment_id: assignment.id).take.cached_due_date)
|
||||
.to eq due_date_timestamp
|
||||
UserMerge.from(user2).into(user1)
|
||||
run_jobs
|
||||
expect(Submission.where(user_id: user1.id, assignment_id: assignment.id).take.cached_due_date)
|
||||
.to eq due_date_timestamp
|
||||
end
|
||||
|
||||
it "does not move or delete submission when both users have submissions" do
|
||||
a1 = assignment_model
|
||||
s1 = a1.find_or_create_submission(user1)
|
||||
|
|
Loading…
Reference in New Issue