MSFT sync: handle adding/removing users near limit

refs INTEROP-6805
flag=microsoft_group_enrollments_syncing

Test plan:
- have a course with one owner
- remove an owner and add a different owner. recall that enrollments
  have to be in an "active" workflow_state to count.
- run a partial sync (`group.syncer_job.run_synchronously :partial`)
- sync should fail, but it should still add the new owner. Check in the
  Microsoft admin console. Both owners will exist in the group.
- run a full sync (`group.syncer_job.run_synchronously`). Check in the
  MS admin console. The old owner should now be removed.
- Repeat but run a full sync instead of a partial sync the first time.

Change-Id: I0cce5a50a51f40ed36f5eea928638c4782b31937
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276019
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>

lib/microsoft_sync/errors.rb

@@ -115,7 +115,8 @@ module MicrosoftSync
       def self.public_message
         I18n.t 'A Microsoft 365 Group must have owners, and no users ' \
                'corresponding to the instructors of the Canvas course could be found on the ' \
-               'Microsoft side.'
+               'Microsoft side. If you recently added and removed course owners, a re-sync ' \
+               'may resolve the issue.'
       end
     end
 

lib/microsoft_sync/syncer_steps.rb

@@ -274,27 +274,44 @@ module MicrosoftSync
     # Execute a MembershipDiff or PartialMembershipDiff -- add and remove
     # users in batches
     def execute_diff(diff)
-      batch_size = GraphService::GROUP_USERS_BATCH_SIZE
-      diff.additions_in_slices_of(batch_size) do |members_and_owners|
+      execute_diff_remove_users(diff)
+      execute_diff_add_users(diff)
+    rescue Errors::MissingOwners
+      # If the group is close to the max number of users, we might need to
+      # remove users first to make room for new users.
+      # e.g.: group has 25000 users, course has 100 removed but 1 added. Need
+      # to remove at least 1 user before we can add 1.
+      #
+      # However, Microsoft will not let you remove the last owner in a group.
+      # So if a course has 1 owner and it is swapped out for a different owner,
+      # we should add the new one first. This is a rare scenario and because
+      # the Microsoft API is eventually consistent, we'd have to wait a bit to
+      # remove the old owner. So just add the new owners, raise the error and
+      # have them manually re-sync.
+      execute_diff_add_users(diff)
+      raise
+    end
+
+    def execute_diff_add_users(diff)
+      diff.additions_in_slices_of(GraphService::GROUP_USERS_BATCH_SIZE) do |members_and_owners|
         skipped = graph_service.add_users_to_group_ignore_duplicates(
           group.ms_group_id, **members_and_owners
         )
         log_batch_skipped(:add, skipped)
       end
+    rescue Errors::MembersQuotaExceeded
+      raise_and_disable_group(MaxMemberEnrollmentsReached)
+    rescue Errors::OwnersQuotaExceeded
+      raise_and_disable_group(MaxOwnerEnrollmentsReached)
+    end
 
-      # Microsoft will not let you remove the last owner in a group, so it's
-      # slightly safer to remove users last in case we need to completely
-      # change owners.
-      diff.removals_in_slices_of(batch_size) do |members_and_owners|
+    def execute_diff_remove_users(diff)
+      diff.removals_in_slices_of(GraphService::GROUP_USERS_BATCH_SIZE) do |members_and_owners|
         skipped = graph_service.remove_group_users_ignore_missing(
           group.ms_group_id, **members_and_owners
         )
         log_batch_skipped(:remove, skipped)
       end
-    rescue Errors::MembersQuotaExceeded
-      raise_and_disable_group(MaxMemberEnrollmentsReached)
-    rescue Errors::OwnersQuotaExceeded
-      raise_and_disable_group(MaxOwnerEnrollmentsReached)
     end
 
     def step_check_team_exists(_mem_data, _job_state_data)

spec/lib/microsoft_sync/syncer_steps_spec.rb

@@ -574,6 +574,7 @@ describe MicrosoftSync::SyncerSteps do
 
     context 'when the Microsoft API says there are too many members in a group' do
       before do
+        allow(graph_service).to receive(:remove_group_users_ignore_missing)
         allow(graph_service).to receive(:add_users_to_group_ignore_duplicates)
           .and_raise(MicrosoftSync::Errors::MembersQuotaExceeded)
       end
@@ -583,6 +584,7 @@ describe MicrosoftSync::SyncerSteps do
 
     context 'when the Microsoft API says there are too many owners in a group' do
       before do
+        allow(graph_service).to receive(:remove_group_users_ignore_missing)
         allow(graph_service).to receive(:add_users_to_group_ignore_duplicates)
           .and_raise(MicrosoftSync::Errors::OwnersQuotaExceeded)
       end
@@ -635,6 +637,25 @@ describe MicrosoftSync::SyncerSteps do
                                                                  tags: { sync_type: sync_type_statsd_tag })
       end
     end
+
+    context "when the last owner is removed but a new owner is added" do
+      it "adds the new owner but raises the error still" do
+        expect(graph_service).to receive(:remove_group_users_ignore_missing)
+          .and_raise(MicrosoftSync::Errors::MissingOwners)
+        expect(graph_service).to receive(:add_users_to_group_ignore_duplicates).twice
+        expect { subject }.to raise_error(MicrosoftSync::Errors::MissingOwners)
+      end
+    end
+
+    context 'when there is an error in adding classes (e.g. too many users)' do
+      it 'still removes users' do
+        err_class = Class.new(StandardError)
+        expect(graph_service).to receive(:remove_group_users_ignore_missing).twice
+        expect(graph_service).to receive(:add_users_to_group_ignore_duplicates)
+          .and_raise(err_class)
+        expect { subject }.to raise_error(err_class)
+      end
+    end
   end
 
   describe '#step_execute_diff' do