MSFT Sync: Ignore fake enrollments in Partial Sync

Note that even previous to this if the job were enqueued the fake user's
username is a random hash and the SIS ID (?) and email are not present so
the user would never really get added. But it's a waste of time to kick
off a job.

Test plan:
- Have a course set up with sync
- Click "Student View" on the upper right corner of the page on the
  page for the course
- Check that there is now a StudentViewEnrollment
- Check that the PartialSyncChanges and Delayed::Job tables are
  unchanged and there is no job

closes INTEROP-6922
flag=microsoft_group_enrollments_syncing

Change-Id: I2e6ba915355a45875e3a24064ab01084eed650a7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/269926
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ryan Hawkins <ryan.hawkins@instructure.com>
QA-Review: Ryan Hawkins <ryan.hawkins@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This commit is contained in:
Evan Battaglia 2021-07-23 16:59:08 -06:00
parent 920781bca8
commit e45eedb53f
4 changed files with 21 additions and 5 deletions

View File

@ -1573,6 +1573,7 @@ class Enrollment < ActiveRecord::Base
end
def sync_microsoft_group
return if self.type == 'StudentViewEnrollment'
return unless self.root_account.feature_enabled?(:microsoft_group_enrollments_syncing)
return unless self.root_account.settings[:microsoft_sync_enabled]

View File

@ -363,7 +363,7 @@ module MicrosoftSync
mappings.each { |user_id, aad_id| diff.set_member_mapping(user_id, aad_id) }
users_with_mappings = mappings.map(&:first)
enrollments = Enrollment.active
enrollments = Enrollment.active.not_fake
.where(course: course, user_id: users_with_mappings)
.pluck(:user_id, :type)
enrollments.each { |user_id, enrollment_type| diff.set_local_member(user_id, enrollment_type) }

View File

@ -858,6 +858,12 @@ describe MicrosoftSync::SyncerSteps do
expect(diff).to have_received(:set_local_member).with(students[2].id, 'StudentEnrollment')
end
it 'ignores StudentViewEnrollment (fake) enrollments' do
Enrollment.where(course: course, user: students[0]).update_all(type: 'StudentViewEnrollment')
subject
expect(diff).to_not have_received(:set_local_member).with(students[0].id, anything)
end
it_behaves_like 'a step that executes a diff' do
# the latter it_behaves_like must be inside this one because 'a step
# that executes a diff' makes the diff return actions. We need actions

View File

@ -3478,6 +3478,7 @@ describe Enrollment do
describe '#sync_microsoft_group' do
let(:course) { course_factory }
let(:enrollment_type) { 'StudentEnrollment' }
before :each do
MicrosoftSync::Group.create!(course: course)
@ -3486,7 +3487,7 @@ describe Enrollment do
# enroll user without running callbacks like update_user_account_associations,
# so that the only jobs getting enqueued are the MSFT sync group type
def enroll_user
course.enroll_user(user_factory, 'StudentViewEnrollment', skip_touch_user: true)
course.enroll_user(user_factory, enrollment_type, skip_touch_user: true)
end
context 'when feature flag is off' do
@ -3495,7 +3496,7 @@ describe Enrollment do
end
it 'does not enqueue a job' do
expect { enroll_user }.not_to change { Delayed::Job.count }
expect { enroll_user }.to not_change { Delayed::Job.where(tag: 'MicrosoftSync::StateMachineJob#run_later').count }
end
end
@ -3511,7 +3512,7 @@ describe Enrollment do
end
it 'does not enqueue a job' do
expect { enroll_user }.not_to change { Delayed::Job.count }
expect { enroll_user }.to not_change { Delayed::Job.where(tag: 'MicrosoftSync::StateMachineJob#run_later').count }
end
end
@ -3522,13 +3523,21 @@ describe Enrollment do
end
it 'should enqueue a job' do
expect { enroll_user }.to change { Delayed::Job.count }.by 1
expect { enroll_user }.to change { Delayed::Job.where(tag: 'MicrosoftSync::StateMachineJob#run_later').count }.by 1
end
it 'calls MicrosoftSync::Group#enqueue_future_partial_sync' do
expect_any_instance_of(MicrosoftSync::Group).to receive(:enqueue_future_partial_sync)
enroll_user
end
context 'when the enrollment is a StudentViewEnrollment' do
let(:enrollment_type) { 'StudentViewEnrollment' }
it 'should not enqueue a job' do
expect { enroll_user }.to not_change { Delayed::Job.where(tag: 'MicrosoftSync::StateMachineJob#run_later').count }
end
end
end
end
end