Merge pull request #47501 from nextcloud/fix/files-favorites

[stable29] fix: Correctly load favorites for subpath

apps/files/src/services/Favorites.ts

@@ -20,44 +20,38 @@
  *
  */
 import type { ContentsWithRoot } from '@nextcloud/files'
-import type { FileStat, ResponseDataDetailed } from 'webdav'
 
-import { Folder, davGetDefaultPropfind, davGetFavoritesReport } from '@nextcloud/files'
+import { getCurrentUser } from '@nextcloud/auth'
+import { Folder, Permission, davRemoteURL, davRootPath, getFavoriteNodes } from '@nextcloud/files'
+import { CancelablePromise } from 'cancelable-promise'
+import { getContents as filesContents } from './Files.ts'
+import { client } from './WebdavClient.ts'
 
-import { getClient } from './WebdavClient'
-import { resultToNode } from './Files'
-
-const client = getClient()
-
-export const getContents = async (path = '/'): Promise<ContentsWithRoot> => {
-	const propfindPayload = davGetDefaultPropfind()
-	const reportPayload = davGetFavoritesReport()
-
-	// Get root folder
-	let rootResponse
-	if (path === '/') {
-		rootResponse = await client.stat(path, {
-			details: true,
-			data: propfindPayload,
-		}) as ResponseDataDetailed<FileStat>
+export const getContents = (path = '/'): CancelablePromise<ContentsWithRoot> | Promise<ContentsWithRoot> => {
+	// We only filter root files for favorites, for subfolders we can simply reuse the files contents
+	if (path !== '/') {
+		return filesContents(path)
 	}
 
-	const contentsResponse = await client.getDirectoryContents(path, {
-		details: true,
-		// Only filter favorites if we're at the root
-		data: path === '/' ? reportPayload : propfindPayload,
-		headers: {
-			// Patched in WebdavClient.ts
-			method: path === '/' ? 'REPORT' : 'PROPFIND',
-		},
-		includeSelf: true,
-	}) as ResponseDataDetailed<FileStat[]>
-
-	const root = rootResponse?.data || contentsResponse.data[0]
-	const contents = contentsResponse.data.filter(node => node.filename !== path)
-
-	return {
-		folder: resultToNode(root) as Folder,
-		contents: contents.map(resultToNode),
-	}
+	return new CancelablePromise((resolve, reject, cancel) => {
+		const promise = getFavoriteNodes(client)
+			.catch(reject)
+			.then((contents) => {
+				if (!contents) {
+					reject()
+					return
+				}
+				resolve({
+					contents,
+					folder: new Folder({
+						id: 0,
+						source: `${davRemoteURL}${davRootPath}`,
+						root: davRootPath,
+						owner: getCurrentUser()?.uid || null,
+						permissions: Permission.READ,
+					}),
+				})
+			})
+		cancel(() => promise.cancel())
+	})
 }

apps/files/src/services/WebdavClient.ts

@@ -20,47 +20,16 @@
  *
  */
 
-import { createClient, getPatcher } from 'webdav'
 import { generateRemoteUrl } from '@nextcloud/router'
 import { getCurrentUser, getRequestToken, onRequestTokenUpdate } from '@nextcloud/auth'
+import { davGetClient } from '@nextcloud/files'
 
 export const rootPath = `/files/${getCurrentUser()?.uid}`
 export const defaultRootUrl = generateRemoteUrl('dav' + rootPath)
 
-export const getClient = (rootUrl = defaultRootUrl) => {
-	const client = createClient(rootUrl)
-
-	// set CSRF token header
-	const setHeaders = (token: string | null) => {
-		client?.setHeaders({
-			// Add this so the server knows it is an request from the browser
-			'X-Requested-With': 'XMLHttpRequest',
-			// Inject user auth
-			requesttoken: token ?? '',
-		});
-	}
-
-	// refresh headers when request token changes
-	onRequestTokenUpdate(setHeaders)
-	setHeaders(getRequestToken())
-
-	/**
-	 * Allow to override the METHOD to support dav REPORT
-	 *
-	 * @see https://github.com/perry-mitchell/webdav-client/blob/8d9694613c978ce7404e26a401c39a41f125f87f/source/request.ts
-	 */
-	const patcher = getPatcher()
-	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-	// @ts-ignore
-	// https://github.com/perry-mitchell/hot-patcher/issues/6
-	patcher.patch('fetch', (url: string, options: RequestInit): Promise<Response> => {
-		const headers = options.headers as Record<string, string>
-		if (headers?.method) {
-			options.method = headers.method
-			delete headers.method
-		}
-		return fetch(url, options)
-	})
+/**
+ * @deprecated use `davGetClient` from `@nextcloud/files`
+ */
+export const getClient = (rootUrl = defaultRootUrl) => davGetClient(rootUrl)
 
-	return client;
-}
+export const client = davGetClient()

apps/files/src/views/favorites.spec.ts

@@ -23,7 +23,8 @@
 import { basename } from 'path'
 import { expect } from '@jest/globals'
 import { Folder, Navigation, getNavigation } from '@nextcloud/files'
-import eventBus, { emit } from '@nextcloud/event-bus'
+import { CancelablePromise } from 'cancelable-promise'
+import eventBus from '@nextcloud/event-bus'
 import * as initialState from '@nextcloud/initial-state'
 
 import { action } from '../actions/favoriteAction'
@@ -57,7 +58,7 @@ describe('Favorites view definition', () => {
 
 	test('Default empty favorite view', () => {
 		jest.spyOn(eventBus, 'subscribe')
-		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
+		jest.spyOn(favoritesService, 'getContents').mockReturnValue(CancelablePromise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()
 		const favoritesView = Navigation.views.find(view => view.id === 'favorites')
@@ -89,7 +90,7 @@ describe('Favorites view definition', () => {
 			{ fileid: 3, path: '/foo/bar' },
 		]
 		jest.spyOn(initialState, 'loadState').mockReturnValue(favoriteFolders)
-		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
+		jest.spyOn(favoritesService, 'getContents').mockReturnValue(CancelablePromise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()
 		const favoritesView = Navigation.views.find(view => view.id === 'favorites')
@@ -132,7 +133,7 @@ describe('Dynamic update of favourite folders', () => {
 	test('Add a favorite folder creates a new entry in the navigation', async () => {
 		jest.spyOn(eventBus, 'emit')
 		jest.spyOn(initialState, 'loadState').mockReturnValue([])
-		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
+		jest.spyOn(favoritesService, 'getContents').mockReturnValue(CancelablePromise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()
 		const favoritesView = Navigation.views.find(view => view.id === 'favorites')
@@ -161,7 +162,7 @@ describe('Dynamic update of favourite folders', () => {
 		jest.spyOn(eventBus, 'emit')
 		jest.spyOn(eventBus, 'subscribe')
 		jest.spyOn(initialState, 'loadState').mockReturnValue([{ fileid: 42, path: '/Foo/Bar' }])
-		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
+		jest.spyOn(favoritesService, 'getContents').mockReturnValue(CancelablePromise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()
 		let favoritesView = Navigation.views.find(view => view.id === 'favorites')
@@ -201,7 +202,8 @@ describe('Dynamic update of favourite folders', () => {
 	test('Renaming a favorite folder updates the navigation', async () => {
 		jest.spyOn(eventBus, 'emit')
 		jest.spyOn(initialState, 'loadState').mockReturnValue([])
-		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
+		jest.spyOn(favoritesService, 'getContents')
+			.mockReturnValue(CancelablePromise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()
 		const favoritesView = Navigation.views.find(view => view.id === 'favorites')
@@ -212,8 +214,6 @@ describe('Dynamic update of favourite folders', () => {
 		expect(favoritesView).toBeDefined()
 		expect(favoriteFoldersViews.length).toBe(0)
 
-		// expect(eventBus.emit).toHaveBeenCalledTimes(2)
-
 		// Create new folder to favorite
 		const folder = new Folder({
 			id: 1,
@@ -233,7 +233,7 @@ describe('Dynamic update of favourite folders', () => {
 		})
 
 		// Exec the rename action
-		emit('files:node:renamed', renamedFolder)
+		eventBus.emit('files:node:renamed', renamedFolder)
 		expect(eventBus.emit).toHaveBeenNthCalledWith(2, 'files:node:renamed', renamedFolder)
 	})
 })