Skip to content

Commit

Permalink
Fix removing sticky fields from Pseudonym
Browse files Browse the repository at this point in the history
When no changes happened for a user and
clear_sis_stickiness flag was on, the
'stuck_sis_field' didn't get cleared on Pseudonym.

This is because the before_save callback didn't
fire in lib/sticky_sis_fields.rb.

By changing the sis_batch_id on the Pseudonym, we
can force an update (and with that clearing the
sticky fields).

Test plan:
 - Import a user
 - Change its login_id to something
 - Do another SIS import, using the same login_id
   and with setting 'Clear UI-changed state'
 - Observe the 'stuck_sis_fields' gets cleared on
   the Pseudonym. Next SIS import will be able to
   overwrite any property

References SOS-2633

flag=none

Change-Id: I80ee45198863785b4c693375bc54557cc13f66e4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/283658
Reviewed-by: Kozma Jozsef <[email protected]>
Reviewed-by: Richard Zana <[email protected]>
Reviewed-by: August Thornton <[email protected]>
QA-Review: Balazs Komaromi <[email protected]>
Product-Review: Balazs Komaromi <[email protected]>
Tested-by: Service Cloud Jenkins <[email protected]>
  • Loading branch information
balazskomaromi committed Jan 28, 2022
1 parent f4f5eed commit f2b3a9b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/sis/user_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def process_batch(login_only: false)
next
end

if pseudo.changed?
if pseudo.changed? || (Pseudonym.sis_stickiness_options[:clear_sis_stickiness] && pseudo.read_attribute("stuck_sis_fields").present?)
pseudo.sis_batch_id = user_row.sis_batch_id if user_row.sis_batch_id
pseudo.sis_batch_id = @batch.id if @batch
if pseudo.valid?
Expand Down
23 changes: 23 additions & 0 deletions spec/lib/sis/user_importer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,29 @@
expect(Pseudonym.last.deleted_at).not_to be_nil
end

it "clears sticky fields, even when there are no changes for the pseudonym" do
account_model
Setting.set("sis_transaction_seconds", "1")
active_user = SIS::Models::User.new(user_id: "sis_id", login_id: "123", status: "active",
full_name: "User One", email: "[email protected]")
SIS::UserImporter.new(@account, { batch: @account.sis_batches.create! }).process([]) do |importer|
importer.add_user(active_user)
end

Pseudonym.where(unique_id: "123").first.tap do |pseudonym|
pseudonym.unique_id = "321"
pseudonym.save!
end

unchanged_user = SIS::Models::User.new(user_id: "sis_id", login_id: "321", status: "active",
full_name: "User One", email: "[email protected]")
SIS::UserImporter.new(@account, { batch: @account.sis_batches.create!, clear_sis_stickiness: true }).process([]) do |importer|
importer.add_user(unchanged_user)
end

expect(Pseudonym.last.read_attribute("stuck_sis_fields")).to eq ""
end

it "does not update deleted_at property when user gets deleted but workflow_state is stuck" do
account_model
Setting.set("sis_transaction_seconds", "1")
Expand Down

0 comments on commit f2b3a9b

Please sign in to comment.