fix(files): group duplicate shares

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

4 files changed, 100 insertions, 25 deletions

diff --git a/apps/files/src/actions/deleteAction.spec.ts b/apps/files/src/actions/deleteAction.spec.ts
index cf29d385240..195d257c017 100644
--- a/apps/files/src/actions/deleteAction.spec.ts
+++ b/apps/files/src/actions/deleteAction.spec.ts
@@ -22,6 +22,7 @@
 import { action } from './deleteAction'
 import { expect } from '@jest/globals'
 import { File, Folder, Permission, View, FileAction } from '@nextcloud/files'
+import * as auth from '@nextcloud/auth'
 import * as eventBus from '@nextcloud/event-bus'
 import axios from '@nextcloud/axios'
 import logger from '../logger'
@@ -37,17 +38,42 @@ const trashbinView = {
 } as View
 
 describe('Delete action conditions tests', () => {
+	const file = new File({
+		id: 1,
+		source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+		owner: 'test',
+		mime: 'text/plain',
+		permissions: Permission.ALL,
+	})
+
+	const file2 = new File({
+		id: 1,
+		source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+		owner: 'user2',
+		mime: 'text/plain',
+		permissions: Permission.ALL,
+	})
+
 	test('Default values', () => {
 		expect(action).toBeInstanceOf(FileAction)
 		expect(action.id).toBe('delete')
-		expect(action.displayName([], view)).toBe('Delete')
+		expect(action.displayName([file], view)).toBe('Delete')
 		expect(action.iconSvgInline([], view)).toBe('<svg>SvgMock</svg>')
 		expect(action.default).toBeUndefined()
 		expect(action.order).toBe(100)
 	})
 
 	test('Default trashbin view values', () => {
-		expect(action.displayName([], trashbinView)).toBe('Delete permanently')
+		expect(action.displayName([file], trashbinView)).toBe('Delete permanently')
+	})
+
+	test('Shared node view values', () => {
+		jest.spyOn(auth, 'getCurrentUser').mockReturnValue(null)
+		expect(action.displayName([file2], view)).toBe('Unshare')
+	})
+
+	test('Shared and owned nodes view values', () => {
+		expect(action.displayName([file, file2], view)).toBe('Delete and unshare')
 	})
 })
 
@@ -55,8 +81,8 @@ describe('Delete action enabled tests', () => {
 	test('Enabled with DELETE permissions', () => {
 		const file = new File({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.ALL,
 		})
@@ -68,8 +94,8 @@ describe('Delete action enabled tests', () => {
 	test('Disabled without DELETE permissions', () => {
 		const file = new File({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.READ,
 		})
@@ -86,14 +112,14 @@ describe('Delete action enabled tests', () => {
 	test('Disabled if not all nodes can be deleted', () => {
 		const folder1 = new Folder({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/Foo/',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/Foo/',
+			owner: 'test',
 			permissions: Permission.DELETE,
 		})
 		const folder2 = new Folder({
 			id: 2,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/Bar/',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/Bar/',
+			owner: 'test',
 			permissions: Permission.READ,
 		})
 
@@ -111,8 +137,8 @@ describe('Delete action execute tests', () => {
 
 		const file = new File({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
 		})
@@ -121,7 +147,7 @@ describe('Delete action execute tests', () => {
 
 		expect(exec).toBe(true)
 		expect(axios.delete).toBeCalledTimes(1)
-		expect(axios.delete).toBeCalledWith('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+		expect(axios.delete).toBeCalledWith('https://cloud.domain.com/remote.php/dav/files/test/foobar.txt')
 
 		expect(eventBus.emit).toBeCalledTimes(1)
 		expect(eventBus.emit).toBeCalledWith('files:node:deleted', file)
@@ -133,16 +159,16 @@ describe('Delete action execute tests', () => {
 
 		const file1 = new File({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/foo.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
 		})
 
 		const file2 = new File({
 			id: 2,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/bar.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
 		})
@@ -151,8 +177,8 @@ describe('Delete action execute tests', () => {
 
 		expect(exec).toStrictEqual([true, true])
 		expect(axios.delete).toBeCalledTimes(2)
-		expect(axios.delete).toHaveBeenNthCalledWith(1, 'https://cloud.domain.com/remote.php/dav/files/admin/foo.txt')
-		expect(axios.delete).toHaveBeenNthCalledWith(2, 'https://cloud.domain.com/remote.php/dav/files/admin/bar.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(1, 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(2, 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt')
 
 		expect(eventBus.emit).toBeCalledTimes(2)
 		expect(eventBus.emit).toHaveBeenNthCalledWith(1, 'files:node:deleted', file1)
@@ -165,8 +191,8 @@ describe('Delete action execute tests', () => {
 
 		const file = new File({
 			id: 1,
-			source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
-			owner: 'admin',
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foobar.txt',
+			owner: 'test',
 			mime: 'text/plain',
 			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
 		})
@@ -175,7 +201,7 @@ describe('Delete action execute tests', () => {
 
 		expect(exec).toBe(false)
 		expect(axios.delete).toBeCalledTimes(1)
-		expect(axios.delete).toBeCalledWith('https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt')
+		expect(axios.delete).toBeCalledWith('https://cloud.domain.com/remote.php/dav/files/test/foobar.txt')
 
 		expect(eventBus.emit).toBeCalledTimes(0)
 		expect(logger.error).toBeCalledTimes(1)
diff --git a/apps/files/src/actions/deleteAction.ts b/apps/files/src/actions/deleteAction.ts
index 29f1cf86c9b..d3f3a256517 100644
--- a/apps/files/src/actions/deleteAction.ts
+++ b/apps/files/src/actions/deleteAction.ts
@@ -29,10 +29,16 @@ import CloseSvg from '@mdi/svg/svg/close.svg?raw'
 import logger from '../logger.js'
 import { getCurrentUser } from '@nextcloud/auth'
 
+// A file which the owner is NOT the current user
+// is considered shared. Other methods like group folders
+// will always use the current user as the owner.
+// It will therefore not be considered shared.
 const isAllUnshare = (nodes: Node[]) => {
-	return !nodes.some(node => node.owner === getCurrentUser()?.uid)
+	return !nodes.some(node => node.owner !== getCurrentUser()?.uid)
 }
 
+// Check whether the selection contains a mix of files
+// the current user owns and files owned by other users.
 const isMixedUnshareAndDelete = (nodes: Node[]) => {
 	const hasUnshareItems = nodes.some(node => node.owner !== getCurrentUser()?.uid)
 	const hasDeleteItems = nodes.some(node => node.owner === getCurrentUser()?.uid)
@@ -73,7 +79,7 @@ export const action = new FileAction({
 
 			// Let's delete even if it's moved to the trashbin
 			// since it has been removed from the current view
-			//  and changing the view will trigger a reload anyway.
+			// and changing the view will trigger a reload anyway.
 			emit('files:node:deleted', node)
 			return true
 		} catch (error) {
diff --git a/apps/files_sharing/src/actions/sharingStatusAction.ts b/apps/files_sharing/src/actions/sharingStatusAction.ts
index 376f294d1e5..98a7d3d6112 100644
--- a/apps/files_sharing/src/actions/sharingStatusAction.ts
+++ b/apps/files_sharing/src/actions/sharingStatusAction.ts
@@ -62,6 +62,11 @@ export const action = new FileAction({
 		const ownerId = node?.attributes?.['owner-id']
 		const ownerDisplayName = node?.attributes?.['owner-display-name']
 
+		// Mixed share types
+		if (Array.isArray(node.attributes?.['share-types'])) {
+			return t('files_sharing', 'Shared multiple times with different people')
+		}
+
 		if (ownerId && ownerId !== getCurrentUser()?.uid) {
 			return t('files_sharing', 'Shared by {ownerDisplayName}', { ownerDisplayName })
 		}
@@ -73,6 +78,11 @@ export const action = new FileAction({
 		const node = nodes[0]
 		const shareTypes = Object.values(node?.attributes?.['share-types'] || {}).flat() as number[]
 
+		// Mixed share types
+		if (Array.isArray(node.attributes?.['share-types'])) {
+			return AccountPlusSvg
+		}
+
 		// Link shares
 		if (shareTypes.includes(Type.SHARE_TYPE_LINK)
 			|| shareTypes.includes(Type.SHARE_TYPE_EMAIL)) {
@@ -105,6 +115,15 @@ export const action = new FileAction({
 
 		const node = nodes[0]
 		const ownerId = node?.attributes?.['owner-id']
+		const isMixed = Array.isArray(node.attributes?.['share-types'])
+
+		// If the node is shared multiple times with
+		// different share types to the current user
+		if (isMixed) {
+			return true
+		}
+
+		// If the node is shared by someone else
 		if (ownerId && ownerId !== getCurrentUser()?.uid) {
 			return true
 		}
diff --git a/apps/files_sharing/src/services/SharingService.ts b/apps/files_sharing/src/services/SharingService.ts
index 4534183a637..2f167dab535 100644
--- a/apps/files_sharing/src/services/SharingService.ts
+++ b/apps/files_sharing/src/services/SharingService.ts
@@ -76,6 +76,10 @@ const ocsEntryToNode = function(ocsEntry: any): Folder | File | null {
 			attributes: {
 				...ocsEntry,
 				'has-preview': hasPreview,
+				// Also check the sharingStatusAction.ts code
+				'owner-id': ocsEntry?.uid_owner,
+				'owner-display-name': ocsEntry?.displayname_owner,
+				'share-types': ocsEntry?.share_type,
 				favorite: ocsEntry?.tags?.includes(window.OC.TAG_FAVORITE) ? 1 : 0,
 			},
 		})
@@ -144,6 +148,17 @@ const getDeletedShares = function(): AxiosPromise<OCSResponse<any>> {
 	})
 }
 
+/**
+ * Group an array of objects (here Nodes) by a key
+ * and return an array of arrays of them.
+ */
+const groupBy = function(nodes: (Folder | File)[], key: string) {
+	return Object.values(nodes.reduce(function(acc, curr) {
+		(acc[curr[key]] = acc[curr[key]] || []).push(curr)
+		return acc
+	}, {})) as (Folder | File)[][]
+}
+
 export const getContents = async (sharedWithYou = true, sharedWithOthers = true, pendingShares = false, deletedshares = false, filterTypes: number[] = []): Promise<ContentsWithRoot> => {
 	const promises = [] as AxiosPromise<OCSResponse<any>>[]
 
@@ -162,12 +177,21 @@ export const getContents = async (sharedWithYou = true, sharedWithOthers = true,
 
 	const responses = await Promise.all(promises)
 	const data = responses.map((response) => response.data.ocs.data).flat()
-	let contents = data.map(ocsEntryToNode).filter((node) => node !== null) as (Folder | File)[]
+	let contents = data.map(ocsEntryToNode)
+		.filter((node) => node !== null) as (Folder | File)[]
 
 	if (filterTypes.length > 0) {
 		contents = contents.filter((node) => filterTypes.includes(node.attributes?.share_type))
 	}
 
+	// Merge duplicate shares and group their attributes
+	// Also check the sharingStatusAction.ts code
+	contents = groupBy(contents, 'source').map((nodes) => {
+		const node = nodes[0]
+		node.attributes['share-types'] = nodes.map(node => node.attributes['share-types'])
+		return node
+	})
+
 	return {
 		folder: new Folder({
 			id: 0,