Skip to content

Commit

Permalink
Merge pull request #13235 from opf/implementation/49331-add-invocatio…
Browse files Browse the repository at this point in the history
…n-of-delete-folder-command-to-deletion-of-project-storage

[#49331] Added project folder deletion on project storage deletion
  • Loading branch information
Kharonus authored Jul 26, 2023
2 parents 4693c7f + f63c38e commit 498fe1a
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 6 deletions.
7 changes: 7 additions & 0 deletions modules/storages/app/models/storages/storage_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#++

class Storages::StorageError
extend ActiveModel::Naming

attr_reader :code, :log_message, :data

def initialize(code:, log_message: nil, data: nil)
Expand All @@ -47,4 +49,9 @@ def to_s
output << " | #{data}" unless data.nil?
output
end

def storage_error = "storage error"
def read_attribute_for_validation(attr) = send(attr)
def self.human_attribute_name(attr, _options = {}) = attr
def self.lookup_ancestors = [self]
end
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,31 @@ module Storages::ProjectStorages
# Performs the deletion in the superclass. Associated FileLinks are deleted
# by the model before_destroy hook.
class DeleteService < ::BaseServices::Delete
using Storages::Peripherals::ServiceResultRefinements

# rubocop:disable Metrics/AbcSize
def before_perform(params, service_result)
before_result = super(params, service_result)
return before_result if before_result.failure? || !model.storage.is_a?(Storages::NextcloudStorage)

Storages::Peripherals::StorageRequests
.new(storage: model.storage)
.delete_folder_command
.call(location: model.project_folder_path)
.match(
on_success: ->(*) { ServiceResult.success(result: model) },
on_failure: ->(error) do
if error.code == :not_found
ServiceResult.success(result: model)
else
ServiceResult.failure(errors: error.to_active_model_errors)
end
end
)
end

# rubocop:enable Metrics/AbcSize

# "persist" is a callback from BaseContracted.perform
# that is supposed to do the actual work in a contract.
# So in a DeleteService it performs the actual delete,
Expand Down
6 changes: 6 additions & 0 deletions modules/storages/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ en:
permission_share_files: "Share files"
project_module_storages: "File storages"

errors:
attributes:
storage_error:
not_authorized: "Not authorized for external connection to storage."
not_found: "The requested resource could not be found at the external file storage."

activerecord:
models:
file_link: "File link"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

# Test if the deletion of a ProjectStorage actually deletes related FileLink
# objects.
RSpec.describe 'Delete ProjectStorage with FileLinks', js: true do
RSpec.describe 'Delete ProjectStorage with FileLinks', js: true, webmock: true do
let(:user) { create(:user) }
let(:role) { create(:existing_role, permissions: [:manage_storages_in_project]) }
let(:project) do
Expand All @@ -40,13 +40,18 @@
members: { user => role },
enabled_module_names: %i[storages work_package_tracking])
end
let(:storage) { create(:storage, name: "Storage 1") }
let(:storage) { create(:nextcloud_storage, name: "Storage 1") }
let(:work_package) { create(:work_package, project:) }
let(:project_storage) { create(:project_storage, storage:, project:) }
let(:file_link) { create(:file_link, storage:, container: work_package) }
let(:second_file_link) { create(:file_link, container: work_package, storage:) }
let(:delete_folder_url) do
"#{storage.host}/remote.php/dav/files/#{storage.username}/#{project_storage.project_folder_path.chop}"
end

before do
stub_request(:delete, delete_folder_url).to_return(status: 204, body: nil, headers: {})

# The objects defined by let(...) above are lazy instantiated, so we need
# to "use" (just write their name) below to really create them.
project_storage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,21 @@
require 'services/base_services/behaves_like_delete_service'
require_relative 'shared_synchronization_trigger_examples'

RSpec.describe Storages::ProjectStorages::DeleteService, type: :model do
RSpec.describe Storages::ProjectStorages::DeleteService, type: :model, webmock: true do
context 'with records written to DB' do
let(:user) { create(:user) }
let(:role) { create(:existing_role, permissions: [:manage_storages_in_project]) }
let(:project) { create(:project, members: { user => role }) }
let(:other_project) { create(:project) }
let(:project_storage) { create(:project_storage, project:) }
let(:storage) { create(:storage) }
let(:project_storage) { create(:project_storage, project:, storage:) }
let(:work_package) { create(:work_package, project:) }
let(:other_work_package) { create(:work_package, project: other_project) }
let(:file_link) { create(:file_link, container: work_package, storage: project_storage.storage) }
let(:other_file_link) { create(:file_link, container: other_work_package, storage: project_storage.storage) }
let(:file_link) { create(:file_link, container: work_package, storage:) }
let(:other_file_link) { create(:file_link, container: other_work_package, storage:) }
let(:delete_folder_url) do
"#{storage.host}/remote.php/dav/files/#{storage.username}/#{project_storage.project_folder_path.chop}"
end

it 'destroys the record' do
project_storage
Expand All @@ -62,6 +66,39 @@
expect(Storages::FileLink.where(id: other_file_link.id))
.to exist
end

context 'with Nextcloud storage' do
let(:storage) { create(:nextcloud_storage) }

before do
stub_request(:delete, delete_folder_url).to_return(status: 204, body: nil, headers: {})
end

it 'tries to remove the project folder at the external nextcloud storage' do
expect(described_class.new(model: project_storage, user:).call).to be_success
end

context 'if project folder is not present' do
before do
stub_request(:delete, delete_folder_url).to_return(status: 404, body: nil, headers: {})
end

it 'tries to remove the project folder at the external nextcloud storage and still succeed with deletion' do
expect(described_class.new(model: project_storage, user:).call).to be_success
end
end

context 'if access is not authorized' do
before do
stub_request(:delete, delete_folder_url).to_return(status: 401, body: nil, headers: {})
end

it 'tries to remove the project folder at the external nextcloud storage, fails and does not delete project storage' do
expect(described_class.new(model: project_storage, user:).call).not_to be_success
expect(Storages::ProjectStorage.where(id: project_storage.id)).to exist
end
end
end
end

# Includes many specs that are common for every DeleteService that inherits from ::BaseServices::Delete.
Expand Down

0 comments on commit 498fe1a

Please sign in to comment.