Add new method to update group service

WIP

app/models/group.rb

@@ -560,16 +560,12 @@ class Group < Namespace
     access_level_roles.values
   end
 
-  def update_two_factor_requirement_for_members
-    members_with_descendants.find_each(&:update_two_factor_requirement)
-  end
-
   private
 
   def update_two_factor_requirement
     return unless saved_change_to_require_two_factor_authentication? || saved_change_to_two_factor_grace_period?
-
-    update_two_factor_requirement_for_members
+    binding.pry
+    members_with_descendants.find_each(&:update_two_factor_requirement)
   end
 
   def path_changed_hook

app/services/groups/update_service.rb

@@ -91,6 +91,18 @@ module Groups
         # don't enqueue immediately to prevent todos removal in case of a mistake
         TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id)
       end
+
+      update_two_factor_requirement_for_subgroups
+    end
+
+    def update_two_factor_requirement_for_subgroups
+      settings = group.namespace_settings
+      return if settings.allow_mfa_for_subgroups
+
+      if settings.previous_changes.include?(:allow_mfa_for_subgroups)
+        # enque in batches members update
+        Disallow2FAWorker.perform_async(group.id)
+      end
     end
 
     def reject_parent_id!

app/services/namespace_settings/update_service.rb

@@ -18,18 +18,6 @@ module NamespaceSettings
       else
         group.build_namespace_settings(settings_params)
       end
-
-      after_update
-    end
-
-    def after_update
-      settings = group.namespace_settings
-      return if settings.allow_mfa_for_subgroups
-
-      if settings.previous_changes.include?(:allow_mfa_for_subgroups)
-        # enque in batches
-        TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id)
-      end
     end
   end
 end

app/workers/disallow_2fa_for_subgroups_members_worker.rb → app/workers/disallow_2fa_for_subgroups_worker.rb

 # frozen_string_literal: true
 
-class Disallow2FAWorker # rubocop:disable Scalability/IdempotentWorker
+class Disallow2FAForSubgroupsWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
   include ExceptionBacktrace
 
@@ -13,6 +13,6 @@ class Disallow2FAWorker # rubocop:disable Scalability/IdempotentWorker
       return
     end
 
-    group.update_two_factor_requirement_for_members
+    group.update!(require_two_factor_authentication: false)
   end
 end

app/workers/disallow_2fa_worker.rb

@@ -9,19 +9,19 @@ class Disallow2FAWorker # rubocop:disable Scalability/IdempotentWorker
   feature_category :subgroups
 
   def perform(group_id)
+    binding.pry
     begin
       group = Group.find(group_id)
     rescue ActiveRecord::RecordNotFound
       return
     end
-    subgroups = group.subgroups.where(require_two_factor_authentication: true)
-    subgroups.update_all(require_two_factor_authentication: false)
 
+    subgroups = group.descendants.where(require_two_factor_authentication: true)
     subgroups.find_each(batch_size: 100).with_index do |subgroup, index| # rubocop: disable CodeReuse/ActiveRecord
       delay = index * INTERVAL
 
-      with_context(subgroup) do
-        Update2FAForSubgroupsMembersWorker.perform_in(delay, subgroup.id)
+      with_context(namespace: subgroup) do
+        Disallow2FAForSubgroupsWorker.perform_in(delay, subgroup.id)
       end
     end
   end

spec/services/groups/update_service_spec.rb

@@ -319,6 +319,12 @@ RSpec.describe Groups::UpdateService do
 
       expect(group.namespace_settings.reload.allow_mfa_for_subgroups).to eq(false)
     end
+
+    it 'enqueues update subgroups and its members' do
+      expect(Disallow2FAWorker).to receive(:perform_async).with(group.id)
+
+      subject
+    end
   end
 
   def update_group(group, user, opts)

spec/workers/disallow_2fa_for_subgroups_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Disallow2FAForSubgroupsWorker do
+  let_it_be(:group) { create(:group, require_two_factor_authentication: true) }
+  let_it_be(:subgroup) { create(:group, parent: group, require_two_factor_authentication: true) }
+  let_it_be(:user) { create(:user, :two_factor, require_two_factor_authentication_from_group: true) }
+  let_it_be(:user_for_subgroup) { create(:user, :two_factor, require_two_factor_authentication_from_group: true) }
+
+  it "updates group" do
+    described_class.new.perform(group.id)
+
+    expect(group.reload.require_two_factor_authentication).to eq(false)
+  end
+
+  it "updates group members" do
+    group.add_user(user, GroupMember::DEVELOPER)
+    binding.pry
+    described_class.new.perform(group.id)
+
+    expect(user.reload.require_two_factor_authentication_from_group).to eq(false)
+  end
+
+  it "updates descendant members" do
+    subgroup.add_user(user_for_subgroup, GroupMember::DEVELOPER)
+
+    described_class.new.perform(group.id)
+
+    expect(user_for_subgroup.reload.require_two_factor_authentication_from_group).to eq(false)
+  end
+end

spec/workers/disallow_2fa_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Disallow2FAWorker do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:subgroup_with_2fa) { create(:group, parent: group, require_two_factor_authentication: true) }
+  let_it_be(:subgroup_without_2fa) { create(:group, parent: group, require_two_factor_authentication: false) }
+  let_it_be(:subsubgroup_with_2fa) { create(:group, parent: subgroup_with_2fa, require_two_factor_authentication: true) }
+
+  it "schedules updating subgroups" do
+    expect(Disallow2FAForSubgroupsWorker).to receive(:perform_in).with(0, subgroup_with_2fa.id)
+    expect(Disallow2FAForSubgroupsWorker).to receive(:perform_in).with(2, subsubgroup_with_2fa.id)
+
+    described_class.new.perform(group.id)
+  end
+end