leave enrollments when a pseudonym is deleted

closes gh-1402

test plan
 - Create a user with two logins
 - have an enrollment tied to the sis id of one
 - delete that pseudonym
 - run a sis import enrollments referencing the
   deleted sis id
 - the enrollment should still be active

Change-Id: I350a998f53aae00662f2a133c17dd9596793ed6a
Reviewed-on: https://gerrit.instructure.com/178116
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
This commit is contained in:
Rob Orton 2019-01-16 09:24:34 -07:00
parent 73a0c5d479
commit 3805e3bc78
2 changed files with 53 additions and 7 deletions

View File

@ -279,13 +279,8 @@ module SIS
end end
if enrollment_info.status =~ /\Aactive/i if enrollment_info.status =~ /\Aactive/i
if user.workflow_state != 'deleted' && pseudo.workflow_state != 'deleted' message = set_enrollment_workflow_state(enrollment, enrollment_info, pseudo, user)
enrollment.workflow_state = 'active' @messages << SisBatch.build_error(enrollment_info.csv, message, sis_batch: @batch, row: enrollment_info.lineno, row_info: enrollment_info.row_info) if message
else
enrollment.workflow_state = 'deleted'
message = "Attempted enrolling of deleted user #{enrollment_info.user_id} in course #{enrollment_info.course_id}"
@messages << SisBatch.build_error(enrollment_info.csv, message, sis_batch: @batch, row: enrollment_info.lineno, row_info: enrollment_info.row_info)
end
elsif enrollment_info.status =~ /\Adeleted/i elsif enrollment_info.status =~ /\Adeleted/i
# we support creating deleted enrollments, but we want to preserve # we support creating deleted enrollments, but we want to preserve
# the state for roll_back_data so only set workflow_state for new # the state for roll_back_data so only set workflow_state for new
@ -389,6 +384,30 @@ module SIS
private private
def set_enrollment_workflow_state(enrollment, enrollment_info, pseudo, user)
message = nil
# the user is active, and the pseudonym is active
if user.workflow_state != 'deleted' && pseudo.workflow_state != 'deleted'
enrollment.workflow_state = 'active'
# the user is active, but the pseudonym is deleted, check for other active pseudonym
elsif user.workflow_state != 'deleted' && pseudo.workflow_state == 'deleted'
if @root_account.pseudonyms.active.where(user_id: user).where("sis_user_id != ? OR sis_user_id IS NULL", enrollment_info.user_id).exists?
enrollment.workflow_state = 'active'
message = "Enrolled a user #{enrollment_info.user_id} in course #{enrollment_info.course_id}, but referenced a deleted sis login"
else
message = invalid_active_enrollment(enrollment, enrollment_info)
end
else # the user is deleted
message = invalid_active_enrollment(enrollment, enrollment_info)
end
message
end
def invalid_active_enrollment(enrollment, enrollment_info)
enrollment.workflow_state = 'deleted'
"Attempted enrolling of deleted user #{enrollment_info.user_id} in course #{enrollment_info.course_id}"
end
def enrollment_needs_due_date_recaching?(enrollment) def enrollment_needs_due_date_recaching?(enrollment)
unless %w(active inactive).include? enrollment.workflow_state_before_last_save unless %w(active inactive).include? enrollment.workflow_state_before_last_save
return %w(active inactive).include? enrollment.workflow_state return %w(active inactive).include? enrollment.workflow_state

View File

@ -930,6 +930,33 @@ describe SIS::CSV::EnrollmentImporter do
expect(student.enrollments.first).to be_deleted expect(student.enrollments.first).to be_deleted
end end
it "do not create enrollments for deleted pseudonyms except when they have an active pseudonym too" do
process_csv_data_cleanly(
"course_id,short_name,long_name,account_id,term_id,status",
"test_1,TC 101,Test Course 101,,,active"
)
process_csv_data_cleanly(
"user_id,login_id,first_name,last_name,email,status",
"student_user,user1,User,Uno,user@example.com,active"
)
p = Pseudonym.where(sis_user_id: "student_user").first
p.user.pseudonyms.create(account: p.account, sis_user_id: 'second_sis', unique_id: 'second_sis')
process_csv_data_cleanly(
"user_id,login_id,first_name,last_name,email,status",
"student_user,user1,User,Uno,user@example.com,deleted"
)
importer = process_csv_data(
"course_id,user_id,role,section_id,status,associated_user_id",
"test_1,student_user,student,,active,",
)
errors = importer.errors.map { |r| r.last }
expect(errors).to eq ["Enrolled a user student_user in course test_1, but referenced a deleted sis login"]
student = p.user
expect(student.enrollments.count).to eq 1
expect(student.enrollments.first).to be_active
end
it "should not enroll users into deleted sections" do it "should not enroll users into deleted sections" do
process_csv_data_cleanly( process_csv_data_cleanly(
"course_id,short_name,long_name,account_id,term_id,status", "course_id,short_name,long_name,account_id,term_id,status",