[full-ci] Resolve upload existing folder conflict dialog

[lookacat]

Description

See #6996

Related Issue

• Fixes Upload of folder with same name shows error #6996

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[ownclouders]

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/28165/12/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/28165/tracing/public-link-for-space-alice-2022-9-8-08-30-32.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/28165/tracing/public-link-for-space-brian-2022-9-8-08-30-59.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/28165/tracing/public-link-for-space-marie-2022-9-8-08-31-03.zip

[kulmann]

The control flow in onFilesSelected is rather complex now and, if I understood correctly, still falls through to the old create version dialog. Can you simplify the control flow (e.g. early returns / continues where possible)? Also, the "create version" dialog should disappear entirely, as it's not needed anymore if we handle the replace use case in the different resolve strategies already. Please delete the code related to the create version dialog then.

[kulmann]

Sorry if this appears to be harsh, but I don't get which concept you're following with this PR. I had the impression that the ticket describes pretty clear what's needed, but let me summarize it again here:

1. No matter if you try to upload a single file, a single folder, multiple files, multiple folders or any mix of that: we only handle conflicts on the "current folder"-level. Meaning if you already have folder 1 in the current folder and try to upload a folder named folder 1, we show the conflict resolve dialog. We don't need to look into the folder and we only need to resolve the conflict once on the current folder level.
2. Files and folders are being handled the same (except for slightly different messages in the modal). I don't get why you have different handlers implemented for files and for folders. For folders you're invoking the conflict resolution directly, for files you only add them to the conflicts array and then ignore that array in the handler you're calling afterwards.
3. The technical process should be:
   a. Go through all uppyResources and check if they are conflicting with existing files. Resolve the conflict with a respective user choice. Don't do any backend requests here. Only memorize the user choices.
   b. Modify the uppyResources according to the user choices
   c. Push the upload queue into the uppy service

What's not clear to me is how we should handle the Overwrite case for folders, as the backend can only create a new version for files, not for folders. Sounds like a delete & upload-thing to me, which would then need to be reverted if an error happens during the upload. Ideally the backend supports an overwrite flag for folders, but not sure if that exists.

By the way, trying this locally, I immediately ran into situations where folders are attempted to be created and fail because they already exist.

[kulmann]

Added some comments regarding code style. Checked the branch locally, works like a charm. 🚀

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

45.0% Coverage
0.0% Duplication

[kulmann]

I have two recommendations (see comments) - but they can happen in a subsequent PR if you want to get this one merged (squash merge please!).

[kulmann]

The return type is missing 🤔

[kulmann]

Please include the condition in line 870. Doesn't make much sense iterating twice for filtering, when you can do all the filtering in one iteration.

Filter should look like this:

files = files.filter(
        (file) =>
          !filesToSkip.includes(file.name) &&
          !foldersToSkip.some((folderName) => file.meta.relativeFolder.split('/')[1] === folderName)
      )

outdated, review is approved