fix(files): properly update paths and folder children on node move

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

apps/files/src/store/files.ts

@@ -13,6 +13,7 @@ import Vue from 'vue'
 
 import { fetchNode } from '../services/WebdavClient.ts'
 import { usePathsStore } from './paths.ts'
+import { dirname } from '@nextcloud/paths'
 
 export const useFilesStore = function(...args) {
 	const store = defineStore('files', {
@@ -24,14 +25,12 @@ export const useFilesStore = function(...args) {
 		getters: {
 			/**
 			 * Get a file or folder by its source
-			 * @param state
 			 */
 			getNode: (state) => (source: FileSource): Node|undefined => state.files[source],
 
 			/**
 			 * Get a list of files or folders by their IDs
 			 * Note: does not return undefined values
-			 * @param state
 			 */
 			getNodes: (state) => (sources: FileSource[]): Node[] => sources
 				.map(source => state.files[source])
@@ -41,13 +40,11 @@ export const useFilesStore = function(...args) {
 			 * Get files or folders by their file ID
 			 * Multiple nodes can have the same file ID but different sources
 			 * (e.g. in a shared context)
-			 * @param state
 			 */
 			getNodesById: (state) => (fileId: number): Node[] => Object.values(state.files).filter(node => node.fileid === fileId),
 
 			/**
 			 * Get the root folder of a service
-			 * @param state
 			 */
 			getRoot: (state) => (service: Service): Folder|undefined => state.roots[service],
 		},
@@ -115,6 +112,17 @@ export const useFilesStore = function(...args) {
 				this.updateNodes([node])
 			},
 
+			onMovedNode({ node, oldSource }: { node: Node, oldSource: string }) {
+				if (!node.fileid) {
+					logger.error('Trying to update/set a node without fileid', { node })
+					return
+				}
+
+				// Update the path of the node
+				Vue.delete(this.files, oldSource)
+				this.updateNodes([node])
+			},
+
 			async onUpdatedNode(node: Node) {
 				if (!node.fileid) {
 					logger.error('Trying to update/set a node without fileid', { node })
@@ -147,6 +155,7 @@ export const useFilesStore = function(...args) {
 		subscribe('files:node:created', fileStore.onCreatedNode)
 		subscribe('files:node:deleted', fileStore.onDeletedNode)
 		subscribe('files:node:updated', fileStore.onUpdatedNode)
+		subscribe('files:node:moved', fileStore.onMovedNode)
 
 		fileStore._initialized = true
 	}

apps/files/src/store/paths.spec.ts

@@ -127,4 +127,40 @@ describe('Path store', () => {
 		// See the child is removed
 		expect(root._children).toEqual([])
 	})
+
+	test('Folder is moved', () => {
+		const node = new Folder({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/folder', id: 2 })
+		emit('files:node:created', node)
+		// see that the path is added and the children are set-up
+		expect(store.paths).toEqual({ files: { [node.path]: node.source } })
+		expect(root._children).toEqual([node.source])
+
+		const renamedNode = node.clone()
+		renamedNode.rename('new-folder')
+
+		expect(renamedNode.path).toBe('/new-folder')
+		expect(renamedNode.source).toBe('http://example.com/remote.php/dav/files/test/new-folder')
+
+		emit('files:node:moved', { node: renamedNode, oldSource: node.source })
+		// See the path is updated
+		expect(store.paths).toEqual({ files: { [renamedNode.path]: renamedNode.source } })
+		// See the child is updated
+		expect(root._children).toEqual([renamedNode.source])
+	})
+
+	test('File is moved', () => {
+		const node = new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', id: 2, mime: 'text/plain' })
+		emit('files:node:created', node)
+		// see that the children are set-up
+		expect(root._children).toEqual([node.source])
+		expect(store.paths).toEqual({})
+
+		const renamedNode = node.clone()
+		renamedNode.rename('new-file.txt')
+
+		emit('files:node:moved', { node: renamedNode, oldSource: node.source })
+		// See the child is updated
+		expect(root._children).toEqual([renamedNode.source])
+		expect(store.paths).toEqual({})
+	})
 })

apps/files/src/store/paths.ts

@@ -4,12 +4,14 @@
  */
 import type { FileSource, PathsStore, PathOptions, ServicesState, Service } from '../types'
 import { defineStore } from 'pinia'
-import { FileType, Folder, Node, getNavigation } from '@nextcloud/files'
+import { File, FileType, Folder, Node, getNavigation } from '@nextcloud/files'
 import { subscribe } from '@nextcloud/event-bus'
 import Vue from 'vue'
 import logger from '../logger'
 
 import { useFilesStore } from './files'
+import { basename, dirname } from '@nextcloud/paths'
+import { F } from 'lodash/fp'
 
 export const usePathsStore = function(...args) {
 	const files = useFilesStore(...args)
@@ -50,6 +52,27 @@ export const usePathsStore = function(...args) {
 				Vue.delete(this.paths[service], path)
 			},
 
+			onCreatedNode(node: Node) {
+				const service = getNavigation()?.active?.id || 'files'
+				if (!node.fileid) {
+					logger.error('Node has no fileid', { node })
+					return
+				}
+
+				// Only add path if it's a folder
+				if (node.type === FileType.Folder) {
+					this.addPath({
+						service,
+						path: node.path,
+						source: node.source,
+					})
+				}
+
+				// Update parent folder children if exists
+				// If the folder is the root, get it and update it
+				this.addNodeToParentChildren(node)
+			},
+
 			onDeletedNode(node: Node) {
 				const service = getNavigation()?.active?.id || 'files'
 
@@ -61,95 +84,80 @@ export const usePathsStore = function(...args) {
 					)
 				}
 
-				// Remove node from children
-				if (node.dirname === '/') {
-					const root = files.getRoot(service) as Folder & { _children?: string[] }
-					// ensure sources are unique
-					const children = new Set(root._children ?? [])
-					children.delete(node.source)
-					Vue.set(root, '_children', [...children.values()])
-					return
-				}
-
-				if (this.paths[service][node.dirname]) {
-					const parentSource = this.paths[service][node.dirname]
-					const parentFolder = files.getNode(parentSource) as Folder & { _children?: string[] }
-
-					if (!parentFolder) {
-						logger.error('Parent folder not found', { parentSource })
-						return
-					}
-
-					logger.debug('Path exists, removing from children', { parentFolder, node })
-
-					// ensure sources are unique
-					const children = new Set(parentFolder._children ?? [])
-					children.delete(node.source)
-					Vue.set(parentFolder, '_children', [...children.values()])
-					return
-				}
-
-				logger.debug('Parent path does not exists, skipping children update', { node })
+				this.deleteNodeFromParentChildren(node)
 			},
 
-			onCreatedNode(node: Node) {
+			onMovedNode({ node, oldSource }: { node: Node, oldSource: string }) {
 				const service = getNavigation()?.active?.id || 'files'
-				if (!node.fileid) {
-					logger.error('Node has no fileid', { node })
-					return
-				}
 
-				// Only add path if it's a folder
+				// Update the path of the node
 				if (node.type === FileType.Folder) {
+					// Delete the old path if it exists
+					const oldPath = Object.entries(this.paths[service]).find(([, source]) => source === oldSource)
+					if (oldPath?.[0]) {
+						this.deletePath(service, oldPath[0])
+					}
+
+					// Add the new path
 					this.addPath({
 						service,
 						path: node.path,
 						source: node.source,
 					})
 				}
 
-				// Update parent folder children if exists
-				// If the folder is the root, get it and update it
-				if (node.dirname === '/') {
-					const root = files.getRoot(service) as Folder & { _children?: string[] }
+				// Dummy simple clone of the renamed node from a previous state
+				const oldNode = new File({ source: oldSource, owner: node.owner, mime: node.mime })
+
+				this.deleteNodeFromParentChildren(oldNode)
+				this.addNodeToParentChildren(node)
+			},
+
+			deleteNodeFromParentChildren(node: Node) {
+				const service = getNavigation()?.active?.id || 'files'
+
+				// Update children of a root folder
+				const parentSource = dirname(node.source)
+				const folder = (node.dirname === '/' ? files.getRoot(service) : files.getNode(parentSource)) as Folder & { _children?: string[] }
+				if (folder) {
 					// ensure sources are unique
-					const children = new Set(root._children ?? [])
-					children.add(node.source)
-					Vue.set(root, '_children', [...children.values()])
+					const children = new Set(folder._children ?? [])
+					children.delete(node.source)
+					Vue.set(folder, '_children', [...children.values()])
+					logger.debug('Children updated', { parent: folder, node, children: folder._children })
 					return
 				}
 
-				// If the folder doesn't exists yet, it will be
-				// fetched later and its children updated anyway.
-				if (this.paths[service][node.dirname]) {
-					const parentSource = this.paths[service][node.dirname]
-					const parentFolder = files.getNode(parentSource) as Folder & { _children?: string[] }
-					logger.debug('Path already exists, updating children', { parentFolder, node })
+				logger.debug('Parent path does not exists, skipping children update', { node })
+			},
 
-					if (!parentFolder) {
-						logger.error('Parent folder not found', { parentSource })
-						return
-					}
+			addNodeToParentChildren(node: Node) {
+				const service = getNavigation()?.active?.id || 'files'
 
+				// Update children of a root folder
+				const parentSource = dirname(node.source)
+				const folder = (node.dirname === '/' ? files.getRoot(service) : files.getNode(parentSource)) as Folder & { _children?: string[] }
+				if (folder) {
 					// ensure sources are unique
-					const children = new Set(parentFolder._children ?? [])
+					const children = new Set(folder._children ?? [])
 					children.add(node.source)
-					Vue.set(parentFolder, '_children', [...children.values()])
+					Vue.set(folder, '_children', [...children.values()])
+					logger.debug('Children updated', { parent: folder, node, children: folder._children })
 					return
 				}
 
 				logger.debug('Parent path does not exists, skipping children update', { node })
 			},
+
 		},
 	})
 
 	const pathsStore = store(...args)
 	// Make sure we only register the listeners once
 	if (!pathsStore._initialized) {
-		// TODO: watch folders to update paths?
 		subscribe('files:node:created', pathsStore.onCreatedNode)
 		subscribe('files:node:deleted', pathsStore.onDeletedNode)
-		// subscribe('files:node:moved', pathsStore.onMovedNode)
+		subscribe('files:node:moved', pathsStore.onMovedNode)
 
 		pathsStore._initialized = true
 	}