Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle errors when Copying Project Folders when the Storage is broken #15893

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

mereghost
Copy link
Contributor

@mereghost mereghost commented Jun 19, 2024

Related WP: OP#55805

What?

I hadn't handled the issue of trying to copy a project folder on a broken storage. This PR aims to fix that.

How?

Well... by handling the service result failure. =/

This PR also:

  1. Adds the auth_strategy to CopyTemplateFolder
  2. Standardizes the error responses for CopyTemplateFolder

@mereghost mereghost self-assigned this Jun 19, 2024
Updates Nextcloud to use the same interface
@mereghost mereghost force-pushed the bug/no-method-for-symbol branch from 3226cba to a767385 Compare June 19, 2024 09:09
@mereghost mereghost marked this pull request as ready for review June 19, 2024 09:10
@mereghost mereghost requested a review from a team June 19, 2024 09:27
Copy link
Member

@Kharonus Kharonus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the auth rework of those commands. Just a question left.

@@ -33,28 +33,27 @@ module Nextcloud
class CopyTemplateFolderCommand
using ServiceResultRefinements

def self.call(storage:, source_path:, destination_path:)
new(storage).call(source_path:, destination_path:)
def self.call(auth_strategy:, storage:, source_path:, destination_path:)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 good thing I saw that now, was going to start with that command almost myself.

@@ -42,6 +42,7 @@ def perform(source:, target:, work_packages_map:)
user = batch.properties[:user]

project_folder_result = results_from_polling || initiate_copy(target)
project_folder_result.on_failure { |failed| return log_failure(failed) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ so, this is the actual fix, the rest is boyscouting, right? Or did I miss another piece of changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is the entire fix, the rest was just a case of "Ok, since I'm here already..."

@mereghost mereghost merged commit 4b94ff0 into dev Jun 21, 2024
8 checks passed
@mereghost mereghost deleted the bug/no-method-for-symbol branch June 21, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants