fix(files): Allow downloading multiple nodes not from same base

When downloading files in e.g. the *favorites* or *recent* view,
then the nodes are not always share the same parent folder
and we can not use the current directory as it is probably just a
virtual one.

So we calculate the longest common base and use that as the directory
for the download endpoint.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts
index 56ad3882d213bfd4e757b92fedcdb4cb14a6ae8b..56a7c498d95fff88defa8e02530504b001f9a1be 100644 (file)
--- a/apps/files/src/actions/downloadAction.spec.ts
+++ b/apps/files/src/actions/downloadAction.spec.ts
@@ -21,7 +21,14 @@
  */
 import { action } from './downloadAction'
 import { expect } from '@jest/globals'
-import { File, Folder, Permission, View, FileAction, DefaultType } from '@nextcloud/files'
+import {
+       File,
+       Folder,
+       Permission,
+       View,
+       FileAction,
+       DefaultType,
+} from '@nextcloud/files'
 
 const view = {
        id: 'files',
@@ -115,7 +122,9 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toBe(null)
                expect(link.download).toEqual('')
-               expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+               expect(link.href).toEqual(
+                       'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
+               )
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -133,7 +142,9 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toStrictEqual([null])
                expect(link.download).toEqual('')
-               expect(link.href).toEqual('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+               expect(link.href).toEqual(
+                       'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
+               )
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -150,7 +161,11 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toBe(null)
                expect(link.download).toEqual('')
-               expect(link.href.startsWith('/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22FooBar%22%5D&downloadStartSecret=')).toBe(true)
+               expect(
+                       link.href.startsWith(
+                               '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22FooBar%22%5D&downloadStartSecret=',
+                       ),
+               ).toBe(true)
                expect(link.click).toHaveBeenCalledTimes(1)
        })
 
@@ -175,7 +190,76 @@ describe('Download action execute tests', () => {
                // Silent action
                expect(exec).toStrictEqual([null, null])
                expect(link.download).toEqual('')
-               expect(link.href.startsWith('/index.php/apps/files/ajax/download.php?dir=%2FDir&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D&downloadStartSecret=')).toBe(true)
                expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D&downloadStartSecret=',
+               )
+       })
+
+       test('Download multiple nodes from different sources', async () => {
+               const files = [
+                       new File({
+                               id: 1,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1/foo.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new File({
+                               id: 2,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 2/bar.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new File({
+                               id: 3,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 2/baz.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+               ]
+
+               const exec = await action.execBatch!(files, view, '/Dir')
+
+               // Silent action
+               expect(exec).toStrictEqual([null, null, null])
+               expect(link.download).toEqual('')
+               expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22bar.txt%22%2C%22baz.txt%22%5D&downloadStartSecret=',
+               )
+       })
+
+       test('Download node and parent folder', async () => {
+               const files = [
+                       new File({
+                               id: 1,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1/foo.txt',
+                               owner: 'admin',
+                               mime: 'text/plain',
+                               permissions: Permission.READ,
+                       }),
+                       new Folder({
+                               id: 2,
+                               source: 'https://cloud.domain.com/remote.php/dav/files/admin/Folder 1',
+                               owner: 'admin',
+                               permissions: Permission.READ,
+                       }),
+               ]
+
+               const exec = await action.execBatch!(files, view, '/Dir')
+
+               // Silent action
+               expect(exec).toStrictEqual([null, null])
+               expect(link.download).toEqual('')
+               expect(link.click).toHaveBeenCalledTimes(1)
+
+               expect(link.href).toMatch(
+                       '/index.php/apps/files/ajax/download.php?dir=%2F&files=%5B%22foo.txt%22%2C%22Folder%201%22%5D&downloadStartSecret=',
+               )
        })
 })

diff --git a/apps/files/src/actions/downloadAction.ts b/apps/files/src/actions/downloadAction.ts
index 03686cd42438e1f53b17b823ee12dc5cad944875..8fc5d66db9d5f30bab051e6aa86c44ce5bc04b87 100644 (file)
--- a/apps/files/src/actions/downloadAction.ts
+++ b/apps/files/src/actions/downloadAction.ts
@@ -31,12 +31,57 @@ const triggerDownload = function(url: string) {
        hiddenElement.click()
 }
 
-const downloadNodes = function(dir: string, nodes: Node[]) {
+/**
+ * Find the longest common path prefix of both input paths
+ * @param first The first path
+ * @param second The second path
+ */
+function longestCommonPath(first: string, second: string): string {
+       const firstSegments = first.split('/').filter(Boolean)
+       const secondSegments = second.split('/').filter(Boolean)
+       let base = '/'
+       for (const [index, segment] of firstSegments.entries()) {
+               if (index >= second.length) {
+                       break
+               }
+               if (segment !== secondSegments[index]) {
+                       break
+               }
+               const sep = base === '/' ? '' : '/'
+               base = `${base}${sep}${segment}`
+       }
+       return base
+}
+
+/**
+ * Handle downloading multiple nodes
+ * @param nodes The nodes to download
+ */
+function downloadNodes(nodes: Node[]): void {
+       // Remove nodes that are already included in parent folders
+       // Example: Download A/foo.txt and A will only return A as A/foo.txt is already included
+       const filteredNodes = nodes.filter((node) => {
+               const parent = nodes.find((other) => (
+                       other.type === FileType.Folder
+                               && node.path.startsWith(`${other.path}/`)
+               ))
+               return parent === undefined
+       })
+
+       let base = filteredNodes[0].dirname
+       for (const node of filteredNodes.slice(1)) {
+               base = longestCommonPath(base, node.dirname)
+       }
+       base = base || '/'
+
+       // Remove the common prefix
+       const filenames = filteredNodes.map((node) => node.path.slice(base === '/' ? 1 : (base.length + 1)))
+
        const secret = Math.random().toString(36).substring(2)
-       const url = generateUrl('/apps/files/ajax/download.php?dir={dir}&files={files}&downloadStartSecret={secret}', {
-               dir,
+       const url = generateUrl('/apps/files/ajax/download.php?dir={base}&files={files}&downloadStartSecret={secret}', {
+               base,
                secret,
-               files: JSON.stringify(nodes.map(node => node.basename)),
+               files: JSON.stringify(filenames),
        })
        triggerDownload(url)
 }
@@ -83,7 +128,7 @@ export const action = new FileAction({
 
        async exec(node: Node, view: View, dir: string) {
                if (node.type === FileType.Folder) {
-                       downloadNodes(dir, [node])
+                       downloadNodes([node])
                        return null
                }
 
@@ -97,7 +142,7 @@ export const action = new FileAction({
                        return [null]
                }
 
-               downloadNodes(dir, nodes)
+               downloadNodes(nodes)
                return new Array(nodes.length).fill(null)
        },
 

diff --git a/apps/files/src/actions/editLocallyAction.spec.ts b/apps/files/src/actions/editLocallyAction.spec.ts
index 3d32930c27306db74f21794266b799e8d871da4c..fc4eb35e665d841c106f5ec4c66dc924ab36fe1a 100644 (file)
--- a/apps/files/src/actions/editLocallyAction.spec.ts
+++ b/apps/files/src/actions/editLocallyAction.spec.ts
@@ -137,7 +137,7 @@ describe('Edit locally action enabled tests', () => {
 describe('Edit locally action execute tests', () => {
        test('Edit locally opens proper URL', async () => {
                jest.spyOn(axios, 'post').mockImplementation(async () => ({
-                       data: { ocs: { data: { token: 'foobar' } } }
+                       data: { ocs: { data: { token: 'foobar' } } },
                }))
                const mockedShowError = jest.mocked(showError)
                const spyDialogBuilder = jest.spyOn(dialogBuilder, 'build')

diff --git a/apps/files/src/actions/sidebarAction.ts b/apps/files/src/actions/sidebarAction.ts
index 0db985469348c7d98d055e46f54028dd2fb6e4d0..0e61d772e95d69cabab54707a9679fcdd5d1306e 100644 (file)
--- a/apps/files/src/actions/sidebarAction.ts
+++ b/apps/files/src/actions/sidebarAction.ts
@@ -19,7 +19,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import { Permission, type Node, View, FileAction, FileType } from '@nextcloud/files'
+import { Permission, type Node, View, FileAction } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
 import InformationSvg from '@mdi/svg/svg/information-variant.svg?raw'