Skip to content

Commit

Permalink
leave enrollments when a pseudonym is deleted
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
roor0 committed Jan 17, 2019
1 parent 73a0c5d commit 3805e3b
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 7 deletions.
33 changes: 26 additions & 7 deletions lib/sis/enrollment_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,8 @@ def process_batch
end

if enrollment_info.status =~ /\Aactive/i
if user.workflow_state != 'deleted' && pseudo.workflow_state != 'deleted'
enrollment.workflow_state = 'active'
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
message = set_enrollment_workflow_state(enrollment, enrollment_info, pseudo, user)
@messages << SisBatch.build_error(enrollment_info.csv, message, sis_batch: @batch, row: enrollment_info.lineno, row_info: enrollment_info.row_info) if message
elsif enrollment_info.status =~ /\Adeleted/i
# we support creating deleted enrollments, but we want to preserve
# the state for roll_back_data so only set workflow_state for new
Expand Down Expand Up @@ -389,6 +384,30 @@ def incrementally_update_account_associations

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)
unless %w(active inactive).include? enrollment.workflow_state_before_last_save
return %w(active inactive).include? enrollment.workflow_state
Expand Down
27 changes: 27 additions & 0 deletions spec/lib/sis/csv/enrollment_importer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,33 @@
expect(student.enrollments.first).to be_deleted
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
process_csv_data_cleanly(
"course_id,short_name,long_name,account_id,term_id,status",
Expand Down

0 comments on commit 3805e3b

Please sign in to comment.