diff --git a/lib/microsoft_sync/membership_diff.rb b/lib/microsoft_sync/membership_diff.rb index fc111588148..0b9bd433c3f 100644 --- a/lib/microsoft_sync/membership_diff.rb +++ b/lib/microsoft_sync/membership_diff.rb @@ -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 - diff --git a/lib/microsoft_sync/syncer_steps.rb b/lib/microsoft_sync/syncer_steps.rb index ae3f5d16f79..bb71312cc8d 100644 --- a/lib/microsoft_sync/syncer_steps.rb +++ b/lib/microsoft_sync/syncer_steps.rb @@ -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 diff --git a/spec/lib/microsoft_sync/membership_diff_spec.rb b/spec/lib/microsoft_sync/membership_diff_spec.rb index ef56d96802e..d2072af12a8 100644 --- a/spec/lib/microsoft_sync/membership_diff_spec.rb +++ b/spec/lib/microsoft_sync/membership_diff_spec.rb @@ -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 diff --git a/spec/lib/microsoft_sync/syncer_steps_spec.rb b/spec/lib/microsoft_sync/syncer_steps_spec.rb index e8a1e10fa4f..e8ede0b5fc5 100644 --- a/spec/lib/microsoft_sync/syncer_steps_spec.rb +++ b/spec/lib/microsoft_sync/syncer_steps_spec.rb @@ -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