MSFT Sync: limit sync to the max allowed by MS Teams
closes INTEROP-6588 flag=microsoft_group_enrollments_syncing test-plan: * Have a course with ms student and ms teacher enrolled; * Decrease the MAX_ENROLLMENT_MEMBERS to the amount of students and teachers enrolled to that course minus one; * Execute the group sync and notice the job will be cancelled gracefully; * Check that group workflow_state=errored and the last_error matches `A Microsoft 365 must have a maximum X members in a team`; * Reset MAX_ENROLLMENT_MEMBERS to previous value; * Decrease the MAX_ENROLLMENT_OWNERS to the amount of teachers enrolled to that course minus one; * Execute the group sync and notice the job will be cancelled gracefully; * Check that group workflow_state=errored and the last_error matches `A Microsoft 365 must have a maximum X owners in a team`; * Reset MAX_ENROLLMENT_OWNERS to previous value; * Execute the group sync and notice that everything still working as expected; Change-Id: I229b223da8a3c8866b7047e64e28ed19bb221879 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263562 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> QA-Review: Evan Battaglia <ebattaglia@instructure.com> Product-Review: Evan Battaglia <ebattaglia@instructure.com> Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
This commit is contained in:
parent
716e5150e6
commit
2d486eb774
|
@ -34,6 +34,8 @@ module MicrosoftSync
|
|||
end
|
||||
|
||||
OWNER_ENROLLMENT_TYPES = %w[TeacherEnrollment TaEnrollment DesignerEnrollment].freeze
|
||||
MAX_ENROLLMENT_MEMBERS = 25_000
|
||||
MAX_ENROLLMENT_OWNERS = 100
|
||||
|
||||
def set_local_member(member, enrollment_type)
|
||||
if OWNER_ENROLLMENT_TYPES.include?(enrollment_type)
|
||||
|
@ -66,6 +68,13 @@ module MicrosoftSync
|
|||
def owners_to_remove
|
||||
@remote_owners - @local_owners
|
||||
end
|
||||
|
||||
def max_enrollment_members_reached?
|
||||
(@local_members + @local_owners).size > MAX_ENROLLMENT_MEMBERS
|
||||
end
|
||||
|
||||
def max_enrollment_owners_reached?
|
||||
@local_owners.size > MAX_ENROLLMENT_OWNERS
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -35,6 +35,8 @@ module MicrosoftSync
|
|||
# GraphServiceHelpers::USERS_UPNS_TO_AADS_BATCH_SIZE:
|
||||
ENROLLMENTS_UPN_FETCHING_BATCH_SIZE = 750
|
||||
STANDARD_RETRY_DELAY = {delay_amount: [5, 20, 100].freeze}.freeze
|
||||
MAX_ENROLLMENT_MEMBERS = MicrosoftSync::MembershipDiff::MAX_ENROLLMENT_MEMBERS
|
||||
MAX_ENROLLMENT_OWNERS = MicrosoftSync::MembershipDiff::MAX_ENROLLMENT_OWNERS
|
||||
|
||||
# SyncCanceled errors are semi-expected errors -- so we raise them they will
|
||||
# cleanup_after_failure but not produce a failed job.
|
||||
|
@ -45,6 +47,9 @@ module MicrosoftSync
|
|||
class MissingOwners < SyncCanceled; end
|
||||
# Can happen when User disables sync on account-level when jobs are running:
|
||||
class TenantMissingOrSyncDisabled < SyncCanceled; end
|
||||
# Can happen when the Course has more then 25k members's enrolled or 100
|
||||
# owner's enrolled
|
||||
class MaxEnrollmentsReached < SyncCanceled; end
|
||||
|
||||
attr_reader :group
|
||||
delegate :course, to: :group
|
||||
|
@ -54,7 +59,7 @@ module MicrosoftSync
|
|||
end
|
||||
|
||||
def initial_step
|
||||
:step_ensure_class_group_exists
|
||||
:step_ensure_max_enrollments_in_a_course
|
||||
end
|
||||
|
||||
def max_retries
|
||||
|
@ -74,7 +79,16 @@ module MicrosoftSync
|
|||
group.update!(last_synced_at: Time.zone.now)
|
||||
end
|
||||
|
||||
# First step of a full sync. Create group on the Microsoft side.
|
||||
# The first step that checks if the max enrollments in a curse were reached
|
||||
# before starting the full sync with the Microsoft side.
|
||||
def step_ensure_max_enrollments_in_a_course(_mem_data, _job_state_data)
|
||||
raise_max_enrollment_members_reached if max_enrollment_members_reached?
|
||||
raise_max_enrollment_owners_reached if max_enrollment_owners_reached?
|
||||
|
||||
StateMachineJob::NextStep.new(:step_ensure_class_group_exists)
|
||||
end
|
||||
|
||||
# Second step of a full sync. Create group on the Microsoft side.
|
||||
def step_ensure_class_group_exists(_mem_data, _job_state_data)
|
||||
# TODO: as we continue building the job we could possibly just use the
|
||||
# group.ms_group_id and if we get an error know we have to create it.
|
||||
|
@ -148,6 +162,7 @@ module MicrosoftSync
|
|||
owners = graph_service_helpers.get_group_users_aad_ids(group.ms_group_id, owners: true)
|
||||
|
||||
diff = MembershipDiff.new(members, owners)
|
||||
|
||||
UserMapping.enrollments_and_aads(course).find_each do |enrollment|
|
||||
diff.set_local_member(enrollment.aad_id, enrollment.type)
|
||||
end
|
||||
|
@ -167,6 +182,9 @@ module MicrosoftSync
|
|||
'Microsoft side.'
|
||||
end
|
||||
|
||||
raise_max_enrollment_members_reached if diff.max_enrollment_members_reached?
|
||||
raise_max_enrollment_owners_reached if diff.max_enrollment_owners_reached?
|
||||
|
||||
batch_size = GraphService::GROUP_USERS_ADD_BATCH_SIZE
|
||||
diff.additions_in_slices_of(batch_size) do |members_and_owners|
|
||||
graph_service.add_users_to_group(group.ms_group_id, members_and_owners)
|
||||
|
@ -236,5 +254,34 @@ module MicrosoftSync
|
|||
def graph_service
|
||||
@graph_service ||= graph_service_helpers.graph_service
|
||||
end
|
||||
|
||||
def max_enrollment_members_reached?
|
||||
course
|
||||
.enrollments
|
||||
.select(:user_id)
|
||||
.limit(MAX_ENROLLMENT_MEMBERS + 1)
|
||||
.distinct
|
||||
.count > MAX_ENROLLMENT_MEMBERS
|
||||
end
|
||||
|
||||
def max_enrollment_owners_reached?
|
||||
course
|
||||
.enrollments
|
||||
.where(type: MicrosoftSync::MembershipDiff::OWNER_ENROLLMENT_TYPES)
|
||||
.select(:user_id)
|
||||
.limit(MAX_ENROLLMENT_OWNERS + 1)
|
||||
.distinct
|
||||
.count > MAX_ENROLLMENT_OWNERS
|
||||
end
|
||||
|
||||
def raise_max_enrollment_members_reached
|
||||
raise MaxEnrollmentsReached, "Microsoft 365 allows a maximum of " \
|
||||
"#{MAX_ENROLLMENT_MEMBERS} members in a team."
|
||||
end
|
||||
|
||||
def raise_max_enrollment_owners_reached
|
||||
raise MaxEnrollmentsReached, "Microsoft 365 allows a maximum of " \
|
||||
"#{MAX_ENROLLMENT_OWNERS} owners in a team."
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -175,4 +175,36 @@ describe MicrosoftSync::MembershipDiff do
|
|||
expect(subject.local_owners).to eq(Set.new(%w[teacher2 teacher4 teacher5 teacher6]))
|
||||
end
|
||||
end
|
||||
|
||||
describe 'max_enrollment_members_reached?' do
|
||||
let(:half) { max/2 }
|
||||
let(:max) { MicrosoftSync::MembershipDiff::MAX_ENROLLMENT_MEMBERS }
|
||||
let(:min) { 1 }
|
||||
|
||||
it 'when the members size is less than or equal to the max enrollment members' do
|
||||
set_local_members 'student', (min..half), member_enrollment_type
|
||||
set_local_members 'teacher', (half...max), owner_enrollment_type
|
||||
expect(subject.max_enrollment_members_reached?).to eq false
|
||||
end
|
||||
|
||||
it 'when the members size is greater than to the max enrollment members' do
|
||||
set_local_members 'student', (min..half), member_enrollment_type
|
||||
set_local_members 'teacher', (half..max), owner_enrollment_type
|
||||
expect(subject.max_enrollment_members_reached?).to eq true
|
||||
end
|
||||
end
|
||||
|
||||
describe 'max_enrollment_owners_reached?' do
|
||||
let(:max) { MicrosoftSync::MembershipDiff::MAX_ENROLLMENT_OWNERS }
|
||||
|
||||
it 'when the owners size is less than or equal to the max enrollment owners' do
|
||||
set_local_members 'teacher', (1...max), owner_enrollment_type
|
||||
expect(subject.max_enrollment_owners_reached?).to eq false
|
||||
end
|
||||
|
||||
it 'when the owners size is greater than to the max enrollment owners' do
|
||||
set_local_members 'teacher', (0..max), owner_enrollment_type
|
||||
expect(subject.max_enrollment_owners_reached?).to eq true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -82,7 +82,8 @@ describe MicrosoftSync::SyncerSteps do
|
|||
describe '#initial_step' do
|
||||
subject { syncer_steps.initial_step }
|
||||
|
||||
it { is_expected.to eq(:step_ensure_class_group_exists) }
|
||||
it { is_expected.to eq(:step_ensure_max_enrollments_in_a_course) }
|
||||
|
||||
it 'references an existing method' do
|
||||
expect { syncer_steps.method(subject.to_sym) }.to_not raise_error
|
||||
end
|
||||
|
@ -132,6 +133,58 @@ describe MicrosoftSync::SyncerSteps do
|
|||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'max of members enrollment reached' do |max_members|
|
||||
it 'raises a graceful exit error informing the user' do
|
||||
expect { subject }.to raise_error do |error|
|
||||
expect(error).to be_a(MicrosoftSync::SyncerSteps::MaxEnrollmentsReached)
|
||||
expect(error).to be_a(MicrosoftSync::StateMachineJob::GracefulCancelErrorMixin)
|
||||
expect(error.public_message).to eq "Microsoft 365 allows a maximum of #{max_members || 25000} members in a team."
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples_for 'max of owners enrollment reached' do |max_owners|
|
||||
it 'raises a graceful exit error informing the user' do
|
||||
expect { subject }.to raise_error do |error|
|
||||
expect(error).to be_a(MicrosoftSync::SyncerSteps::MaxEnrollmentsReached)
|
||||
expect(error).to be_a(MicrosoftSync::StateMachineJob::GracefulCancelErrorMixin)
|
||||
expect(error.public_message).to eq "Microsoft 365 allows a maximum of #{max_owners || 100} owners in a team."
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#step_ensure_max_enrollments_in_a_course' do
|
||||
subject { syncer_steps.step_ensure_max_enrollments_in_a_course(nil, nil) }
|
||||
|
||||
before do
|
||||
n_students_in_course(2, course: course)
|
||||
teacher_in_course(course: course)
|
||||
teacher_in_course(course: course)
|
||||
end
|
||||
|
||||
context 'when the max enrollments in a course was not reached' do
|
||||
it 'schedule the next step `step_ensure_class_group_exists`' do
|
||||
expect_next_step(subject, :step_ensure_class_group_exists)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when max members enrollments was reached in a course' do
|
||||
before do
|
||||
stub_const('MicrosoftSync::SyncerSteps::MAX_ENROLLMENT_MEMBERS', 3)
|
||||
end
|
||||
|
||||
it_should_behave_like 'max of members enrollment reached', 3
|
||||
end
|
||||
|
||||
context 'when max owners enrollments was reached in a course' do
|
||||
before do
|
||||
stub_const('MicrosoftSync::SyncerSteps::MAX_ENROLLMENT_OWNERS', 1)
|
||||
end
|
||||
|
||||
it_behaves_like 'max of owners enrollment reached', 1
|
||||
end
|
||||
end
|
||||
|
||||
describe '#step_ensure_class_group_exists' do
|
||||
subject { syncer_steps.step_ensure_class_group_exists(nil, nil) }
|
||||
|
||||
|
@ -426,6 +479,8 @@ describe MicrosoftSync::SyncerSteps do
|
|||
|
||||
allow(diff).to receive(:owners_to_remove).and_return(Set.new(%w[o1]))
|
||||
allow(diff).to receive(:members_to_remove).and_return(Set.new(%w[m1 m2]))
|
||||
allow(diff).to receive(:max_enrollment_members_reached?).and_return(false)
|
||||
allow(diff).to receive(:max_enrollment_owners_reached?).and_return(false)
|
||||
allow(diff).to \
|
||||
receive(:additions_in_slices_of).
|
||||
with(MicrosoftSync::GraphService::GROUP_USERS_ADD_BATCH_SIZE).
|
||||
|
@ -469,6 +524,22 @@ describe MicrosoftSync::SyncerSteps do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when there are more than `MAX_ENROLLMENT_MEMBERS` enrollments in a course' do
|
||||
before do
|
||||
allow(diff).to receive(:max_enrollment_members_reached?).and_return true
|
||||
end
|
||||
|
||||
it_behaves_like 'max of members enrollment reached'
|
||||
end
|
||||
|
||||
context 'when there are more than `MAX_ENROLLMENT_OWNERS` enrollments in a course' do
|
||||
before do
|
||||
allow(diff).to receive(:max_enrollment_owners_reached?).and_return true
|
||||
end
|
||||
|
||||
it_behaves_like 'max of owners enrollment reached'
|
||||
end
|
||||
end
|
||||
|
||||
describe '#step_check_team_exists' do
|
||||
|
|
Loading…
Reference in New Issue