Reorganizes the code to ease error reporting

Gemfile

@@ -209,9 +209,9 @@ gem "mini_magick", "~> 4.13.0", require: false
 gem "validate_url"
 
 # Storages support code
+gem "dry-auto_inject"
 gem "dry-container"
 gem "dry-monads"
-gem "dry-auto_inject"
 
 # ActiveRecord extension which adds typecasting to store accessors
 gem "store_attribute", "~> 1.0"

modules/storages/app/common/storages/peripherals/managed_folder_identifier/nextcloud.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 #++
 
@@ -10,8 +12,12 @@ module Storages
           @project = project_storage.project
         end
 
+        def name
+          "#{@project.name.tr('/', '|')} (#{@project.id})"
+        end
+
         def path
-          "/#{@storage.group_folder}/#{@project.name.tr('/', '|')} (#{@project.id})/"
+          "/#{@storage.group_folder}/#{name}/"
         end
 
         def location

modules/storages/app/common/storages/peripherals/managed_folder_identifier/one_drive.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 #++
 
@@ -12,6 +14,10 @@ module Storages
           @project = project_storage.project
         end
 
+        def name
+          path
+        end
+
         def path
           "#{@project.name.gsub(CHARACTER_BLOCKLIST, '_')} (#{@project.id})"
         end

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

@@ -43,7 +43,7 @@ module Storages
 
           def call(path:)
             query_params = { depth: "1", path:, props: %w[oc:fileid] }
-            Rails.logger.tagged(self.class).info "Requesting File Ids with the following args: #{query_params.inspect}"
+            Rails.logger.tagged(self.class).info "Requesting File Ids on path: #{path} and args: #{query_params.inspect}"
             @query.call(**query_params)
           end
         end

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

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) 2012-2024 the OpenProject GmbH
@@ -222,7 +224,7 @@ module Storages
             # https://github.com/nextcloud/server/blob/66648011c6bc278ace57230db44fd6d63d67b864/lib/public/Files/DavUtil.php
             result = []
             result << :readable if permissions_string.include?("G")
-            result << :writeable if %w[CK W].reduce(false) { |s, v| s || permissions_string.include?(v) }
+            result << :writeable if permissions_string.match?(/W|CK/)
             result
           end
         end

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/internal/propfind_query_legacy.rb

@@ -34,35 +34,9 @@ module Storages
       module Nextcloud
         module Internal
           class PropfindQueryLegacy
-            # Only for information purposes currently.
-            # Probably a bit later we could validate `#call` parameters.
-            #
-            # DEPTH = %w[0 1 infinity].freeze
-            # POSSIBLE_PROPS = %w[
-            #   d:getlastmodified
-            #   d:getetag
-            #   d:getcontenttype
-            #   d:resourcetype
-            #   d:getcontentlength
-            #   d:permissions
-            #   d:size
-            #   oc:id
-            #   oc:fileid
-            #   oc:favorite
-            #   oc:comments-href
-            #   oc:comments-count
-            #   oc:comments-unread
-            #   oc:owner-id
-            #   oc:owner-display-name
-            #   oc:share-types
-            #   oc:checksums
-            #   oc:size
-            #   nc:has-preview
-            #   nc:rich-workspace
-            #   nc:contained-folder-count
-            #   nc:contained-file-count
-            #   nc:acl-list
-            # ].freeze
+            def self.call(storage:, depth:, path:, props:)
+              new(storage).call(depth:, path:, props:)
+            end
 
             def initialize(storage)
               @storage = storage
@@ -71,10 +45,6 @@ module Storages
               @group = storage.group
             end
 
-            def self.call(storage:, depth:, path:, props:)
-              new(storage).call(depth:, path:, props:)
-            end
-
             # rubocop:disable Metrics/AbcSize
             def call(depth:, path:, props:)
               Rails.logger.tagged(self.class) do
@@ -87,32 +57,33 @@ module Storages
                     xml["d"].prop do
                       props.each do |prop|
                         namespace, property = prop.split(":")
-                        xml[namespace].send(property)
+                        xml[namespace].public_send(property)
                       end
                     end
                   end
                 end.to_xml
 
-              response = OpenProject
-                           .httpx
-                           .basic_auth(@username, @password)
-                           .with(headers: { "Depth" => depth })
-                           .request(
-                             "PROPFIND",
-                             UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path),
-                             xml: body
-                           )
+                response = OpenProject
+                             .httpx
+                             .basic_auth(@username, @password)
+                             .with(headers: { "Depth" => depth })
+                             .request(
+                               "PROPFIND",
+                               UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path),
+                               xml: body
+                             )
 
                 error_data = StorageErrorData.new(source: self.class, payload: response)
 
-              case response
-              in { status: 200..299 }
-                log_response(response)
-                doc = Nokogiri::XML(response.body.to_s)
-                Rails.logger.info "Parsing response body"
-                result = doc.xpath("/d:multistatus/d:response").each_with_object({}) do |resource_section, hash|
-                  source_path = UrlBuilder.path(@storage.uri.path, "/remote.php/dav/files", @username)
-                  resource = CGI.unescape(resource_section.xpath("d:href").text.strip).gsub!(source_path, "")
+                case response
+                in { status: 200..299 }
+                  log_response(response)
+                  log_message "Parsing XML response body"
+                  doc = Nokogiri::XML(response.body.to_s)
+                  Rails.logger.info "Parsing response body"
+                  result = doc.xpath("/d:multistatus/d:response").each_with_object({}) do |resource_section, hash|
+                    source_path = UrlBuilder.path(@storage.uri.path, "/remote.php/dav/files", @username)
+                    resource = CGI.unescape(resource_section.xpath("d:href").text.strip).gsub!(source_path, "")
 
                     hash[resource] = {}
 
@@ -123,7 +94,7 @@ module Storages
                     end
                   end
 
-                  Rails.logger.info "Response parsed found: #{result.inspect}"
+                  log_message "Response parsed found: #{result.inspect}"
                   ServiceResult.success(result:)
                 in { status: 405 }
                   log_response(response)
@@ -142,7 +113,11 @@ module Storages
             # rubocop:enable Metrics/AbcSize
 
             def log_response(response)
-              Rails.logger.info "Storage responded with a #{response.status} code."
+              log_message "Storage responded with a #{response.status} code."
+            end
+
+            def log_message(message)
+              Rails.logger.info message
             end
           end
         end

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

@@ -28,87 +28,113 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-module Storages::Peripherals::StorageInteraction::Nextcloud
-  class SetPermissionsCommand
-    using Storages::Peripherals::ServiceResultRefinements
+module Storages
+  module Peripherals
+    module StorageInteraction
+      module Nextcloud
+        class SetPermissionsCommand
+          using ServiceResultRefinements
 
-    def initialize(storage)
-      @storage = storage
-      @username = storage.username
-      @password = storage.password
-    end
+          SUCCESS_XPATH = "/d:multistatus/d:response/d:propstat[d:status[text() = 'HTTP/1.1 200 OK']]/d:prop/nc:acl-list"
+          def self.call(storage:, path:, permissions:)
+            new(storage).call(path:, permissions:)
+          end
 
-    def self.call(storage:, path:, permissions:)
-      new(storage).call(path:, permissions:)
-    end
+          def initialize(storage)
+            @storage = storage
+            @username = storage.username
+            @password = storage.password
+          end
 
-    # rubocop:disable Metrics/AbcSize
-    # rubocop:disable Metrics/PerceivedComplexity
-    def call(path:, permissions:)
-      raise ArgumentError if path.blank?
+          def call(path:, permissions:)
+            if path.blank?
+              return ServiceResult.failure(errors: StorageError.new(code: :invalid_path))
+            end
 
-      users_permissions = permissions.fetch(:users)
-      groups_permissions = permissions.fetch(:groups)
+            with_tagged_logger do
+              info "Setting permissions #{permissions.inspect} on #{path}"
 
-      body = Nokogiri::XML::Builder.new do |xml|
-        xml["d"].propertyupdate(
-          "xmlns:d" => "DAV:",
-          "xmlns:nc" => "http://nextcloud.org/ns"
-        ) do
-          xml["d"].set do
-            xml["d"].prop do
-              xml["nc"].send(:"acl-list") do
-                groups_permissions.each do |group, group_permissions|
-                  xml["nc"].acl do
-                    xml["nc"].send(:"acl-mapping-type", "group")
-                    xml["nc"].send(:"acl-mapping-id", group)
-                    xml["nc"].send(:"acl-mask", "31")
-                    xml["nc"].send(:"acl-permissions", group_permissions.to_s)
-                  end
-                end
-                users_permissions.each do |user, user_permissions|
-                  xml["nc"].acl do
-                    xml["nc"].send(:"acl-mapping-type", "user")
-                    xml["nc"].send(:"acl-mapping-id", user)
-                    xml["nc"].send(:"acl-mask", "31")
-                    xml["nc"].send(:"acl-permissions", user_permissions.to_s)
+              body = request_xml_body(permissions[:groups], permissions[:users])
+              # This can raise KeyErrors, we probably should just default to enpty Arrays.
+              response = OpenProject
+                .httpx
+                .basic_auth(@username, @password)
+                .request(
+                  "PROPPATCH",
+                  UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path),
+                  xml: body
+                )
+
+              handle_response(response)
+            end
+          end
+
+          private
+
+          # rubocop:disable Metrics/AbcSize
+          def handle_response(response)
+            error_data = StorageErrorData.new(source: self.class, payload: response)
+
+            case response
+            in { status: 200..299 }
+              doc = Nokogiri::XML(response.body.to_s)
+              if doc.xpath(SUCCESS_XPATH).present?
+                info "Permissions set"
+                ServiceResult.success(result: :success)
+              else
+                Util.error(:permission_not_set, "nc:acl properly has not been set for #{path}", error_data)
+              end
+            in { status: 404 }
+              Util.error(:not_found, "Outbound request destination not found", error_data)
+            in { status: 401 }
+              Util.error(:unauthorized, "Outbound request not authorized", error_data)
+            else
+              Util.error(:error, "Outbound request failed", error_data)
+            end
+          end
+
+          def request_xml_body(groups_permissions, users_permissions)
+            Nokogiri::XML::Builder.new do |xml|
+              xml["d"].propertyupdate(
+                "xmlns:d" => "DAV:",
+                "xmlns:nc" => "http://nextcloud.org/ns"
+              ) do
+                xml["d"].set do
+                  xml["d"].prop do
+                    xml["nc"].send(:"acl-list") do
+                      groups_permissions.each do |group, group_permissions|
+                        xml["nc"].acl do
+                          xml["nc"].send(:"acl-mapping-type", "group")
+                          xml["nc"].send(:"acl-mapping-id", group)
+                          xml["nc"].send(:"acl-mask", "31")
+                          xml["nc"].send(:"acl-permissions", group_permissions.to_s)
+                        end
+                      end
+                      users_permissions.each do |user, user_permissions|
+                        xml["nc"].acl do
+                          xml["nc"].send(:"acl-mapping-type", "user")
+                          xml["nc"].send(:"acl-mapping-id", user)
+                          xml["nc"].send(:"acl-mask", "31")
+                          xml["nc"].send(:"acl-permissions", user_permissions.to_s)
+                        end
+                      end
+                    end
                   end
                 end
               end
-            end
+            end.to_xml
+          end
+          # rubocop:enable Metrics/AbcSize
+
+          def with_tagged_logger(&)
+            Rails.logger.tagged(self.class, &)
+          end
+
+          def info(message)
+            Rails.logger.info message
           end
         end
-      end.to_xml
-
-      response = OpenProject
-                   .httpx
-                   .basic_auth(@username, @password)
-                   .request(
-                     "PROPPATCH",
-                     Storages::UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path),
-                     xml: body
-                   )
-
-      error_data = Storages::StorageErrorData.new(source: self.class, payload: response)
-
-      case response
-      in { status: 200..299 }
-        doc = Nokogiri::XML(response.body.to_s)
-        if doc.xpath("/d:multistatus/d:response/d:propstat[d:status[text() = 'HTTP/1.1 200 OK']]/d:prop/nc:acl-list").present?
-          ServiceResult.success(result: :success)
-        else
-          Util.error(:error, "nc:acl properly has not been set for #{path}", error_data)
-        end
-      in { status: 404 }
-        Util.error(:not_found, "Outbound request destination not found", error_data)
-      in { status: 401 }
-        Util.error(:unauthorized, "Outbound request not authorized", error_data)
-      else
-        Util.error(:error, "Outbound request failed", error_data)
       end
     end
-
-    # rubocop:enable Metrics/AbcSize
-    # rubocop:enable Metrics/PerceivedComplexity
   end
 end

modules/storages/app/models/storages/project_storage.rb

@@ -65,6 +65,10 @@ module Storages
       managed_folder_identifier.path
     end
 
+    def managed_project_folder_name
+      managed_folder_identifier.name
+    end
+
     def project_folder_location
       managed_folder_identifier.location
     end

modules/storages/app/models/storages/storage.rb

@@ -54,7 +54,6 @@ module Storages
 
     store_attribute :provider_fields, :automatically_managed, :boolean
     store_attribute :provider_fields, :health_notifications_enabled, :boolean, default: true
-    store_attribute :provider_fields, :detailed_logging_enabled, :boolean, default: false
 
     has_many :file_links, class_name: "Storages::FileLink"
     belongs_to :creator, class_name: "User"

modules/storages/app/models/storages/storage_error.rb

@@ -40,10 +40,9 @@ module Storages
       @data = data
     end
 
-    def to_active_model_errors(klass = self)
-      errors = ActiveModel::Errors.new(klass)
-      attr = data.source.to_s.demodulize.underscore.to_sym
-      errors.add(attr, code)
+    def to_active_model_errors
+      errors = ActiveModel::Errors.new(self)
+      errors.add(:storage_error, code, message: log_message)
       errors
     end
 

modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb

@@ -30,13 +30,12 @@
 
 module Storages
   class NextcloudGroupFolderPropertiesSyncService
-    using Peripherals::ServiceResultRefinements
-
     extend ActiveModel::Naming
     extend ActiveModel::Translation
 
-    PERMISSIONS_MAP = { read_files: 1, write_files: 2, create_files: 4, delete_files: 8, share_files: 16 }.freeze
+    using Peripherals::ServiceResultRefinements
 
+    PERMISSIONS_MAP = { read_files: 1, write_files: 2, create_files: 4, delete_files: 8, share_files: 16 }.freeze
     PERMISSIONS_KEYS = PERMISSIONS_MAP.keys.freeze
     ALL_PERMISSIONS = PERMISSIONS_MAP.values.sum
     NO_PERMISSIONS = 0
@@ -61,8 +60,8 @@ module Storages
 
     def call
       with_logging do
-        log_message "Starting AMPF Sync for Nextcloud Storage #{@storage.id}"
-        prepare_remote_folders.on_failure { return _1 }
+        info "Starting AMPF Sync for Nextcloud Storage #{@storage.id}"
+        prepare_remote_folders.on_failure { return @result }
         apply_permissions_to_folders
       end
     end
@@ -74,26 +73,24 @@ module Storages
     # @param options [Hash<Symbol, Object>] optional extra parameters for the message generation
     # @return [ServiceResult]
     def add_error(attribute, storage_error, options: {})
-      if storage_error == :error
-        @result.errors.add(:base, storage_error, **options)
+      case storage_error
+      when :error, :unauthorized
+        @result.errors.add(:base, storage_error.code, **options)
       else
-        @result.errors.add(attribute, storage_error, **options)
+        @result.errors.add(attribute, storage_error.code, **options)
       end
+
       @result
     end
 
     # @return [ServiceResult]
     def prepare_remote_folders
-      remote_folders = remote_root_folder_properties.result_or do |error|
-        format_and_log_error(error, { folder: @storage.group_folder })
-        return add_error(:remote_folder_properties, error.code, options: { group: @storage.group }).fail!
-      end
+      info "Preparing the remote group folder #{@storage.group_folder}"
 
-      ensure_root_folder_permissions.result_or do |error|
-        format_and_log_error(error, { folder: @storage.group_folder })
-        return add_error(:ensure_root_folder_permissions, error.code,
-                         options: { group: @storage.group, username: @storage.username }).fail!
-      end
+      remote_folders = remote_root_folder_map(@storage.group_folder).on_failure { return _1 }.result
+      info "Found #{remote_folders.count} remote folders"
+
+      ensure_root_folder_permissions(@storage.group_folder, @storage.group, @storage.username).on_failure { return _1 }
 
       ensure_folders_exist(remote_folders).on_success { hide_inactive_folders(remote_folders) }
     end
@@ -137,6 +134,7 @@ module Storages
       end
     end
 
+    # rubocop:disable Metrics/AbcSize
     def set_folders_permissions(remote_admins, project_storage)
       admin_permissions = remote_admins.to_set.map do |username|
         [username, ALL_PERMISSIONS]
@@ -148,18 +146,22 @@ module Storages
         hash[token.origin_user_id] = PERMISSIONS_MAP.values_at(*(PERMISSIONS_KEYS & permissions)).sum
       end
 
+      folder = project_storage.managed_project_folder_path
+
       command_params = {
-        path: project_storage.managed_project_folder_path,
+        path: folder,
         permissions: {
           users: admin_permissions.to_h.merge(users_permissions),
           groups: { "#{@storage.group}": NO_PERMISSIONS }
         }
       }
 
-      set_permissions.call(storage: @storage, **command_params).result_or do |error|
-        format_and_log_error(error, folder: project_storage.managed_project_folder_path)
+      set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
+        format_and_log_error(service_result.errors, folder:)
+        add_error(:set_folder_permission, service_result.errors, options: { folder: })
       end
     end
+    # rubocop:enable Metrics/AbcSize
 
     def project_tokens(project_storage)
       project_tokens = client_tokens_scope.where.not(id: admin_client_tokens_scope).order(:id)
@@ -171,108 +173,118 @@ module Storages
       end
     end
 
+    # rubocop:disable Metrics/AbcSize
     def hide_inactive_folders(remote_folders)
-      log_message "Hiding folders related to inactive projects"
+      info "Hiding folders related to inactive projects"
       project_folder_ids = active_project_storages_scope.pluck(:project_folder_id).compact
+
       remote_folders.except("/#{@storage.group_folder}/").each do |(path, attrs)|
         next if project_folder_ids.include?(attrs["fileid"])
 
-        log_message "Hiding project folder #{path}"
-        command_params = {
-          path:,
-          permissions: {
-            users: { "#{@storage.username}": ALL_PERMISSIONS },
-            groups: { "#{@storage.group}": NO_PERMISSIONS }
-          }
-        }
+        info "Hiding folder #{path} as it does not belong to any active project"
+        command_params = { path:,
+                           permissions: {
+                             users: { "#{@storage.username}": ALL_PERMISSIONS },
+                             groups: { "#{@storage.group}": NO_PERMISSIONS }
+                           } }
 
         set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
           format_and_log_error(service_result.errors, folder: path, context: "hide_folder")
-          add_error(:hide_inactive_folders, service_result.errors, options: { folder: path })
+          add_error(:hide_inactive_folders, service_result.errors, options: { path: })
         end
       end
     end
+    # rubocop:enable Metrics/AbcSize
 
     def ensure_folders_exist(remote_folders)
-      log_message "Ensuring project folders exist and are correctly named."
+      info "Ensuring that automatically managed project folders exist and are correctly named."
       id_folder_map = remote_folders.to_h { |folder, properties| [properties["fileid"], folder] }
 
       active_project_storages_scope.includes(:project).map do |project_storage|
         unless id_folder_map.key?(project_storage.project_folder_id)
-          log_message "#{project_storage.managed_project_folder_path} does not exist. Creating..."
-          next create_folder_stuff(project_storage)
+          info "#{project_storage.managed_project_folder_path} does not exist. Creating..."
+          next create_remote_folder(project_storage)
         end
 
-        current_path = id_folder_map[project_storage.project_folder_id]
-        if current_path != project_storage.managed_project_folder_path
-          log_message "#{current_path} is misnamed. Renaming to #{project_storage.managed_project_folder_path}"
-          target_folder_name = name_from_path(project_storage.managed_project_folder_path)
-          rename_folder(project_storage.project_folder_id, target_folder_name).on_failure do |service_result|
-            format_and_log_error(service_result.errors,
-                                 folder_id: project_storage.project_folder_id,
-                                 folder_name: target_folder_name)
-
-            return add_error(:rename_folder, service_result.errors).fail!
-          end
-        end
+        rename_folder(project_storage, id_folder_map[project_storage.project_folder_id])&.on_failure { return _1 }
       end
 
       # We processed every folder successfully
       ServiceResult.success
     end
 
-    def name_from_path(path)
-      path.split("/").last
+    # @param project_storage [Storages::ProjectStorage] Storages::ProjectStorage that the remote folder might need renaming
+    # @param current_path [String] current name of the remote project storage folder
+    # @return [ServiceResult, nil]
+    def rename_folder(project_storage, current_path)
+      return if current_path == project_storage.managed_project_folder_path
+
+      name = project_storage.managed_project_folder_name
+      file_id = project_storage.project_folder_id
+
+      info "#{current_path} is misnamed. Renaming to #{name}"
+      rename_file.call(storage: @storage, auth_strategy:, file_id:, name:).on_failure do |service_result|
+        format_and_log_error(service_result.errors, folder_id: file_id, folder_name: name)
+
+        add_error(:rename_project_folder, service_result.errors,
+                  options: { project_folder_name: name, project_folder_id: file_id }).fail!
+      end
     end
 
-    def rename_folder(folder_id, folder_name)
-      rename_file.call(storage: @storage, auth_strategy:, file_id: folder_id, name: folder_name)
-    end
-
-    def create_folder_stuff(project_storage)
+    def create_remote_folder(project_storage)
       folder_name = project_storage.managed_project_folder_path
       parent_location = Peripherals::ParentFolder.new("/")
 
       created_folder = create_folder.call(storage: @storage, auth_strategy:, folder_name:, parent_location:)
-                                            .result_or do |error|
-        format_and_log_error(error, folder_name:)
+                                            .on_failure do |service_result|
+        format_and_log_error(service_result.errors, folder_name:)
 
-        return add_error(:create_folder, error, options: { folder_name:, parent_location: })
-      end
+        return add_error(:create_folder, service_result.errors, options: { folder_name:, parent_location: })
+      end.result
 
-      project_folder_id = created_folder.id
-      last_project_folder = LastProjectFolder
-                              .find_by(project_storage_id: project_storage.id, mode: project_storage.project_folder_mode)
+      last_project_folder = LastProjectFolder.find_by(
+        project_storage_id: project_storage.id, mode: project_storage.project_folder_mode
+      )
 
-      audit_last_project_folder(last_project_folder, project_folder_id)
-      project_storage.project_folder_id
+      audit_last_project_folder(last_project_folder, created_folder.id)
     end
 
     def audit_last_project_folder(last_project_folder, project_folder_id)
       ApplicationRecord.transaction do
-        last_project_folder.update!(origin_folder_id: project_folder_id)
-        project_storage.update!(project_folder_id:)
-        project_storage.project_folder_id
+        success = last_project_folder.update(origin_folder_id: project_folder_id) &&
+          last_project_folder.project_storage.update(project_folder_id:)
+
+        raise ActiveRecord::Rollback unless success
       end
     end
 
-    def ensure_root_folder_permissions
-      log_message "Setting base permissions for user #{@storage.username} on the #{@storage.group_folder} folder"
+    # @param group_folder [string] name of the Group Folder in Nextcloud.
+    # @param username [String] username for the integration user
+    # @param group [String] group that the user should be part of
+    # @return [ServiceResult]
+    def ensure_root_folder_permissions(group_folder, username, group)
+      info "Setting needed permissions for user #{username} and group #{group} on #{group_folder} group folder"
+
       command_params = {
-        path: @storage.group_folder,
+        path: group_folder,
         permissions: {
-          users: { @storage.username.to_sym => ALL_PERMISSIONS },
-          groups: { @storage.group.to_sym => PERMISSIONS_MAP[:read_files] }
+          users: { username.to_sym => ALL_PERMISSIONS },
+          groups: { group.to_sym => PERMISSIONS_MAP[:read_files] }
         }
       }
 
-      set_permissions.call(storage: @storage, **command_params)
+      set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
+        format_and_log_error(service_result.errors, { folder: group_folder })
+        add_error(:ensure_root_folder_permissions, service_result.errors, options: { group:, username: }).fail!
+      end
     end
 
-    ### Base Queries/Commands
-    def remote_root_folder_properties
-      log_message "Retrieving already existing folders under #{@storage.group_folder}"
-      file_ids.call(storage: @storage, path: @storage.group_folder)
+    def remote_root_folder_map(group_folder)
+      info "Retrieving already existing folders under #{group_folder}"
+      file_ids.call(storage: @storage, path: group_folder).on_failure do |service_result|
+        format_and_log_error(service_result.errors, { folder: group_folder })
+        add_error(:remote_folders, service_result.errors, options: { group_folder:, username: @storage.username }).fail!
+      end
     end
 
     def remote_group_users
@@ -313,8 +325,8 @@ module Storages
       logger.error error_message
     end
 
-    def log_message(message)
-      logger.debug(message)
+    def info(message)
+      logger.info(message)
     end
 
     def with_logging(&)

modules/storages/app/workers/storages/automatically_managed_storage_sync_job.rb

@@ -66,7 +66,7 @@ module Storages
                       raise "Unknown Storage Type"
                     end
 
-      sync_result.on_failure { raise Errors::IntegrationJobError, sync_result.errors.to_s }
+      sync_result.on_failure { raise Errors::IntegrationJobError, sync_result.errors.full_messages.join(", ") }
       sync_result.on_success { OpenProject::Notifications.send(OpenProject::Events::STORAGE_TURNED_HEALTHY, storage:) }
     end
   end

modules/storages/config/locales/en.yml

@@ -1,14 +1,22 @@
 ---
 en:
   services:
+    attributes:
+      storages/nextcloud_group_folder_properties_sync_service:
+        remote_folders: 'Reading contents of the group folder:'
+        ensure_root_folder_permissions: 'Setting Basic Permissions:'
+        create_folder: 'Managed Project Folder Creation:'
+        rename_project_folder: 'Renaming managed project Folder:'
+        hide_inactive_folders: 'Hide Inactive Folders Step:'
     errors:
       models:
         storages/nextcloud_group_folder_properties_sync_service:
-          unauthorized: "Please check if your AMPF password is correct"
+          unauthorized: "OpenProject could not sync with Nextcloud. Please check you storage and Nextcloud configuration"
           error: "An unexpected error occurred. Please ensure that you Nextcloud instance is reachable and check OpenProject worker logs for more information"
           attributes:
-            remote_folder_properties:
-              not_found: "The Group Folder %{group} wasn't found. Please check your Nextcloud setup."
+            remote_folders:
+              not_found: "%{group_folder} wasn't found. Please check your Nextcloud setup."
+              not_allowed: "The %{username} doesn't have access to the %{group_folder}. Please check the folder permissions on Nextcloud"
 
   activerecord:
     attributes:
@@ -24,6 +32,7 @@ en:
     errors:
       messages:
         not_linked_to_project: is not linked to project.
+        invalid_url: is not a valid URL.
       models:
         storages/file_link:
           attributes:

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

@@ -372,15 +372,17 @@ RSpec.describe Storages::Peripherals::Registry, :webmock do
 
       context "when forbidden values are given as folder" do
         it "raises an ArgumentError on nil" do
-          expect do
-            registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: nil, permissions:)
-          end.to raise_error(ArgumentError)
+          result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: nil, permissions:)
+
+          expect(result).to be_failure
+          expect(result.errors.code).to eq(:invalid_path)
         end
 
-        it "raises an ArgumentError on empty string" do
-          expect do
-            registry.resolve("nextcloud.commands.set_permissions").call(path: "", permissions:)
-          end.to raise_error(ArgumentError)
+        it "returns a :invalid_path Failure on empty string" do
+          result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: "", permissions:)
+
+          expect(result).to be_failure
+          expect(result.errors.code).to eq(:invalid_path)
         end
       end
     end

modules/storages/spec/services/storages/nextcloud_group_folder_properties_sync_service_spec.rb

@@ -701,8 +701,9 @@ RSpec.describe Storages::NextcloudGroupFolderPropertiesSyncService, :webmock do
             result = described_class.new(storage).call
 
             expect(result).to be_failure
-            expect(result.errors[:remote_folder_properties])
-              .to contain_exactly(I18n.t("#{prefix}.attributes.remote_folder_properties.not_found", group: storage.group))
+            expect(result.errors[:remote_folders])
+              .to contain_exactly(I18n.t("#{prefix}.attributes.remote_folders.not_found",
+                                         group_folder: storage.group_folder))
           end
         end
 

modules/storages/spec/workers/storages/automatically_managed_storage_sync_job_spec.rb

@@ -91,10 +91,13 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do
       allow(Storages::HealthStatusMailerJob).to receive(:set).and_return(job)
       allow(job).to receive(:perform_later)
 
+      errors = ActiveModel::Errors.new(Storages::NextcloudGroupFolderPropertiesSyncService.new(managed_nextcloud))
+      errors.add(:group_folder, :not_found, group_folder: managed_nextcloud.group_folder)
+
       allow(Storages::NextcloudGroupFolderPropertiesSyncService)
         .to receive(:call)
               .with(managed_nextcloud)
-              .and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :not_found)))
+              .and_return(ServiceResult.failure(errors:))
 
       Timecop.freeze("2023-03-14T15:17:00Z") do
         expect do
@@ -103,16 +106,19 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do
         end.to(
           change(managed_nextcloud, :health_changed_at).to(Time.now.utc)
                                               .and(change(managed_nextcloud, :health_status).from("pending").to("unhealthy"))
-                                              .and(change(managed_nextcloud, :health_reason).from(nil).to("not_found"))
+                                              .and(change(managed_nextcloud, :health_reason).from(nil).to(/wasn't found/))
         )
       end
     end
 
     context "when Storages::Errors::IntegrationJobError is raised" do
       before do
+        errors = ActiveModel::Errors.new(Storages::NextcloudGroupFolderPropertiesSyncService.new(managed_nextcloud))
+        errors.add(:base, :error)
+
         allow(Storages::NextcloudGroupFolderPropertiesSyncService)
           .to receive(:call).with(managed_nextcloud)
-                            .and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :custom_error)))
+                            .and_return(ServiceResult.failure(errors:))
 
         allow(OpenProject::Notifications).to receive(:send)
       end
@@ -130,7 +136,7 @@ RSpec.describe Storages::AutomaticallyManagedStorageSyncJob, type: :job do
         expect(OpenProject::Notifications).to have_received(:send).with(
           OpenProject::Events::STORAGE_TURNED_UNHEALTHY,
           storage: managed_nextcloud,
-          reason: "custom_error"
+          reason: /unexpected error occurred/
         )
       end
     end