Skip to content

Commit

Permalink
fix(files): do not rely on unique fileid
Browse files Browse the repository at this point in the history
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
  • Loading branch information
skjnldsv committed Jun 12, 2024
1 parent 6ec6c1f commit 1d7893d
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 80 deletions.
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
24 changes: 13 additions & 11 deletions apps/files/src/store/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { Folder, Node } from '@nextcloud/files'
import type { FilesStore, RootsStore, RootOptions, Service, FilesState, FileId } from '../types'
import type { FilesStore, RootsStore, RootOptions, Service, FilesState, FileSource } from '../types'

import { defineStore } from 'pinia'
import { subscribe } from '@nextcloud/event-bus'
Expand All @@ -19,19 +19,20 @@ export const useFilesStore = function(...args) {

getters: {
/**
* Get a file or folder by id
* Get a file or folder by its source
*/
getNode: (state) => (id: FileId): Node|undefined => state.files[id],
getNode: (state) => (source: FileSource): Node|undefined => state.files[source],

/**
* Get a list of files or folders by their IDs
* Does not return undefined values
* Note: does not return undefined values
*/
getNodes: (state) => (ids: FileId[]): Node[] => ids
.map(id => state.files[id])
getNodes: (state) => (sources: FileSource[]): Node[] => sources
.map(source => state.files[source])
.filter(Boolean),

/**
* Get a file or folder by id
* Get the root folder of a service
*/
getRoot: (state) => (service: Service): Folder|undefined => state.roots[service],
},
Expand All @@ -41,10 +42,11 @@ export const useFilesStore = function(...args) {
// Update the store all at once
const files = nodes.reduce((acc, node) => {
if (!node.fileid) {
logger.error('Trying to update/set a node without fileid', node)
logger.error('Trying to update/set a node without fileid', { node })
return acc
}
acc[node.fileid] = node

acc[node.source] = node
return acc
}, {} as FilesStore)

Expand All @@ -53,8 +55,8 @@ export const useFilesStore = function(...args) {

deleteNodes(nodes: Node[]) {
nodes.forEach(node => {
if (node.fileid) {
Vue.delete(this.files, node.fileid)
if (node.source) {
Vue.delete(this.files, node.source)
}
})
},
Expand Down
Loading

0 comments on commit 1d7893d

Please sign in to comment.