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

[full-ci] Resolve upload existing folder conflict dialog #7504

Merged
merged 29 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6973a0c
It aint much but it kinda works
lookacat Aug 22, 2022
af74fba
Implement "keep both"
lookacat Aug 23, 2022
21143ab
Add changelog
lookacat Aug 23, 2022
f08b9f8
remove dev leftover
lookacat Aug 23, 2022
7b71a91
Fix folder name
lookacat Aug 23, 2022
133cc2b
Add isFolder
lookacat Aug 26, 2022
81c53f1
Make file conflict dialog work
lookacat Aug 31, 2022
9994412
Linting
lookacat Aug 31, 2022
1380d73
Fix folder keep both
lookacat Aug 31, 2022
21ace2c
Check for folder to already exist
lookacat Aug 31, 2022
9c43891
remove dev leftover
lookacat Aug 31, 2022
50b67f3
Address PR issues
lookacat Sep 2, 2022
20f72fb
Use store
lookacat Sep 5, 2022
9d98ed1
Provide existing files with function parameter
lookacat Sep 5, 2022
5f772f5
Add type to interface
lookacat Sep 5, 2022
f3649d5
Refactor resolve file & folder conflicts
lookacat Sep 6, 2022
fa29985
Refactor conflict dialog
lookacat Sep 6, 2022
d2408cd
Bugfix, remove dev leftover
lookacat Sep 6, 2022
1f217ca
Simplify conflict-array structure
JammingBen Sep 6, 2022
fade050
Fix folder upload
JammingBen Sep 6, 2022
dc7615d
Add merge to folders
lookacat Sep 6, 2022
c040bdd
Ignore existing folder errors for now
lookacat Sep 6, 2022
9179c7e
Make Merge reappear if "do for all" ticked
lookacat Sep 7, 2022
4511d5b
Address PR issues
lookacat Sep 7, 2022
afedbd7
Add unittests
lookacat Sep 7, 2022
9a0d3e2
Add more unittests
lookacat Sep 7, 2022
d79fe54
Fix e2e upload version
lookacat Sep 7, 2022
b4fe000
Fix file overwrite acceptance tests
lookacat Sep 8, 2022
4ac0d5a
Address PR issues
lookacat Sep 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-resolve-upload-existing-folder
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Resolve upload existing folder

We've added a conflict dialog which handles name clashes when uploading files and folders.

https://github.com/owncloud/web/pull/7504
https://github.com/owncloud/web/issues/6996
178 changes: 121 additions & 57 deletions packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,15 @@ import { UppyResource, useUpload } from 'web-runtime/src/composables/upload'
import { useUploadHelpers } from '../../composables/upload'
import { SHARE_JAIL_ID } from '../../services/folder'
import { bus } from 'web-pkg/src/instance'
import { buildWebDavSpacesPath } from 'web-client/src/helpers'
import { resolveFileNameDuplicate, extractNameWithoutExtension } from '../../helpers/resource'
import { buildWebDavSpacesPath, Resource } from 'web-client/src/helpers'
import { extractExtensionFromFile, extractNameWithoutExtension } from '../../helpers/resource'
import {
resolveFileExists,
ResolveStrategy,
ResolveConflict,
resolveFileNameDuplicate,
FileExistsResolver
} from '../../helpers/resource/copyMove'

export default defineComponent({
components: {
Expand Down Expand Up @@ -678,43 +685,40 @@ export default defineComponent({
return null
},

onFilesSelected(files: File[]) {
async onFilesSelected(files: File[]) {
const conflicts = []
const uppyResources: UppyResource[] = this.inputFilesToUppyFiles(files)
const quotaExceeded = this.checkQuotaExceeded(uppyResources)

if (quotaExceeded) {
return this.$uppyService.clearInputs()
}

for (const file of uppyResources) {
const relativeFilePath = file.meta.relativePath
if (relativeFilePath) {
// Logic for folders, applies to all files inside folder and subfolders
const rootFolder = relativeFilePath.replace(/^\/+/, '').split('/')[0]
const exists = this.files.find((f) => f.name === rootFolder)
if (exists) {
this.showMessage({
title: this.$gettextInterpolate(
this.$gettext('Folder "%{folder}" already exists.'),
{ folder: rootFolder },
true
),
status: 'danger'
if (conflicts.some((conflict) => conflict.name === rootFolder)) continue
conflicts.push({
name: rootFolder,
type: 'folder'
})
return
continue
}

// Folder does not exist, no need to care about files inside -> continue
continue
}
const exists = this.files.find((f) => f.name === file.name)
// Logic for files
const exists = this.files.find((f) => f.name === file.name && !file.meta.relativeFolder)
if (exists) {
conflicts.push(file)
conflicts.push({
name: file.name,
type: 'file'
})
}
}

if (conflicts.length) {
this.displayOverwriteDialog(uppyResources, conflicts)
await this.displayOverwriteDialog(uppyResources, conflicts, resolveFileExists)
} else {
this.handleUppyFileUpload(uppyResources)
}
Expand Down Expand Up @@ -797,51 +801,111 @@ export default defineComponent({
this.$uppyService.uploadFiles(files)
},

displayOverwriteDialog(files: UppyResource[], conflicts) {
const title = this.$ngettext(
'File already exists',
'Some files already exist',
conflicts.length
)

const isVersioningEnabled = this.isUserContext && this.capabilities?.files?.versioning
async displayOverwriteDialog(
files: UppyResource[],
conflicts,
resolveFileExistsMethod: FileExistsResolver
) {
let count = 0
const allConflictsCount = conflicts.length
const resolvedFileConflicts = []
const resolvedFolderConflicts = []
let doForAllConflicts = false
let allConflictsStrategy
let doForAllConflictsFolders = false
let allConflictsStrategyFolders

for (const conflict of conflicts) {
const isFolder = conflict.type === 'folder'
const conflictArray = isFolder ? resolvedFolderConflicts : resolvedFileConflicts

if (doForAllConflicts && !isFolder) {
conflictArray.push({
name: conflict.name,
strategy: allConflictsStrategy
})
continue
}
if (doForAllConflictsFolders && isFolder) {
conflictArray.push({
name: conflict.name,
strategy: allConflictsStrategyFolders
})
continue
}

let translatedMsg
if (isVersioningEnabled) {
translatedMsg = this.$ngettext(
'The following resource already exists: %{resources}. Do you want to create a new version for it?',
'The following resources already exist: %{resources}. Do you want to create a new version for them?',
conflicts.length
)
} else {
translatedMsg = this.$ngettext(
'The following resource already exists: %{resources}. Do you want to overwrite it?',
'The following resources already exist: %{resources}. Do you want to overwrite them?',
conflicts.length
const resolvedConflict: ResolveConflict = await resolveFileExistsMethod(
this.createModal,
this.hideModal,
{ name: conflict.name, isFolder } as Resource,
allConflictsCount - count,
this.$gettext,
this.$gettextInterpolate,
false,
isFolder
)
count++
if (resolvedConflict.doForAllConflicts) {
if (isFolder) {
doForAllConflictsFolders = true
allConflictsStrategyFolders = resolvedConflict.strategy
} else {
doForAllConflicts = true
allConflictsStrategy = resolvedConflict.strategy
}
}

conflictArray.push({
name: conflict.name,
strategy: resolvedConflict.strategy
})
}
const message = this.$gettextInterpolate(translatedMsg, {
resources: conflicts.map((f) => `${f.name}`).join(', ')
})
const filesToSkip = resolvedFileConflicts
.filter((e) => e.strategy === ResolveStrategy.SKIP)
.map((e) => e.name)
const foldersToSkip = resolvedFolderConflicts
.filter((e) => e.strategy === ResolveStrategy.SKIP)
.map((e) => e.name)

files = files.filter((e) => !filesToSkip.includes(e.name))
JammingBen marked this conversation as resolved.
Show resolved Hide resolved
lookacat marked this conversation as resolved.
Show resolved Hide resolved
files = files.filter(
(file) =>
!foldersToSkip.some((folderName) => file.meta.relativeFolder.split('/')[1] === folderName)
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
      )

)

const modal = {
variation: isVersioningEnabled ? 'passive' : 'danger',
icon: 'upload-cloud',
title,
message,
cancelText: this.$gettext('Cancel'),
confirmText: isVersioningEnabled ? this.$gettext('Create') : this.$gettext('Overwrite'),
onCancel: () => {
this.$uppyService.clearInputs()
this.hideModal()
},
onConfirm: () => {
this.hideModal()
this.handleUppyFileUpload(files)
const filesToKeepBoth = resolvedFileConflicts
.filter((e) => e.strategy === ResolveStrategy.KEEP_BOTH)
.map((e) => e.name)
const foldersToKeepBoth = resolvedFolderConflicts
.filter((e) => e.strategy === ResolveStrategy.KEEP_BOTH)
.map((e) => e.name)

for (const fileName of filesToKeepBoth) {
const file = files.find((e) => e.name === fileName && !e.meta.relativeFolder)
const extension = extractExtensionFromFile({ name: fileName } as Resource)
file.name = resolveFileNameDuplicate(fileName, extension, this.files)
}
for (const folder of foldersToKeepBoth) {
const filesInFolder = files.filter((e) => e.meta.relativeFolder.split('/')[1] === folder)
for (const file of filesInFolder) {
const newFolderName = resolveFileNameDuplicate(folder, '', this.files)
file.meta.relativeFolder = file.meta.relativeFolder.replace(
`/${folder}`,
`/${newFolderName}`
)
file.meta.relativePath = file.meta.relativePath.replace(
`/${folder}/`,
`/${newFolderName}/`
)
file.meta.tusEndpoint = file.meta.tusEndpoint.replace(`/${folder}`, `/${newFolderName}`)
const data = file.data as any
data.relativePath = data.relativePath.replace(`/${folder}/`, `/${newFolderName}/`)
file.meta.routeItem = `/${newFolderName}`
}
}

this.createModal(modal)
if (files.length === 0) return
this.handleUppyFileUpload(files)
}
}
})
Expand Down
27 changes: 21 additions & 6 deletions packages/web-app-files/src/helpers/resource/copyMove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import { join } from 'path'
import { buildResource } from '../resources'
import { DavProperties } from 'web-pkg/src/constants'

enum ResolveStrategy {
export enum ResolveStrategy {
SKIP,
REPLACE,
KEEP_BOTH
KEEP_BOTH,
MERGE
}
export interface ResolveConflict {
strategy: ResolveStrategy
Expand All @@ -18,14 +19,28 @@ interface FileConflict {
strategy?: ResolveStrategy
}

const resolveFileExists = (
export interface FileExistsResolver {
(
createModal: void,
hideModal: void,
resource: Resource,
conflictCount: number,
$gettext: void,
$gettextInterpolate: void,
isSingleConflict: boolean,
suggestMerge: boolean
)
Copy link
Contributor

Choose a reason for hiding this comment

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

The return type is missing 🤔

}

export const resolveFileExists = (
createModal,
hideModal,
resource,
conflictCount,
$gettext,
$gettextInterpolate,
isSingleConflict
isSingleConflict,
suggestMerge = false
): Promise<ResolveConflict> => {
return new Promise<ResolveConflict>((resolve) => {
let doForAllConflicts = false
Expand All @@ -42,7 +57,7 @@ const resolveFileExists = (
),
cancelText: $gettext('Skip'),
confirmText: $gettext('Keep both'),
buttonSecondaryText: $gettext('Replace'),
buttonSecondaryText: suggestMerge ? $gettext('Merge') : $gettext('Replace'),
checkboxLabel: isSingleConflict
? ''
: $gettextInterpolate(
Expand All @@ -59,7 +74,7 @@ const resolveFileExists = (
},
onConfirmSecondary: () => {
hideModal()
const strategy = ResolveStrategy.REPLACE
const strategy = suggestMerge ? ResolveStrategy.MERGE : ResolveStrategy.REPLACE
resolve({ strategy, doForAllConflicts } as ResolveConflict)
},
onConfirm: () => {
Expand Down
Loading