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

fix(files): do not rely on unique fileid #45251

Merged
merged 3 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 13 additions & 11 deletions apps/files/src/components/BreadCrumbs.vue
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { useSelectionStore } from '../store/selection.ts'
import { useUploaderStore } from '../store/uploader.ts'
import filesListWidthMixin from '../mixins/filesListWidth.ts'
import logger from '../logger'
import type { FileSource } from '../types.ts'

export default defineComponent({
name: 'BreadCrumbs',
Expand Down Expand Up @@ -106,8 +107,9 @@ export default defineComponent({

sections() {
return this.dirs.map((dir: string, index: number) => {
const fileid = this.getFileIdFromPath(dir)
const to = { ...this.$route, params: { fileid }, query: { dir } }
const source = this.getFileSourceFromPath(dir)
const node: Node | undefined = source ? this.getNodeFromSource(source) : undefined
const to = { ...this.$route, params: { node: node?.fileid }, query: { dir } }
return {
dir,
exact: true,
Expand Down Expand Up @@ -136,28 +138,28 @@ export default defineComponent({
},

selectedFiles() {
return this.selectionStore.selected
return this.selectionStore.selected as FileSource[]
},

draggingFiles() {
return this.draggingStore.dragging
return this.draggingStore.dragging as FileSource[]
},
},

methods: {
getNodeFromId(id: number): Node | undefined {
return this.filesStore.getNode(id)
getNodeFromSource(source: FileSource): Node | undefined {
return this.filesStore.getNode(source)
},
getFileIdFromPath(path: string): number | undefined {
getFileSourceFromPath(path: string): FileSource | undefined {
return this.pathsStore.getPath(this.currentView?.id, path)
},
getDirDisplayName(path: string): string {
if (path === '/') {
return this.$navigation?.active?.name || t('files', 'Home')
}

const fileId: number | undefined = this.getFileIdFromPath(path)
const node: Node | undefined = (fileId) ? this.getNodeFromId(fileId) : undefined
const source: FileSource | undefined = this.getFileSourceFromPath(path)
const node: Node | undefined = source ? this.getNodeFromSource(source) : undefined
return node?.attributes?.displayName || basename(path)
},

Expand Down Expand Up @@ -227,12 +229,12 @@ export default defineComponent({
}

// Else we're moving/copying files
const nodes = selection.map(fileid => this.filesStore.getNode(fileid)) as Node[]
const nodes = selection.map(source => this.filesStore.getNode(source)) as Node[]
await onDropInternalFiles(nodes, folder, contents.contents, isCopy)

// Reset selection after we dropped the files
// if the dropped files are within the selection
if (selection.some(fileid => this.selectedFiles.includes(fileid))) {
if (selection.some(source => this.selectedFiles.includes(source))) {
logger.debug('Dropped selection, resetting select store...')
this.selectionStore.reset()
}
Expand Down
17 changes: 9 additions & 8 deletions apps/files/src/components/FileEntry/FileEntryCheckbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js'
import { useKeyboardStore } from '../../store/keyboard.ts'
import { useSelectionStore } from '../../store/selection.ts'
import logger from '../../logger.js'
import type { FileSource } from '../../types.ts'

export default defineComponent({
name: 'FileEntryCheckbox',
Expand Down Expand Up @@ -66,10 +67,10 @@ export default defineComponent({
return this.selectionStore.selected
},
isSelected() {
return this.selectedFiles.includes(this.fileid)
return this.selectedFiles.includes(this.source.source)
},
index() {
return this.nodes.findIndex((node: Node) => node.fileid === this.fileid)
return this.nodes.findIndex((node: Node) => node.source === this.source.source)
},
isFile() {
return this.source.type === FileType.File
Expand All @@ -88,20 +89,20 @@ export default defineComponent({

// Get the last selected and select all files in between
if (this.keyboardStore?.shiftKey && lastSelectedIndex !== null) {
const isAlreadySelected = this.selectedFiles.includes(this.fileid)
const isAlreadySelected = this.selectedFiles.includes(this.source.source)

const start = Math.min(newSelectedIndex, lastSelectedIndex)
const end = Math.max(lastSelectedIndex, newSelectedIndex)

const lastSelection = this.selectionStore.lastSelection
const filesToSelect = this.nodes
.map(file => file.fileid)
.map(file => file.source)
.slice(start, end + 1)
.filter(Boolean) as number[]
.filter(Boolean) as FileSource[]

// If already selected, update the new selection _without_ the current file
const selection = [...lastSelection, ...filesToSelect]
.filter(fileid => !isAlreadySelected || fileid !== this.fileid)
.filter(source => !isAlreadySelected || source !== this.source.source)

logger.debug('Shift key pressed, selecting all files in between', { start, end, filesToSelect, isAlreadySelected })
// Keep previous lastSelectedIndex to be use for further shift selections
Expand All @@ -110,8 +111,8 @@ export default defineComponent({
}

const selection = selected
? [...this.selectedFiles, this.fileid]
: this.selectedFiles.filter(fileid => fileid !== this.fileid)
? [...this.selectedFiles, this.source.source]
: this.selectedFiles.filter(source => source !== this.source.source)

logger.debug('Updating selection', { selection })
this.selectionStore.set(selection)
Expand Down
21 changes: 11 additions & 10 deletions apps/files/src/components/FileEntryMixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import type { ComponentPublicInstance, PropType } from 'vue'
import type { FileSource } from '../types.ts'

import { showError } from '@nextcloud/dialogs'
import { FileType, Permission, Folder, File as NcFile, NodeStatus, Node, View } from '@nextcloud/files'
Expand Down Expand Up @@ -85,13 +86,13 @@ export default defineComponent({
},

draggingFiles() {
return this.draggingStore.dragging
return this.draggingStore.dragging as FileSource[]
},
selectedFiles() {
return this.selectionStore.selected
return this.selectionStore.selected as FileSource[]
},
isSelected() {
return this.fileid && this.selectedFiles.includes(this.fileid)
return this.selectedFiles.includes(this.source.source)
},

isRenaming() {
Expand All @@ -116,7 +117,7 @@ export default defineComponent({

// If we're dragging a selection, we need to check all files
if (this.selectedFiles.length > 0) {
const nodes = this.selectedFiles.map(fileid => this.filesStore.getNode(fileid)) as Node[]
const nodes = this.selectedFiles.map(source => this.filesStore.getNode(source)) as Node[]
return nodes.every(canDrag)
}
return canDrag(this.source)
Expand All @@ -128,7 +129,7 @@ export default defineComponent({
}

// If the current folder is also being dragged, we can't drop it on itself
if (this.fileid && this.draggingFiles.includes(this.fileid)) {
if (this.draggingFiles.includes(this.source.source)) {
return false
}

Expand Down Expand Up @@ -269,14 +270,14 @@ export default defineComponent({

// Dragging set of files, if we're dragging a file
// that is already selected, we use the entire selection
if (this.selectedFiles.includes(this.fileid)) {
if (this.selectedFiles.includes(this.source.source)) {
this.draggingStore.set(this.selectedFiles)
} else {
this.draggingStore.set([this.fileid])
this.draggingStore.set([this.source.source])
}

const nodes = this.draggingStore.dragging
.map(fileid => this.filesStore.getNode(fileid)) as Node[]
.map(source => this.filesStore.getNode(source)) as Node[]

const image = await getDragAndDropPreview(nodes)
event.dataTransfer?.setDragImage(image, -10, -10)
Expand Down Expand Up @@ -330,12 +331,12 @@ export default defineComponent({
}

// Else we're moving/copying files
const nodes = selection.map(fileid => this.filesStore.getNode(fileid)) as Node[]
const nodes = selection.map(source => this.filesStore.getNode(source)) as Node[]
await onDropInternalFiles(nodes, folder, contents.contents, isCopy)

// Reset selection after we dropped the files
// if the dropped files are within the selection
if (selection.some(fileid => this.selectedFiles.includes(fileid))) {
if (selection.some(source => this.selectedFiles.includes(source))) {
logger.debug('Dropped selection, resetting select store...')
this.selectionStore.reset()
}
Expand Down
3 changes: 2 additions & 1 deletion apps/files/src/components/FilesListTableHeader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import FilesListTableHeaderButton from './FilesListTableHeaderButton.vue'
import filesSortingMixin from '../mixins/filesSorting.ts'
import logger from '../logger.js'
import type { Node } from '@nextcloud/files'
import type { FileSource } from '../types.ts'

export default defineComponent({
name: 'FilesListTableHeader',
Expand Down Expand Up @@ -169,7 +170,7 @@ export default defineComponent({

onToggleAll(selected) {
if (selected) {
const selection = this.nodes.map(node => node.fileid).filter(Boolean) as number[]
const selection = this.nodes.map(node => node.source).filter(Boolean) as FileSource[]
logger.debug('Added all nodes to selection', { selection })
this.selectionStore.setLastIndex(null)
this.selectionStore.set(selection)
Expand Down
14 changes: 7 additions & 7 deletions apps/files/src/components/FilesListTableHeaderActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { useFilesStore } from '../store/files.ts'
import { useSelectionStore } from '../store/selection.ts'
import filesListWidthMixin from '../mixins/filesListWidth.ts'
import logger from '../logger.js'
import type { FileId } from '../types'
import type { FileSource } from '../types'

// The registered actions list
const actions = getFileActions()
Expand All @@ -64,7 +64,7 @@ export default defineComponent({
required: true,
},
selectedNodes: {
type: Array as PropType<FileId[]>,
type: Array as PropType<FileSource[]>,
default: () => ([]),
},
},
Expand Down Expand Up @@ -100,7 +100,7 @@ export default defineComponent({

nodes() {
return this.selectedNodes
.map(fileid => this.getNode(fileid))
.map(source => this.getNode(source))
.filter(Boolean) as Node[]
},

Expand Down Expand Up @@ -144,7 +144,7 @@ export default defineComponent({

async onActionClick(action) {
const displayName = action.displayName(this.nodes, this.currentView)
const selectionIds = this.selectedNodes
const selectionSources = this.selectedNodes
try {
// Set loading markers
this.loading = action.id
Expand All @@ -165,9 +165,9 @@ export default defineComponent({
// Handle potential failures
if (results.some(result => result === false)) {
// Remove the failed ids from the selection
const failedIds = selectionIds
.filter((fileid, index) => results[index] === false)
this.selectionStore.set(failedIds)
const failedSources = selectionSources
.filter((source, index) => results[index] === false)
this.selectionStore.set(failedSources)

if (results.some(result => result === null)) {
// If some actions returned null, we assume that the dev
Expand Down
4 changes: 2 additions & 2 deletions apps/files/src/store/dragging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
import { defineStore } from 'pinia'
import Vue from 'vue'
import type { FileId, DragAndDropStore } from '../types'
import type { DragAndDropStore, FileSource } from '../types'

export const useDragAndDropStore = defineStore('dragging', {
state: () => ({
Expand All @@ -15,7 +15,7 @@ export const useDragAndDropStore = defineStore('dragging', {
/**
* Set the selection of fileIds
*/
set(selection = [] as FileId[]) {
set(selection = [] as FileSource[]) {
Vue.set(this, 'dragging', selection)
},

Expand Down
Loading
Loading