Merge pull request #13504 from opf/code-maintenance/49718-reduce-nois…

…e-generated-by-nextcloud-synchronization

[#49718] Reduce noise generated by Nextcloud synchronization.

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/remove_user_from_group_command.rb

@@ -62,7 +62,8 @@ def call(user:, group: @group)
         when "104"
           Util.error(:error, "Insufficient privileges")
         when "105"
-          Util.error(:error, "Failed to remove user from group")
+          message = Nokogiri::XML(response.body).xpath('/ocs/meta/message').text
+          Util.error(:error, "Failed to remove user #{user} from group #{group}: #{message}")
         end
       when Net::HTTPMethodNotAllowed
         Util.error(:not_allowed)

modules/storages/app/services/storages/group_folder_properties_sync_service.rb

@@ -254,7 +254,11 @@ def remove_user_from_group(user)
     @requests
       .remove_user_from_group_command
       .call(user:)
-      .on_failure(&failure_handler('remove_user_from_group_command', { user: }))
+      .on_failure do |service_result|
+      ::OpenProject.logger.warn(
+        "Nextcloud user #{user} has not been removed from Nextcloud group #{@group}: '#{service_result.errors.log_message}'"
+      )
+    end
   end
 
   def hide_inactive_project_folders

modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb

@@ -34,7 +34,6 @@ class Storages::ManageNextcloudIntegrationCronJob < Cron::CronJob
   self.cron_expression = '*/5 * * * *'
 
   def perform
-    result = Storages::NextcloudStorage.sync_all_group_folders
-    raise 'Synchronization is being progressed by another process' if result == false
+    Storages::NextcloudStorage.sync_all_group_folders
   end
 end

modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb

@@ -43,6 +43,6 @@ def self.debounce
 
   def perform
     result = Storages::NextcloudStorage.sync_all_group_folders
-    raise 'Synchronization is being progressed by another process' if result == false
+    self.class.debounce if result == false
   end
 end

modules/storages/spec/common/peripherals/storage_requests_spec.rb

@@ -528,6 +528,35 @@ def origin_name
       expect(result).to be_success
       expect(result.message).to eq("User has been removed from group")
     end
+
+    context 'when Nextcloud reponds with 105 code in the response body' do
+      let(:expected_response_body) do
+        <<~XML
+          <?xml version="1.0"?>
+          <ocs>
+          <meta>
+            <status>failure</status>
+            <statuscode>105</statuscode>
+            <message>Not viable to remove user from the last group you are SubAdmin of</message>
+            <totalitems></totalitems>
+            <itemsperpage></itemsperpage>
+          </meta>
+          <data/>
+          </ocs>
+        XML
+      end
+
+      it 'responds with a failure and parses message from the xml response' do
+        result = subject
+                   .remove_user_from_group_command
+                   .call(user: origin_user_id)
+        expect(result).to be_failure
+        expect(result.errors.log_message).to eq(
+          "Failed to remove user #{origin_user_id} from group OpenProject: " \
+          "Not viable to remove user from the last group you are SubAdmin of"
+        )
+      end
+    end
   end
 
   describe '#create_folder_command' do

modules/storages/spec/services/storages/group_folder_properties_sync_service_spec.rb

@@ -650,12 +650,17 @@
       end
 
       it 'sets project folders properties, but does not remove inactive user from group' do
+        allow(OpenProject.logger).to receive(:warn)
         expect(project_storage1.project_folder_id).to be_nil
         expect(project_storage2.project_folder_id).to eq('123')
 
-        expect do
-          described_class.new(storage).call
-        end.to raise_error(RuntimeError, /remove_user_from_group_command was called with/)
+        described_class.new(storage).call
+
+        expect(OpenProject.logger).to have_received(:warn) do |msg, _|
+          expect(msg).to eq "Nextcloud user Darth Maul has not been removed from Nextcloud group " \
+                            "OpenProject: 'Failed to remove user Darth Maul from group OpenProject: Not viable to remove " \
+                            "user from the last group you are SubAdmin of'"
+        end
 
         expect(request_stubs).to all have_been_requested
         project_storage1.reload

modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb

@@ -41,9 +41,9 @@
       subject
     end
 
-    it 'fails itself when sync has been started by another process' do
+    it 'works out silently without doing anything when sync has been started by another process' do
       allow(Storages::NextcloudStorage).to receive(:sync_all_group_folders).and_return(false)
-      expect { subject }.to raise_error('Synchronization is being progressed by another process')
+      subject
     end
   end
 end

modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb

@@ -77,9 +77,13 @@
       subject
     end
 
-    it 'fails itself when sync has been started by another process' do
+    it 'debounces itself when sync has been started by another process' do
       allow(Storages::NextcloudStorage).to receive(:sync_all_group_folders).and_return(false)
-      expect { subject }.to raise_error('Synchronization is being progressed by another process')
+      allow(described_class).to receive(:debounce)
+
+      subject
+
+      expect(described_class).to have_received(:debounce).once
     end
   end
 end