Merge pull request #42584 from nextcloud/fix/files-sharing-various

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,46 @@ const trashbinView = {
 } as View
 
 describe('Delete action conditions tests', () => {
+	afterEach(() => {
+		jest.restoreAllMocks()
+	})
+
+	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/admin/foobar.txt',
+		owner: 'admin',
+		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 values', () => {
+		jest.spyOn(auth, 'getCurrentUser').mockReturnValue(null)
+		expect(action.displayName([file2], view)).toBe('Unshare')
+	})
+
+	test('Shared and owned nodes values', () => {
+		expect(action.displayName([file, file2], view)).toBe('Delete and unshare')
 	})
 })
 
@@ -55,8 +85,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 +98,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 +116,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 +141,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 +151,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 +163,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 +181,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 +195,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 +205,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)

apps/files/src/actions/deleteAction.ts

@@ -24,17 +24,42 @@ import { Permission, Node, View, FileAction } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
 import axios from '@nextcloud/axios'
 import TrashCanSvg from '@mdi/svg/svg/trash-can.svg?raw'
+import CloseSvg from '@mdi/svg/svg/close.svg?raw'
 
 import logger from '../logger.js'
+import { getCurrentUser } from '@nextcloud/auth'
+
+const isAllUnshare = (nodes: Node[]) => {
+	return !nodes.some(node => node.owner === getCurrentUser()?.uid)
+}
+
+const isMixedUnshareAndDelete = (nodes: Node[]) => {
+	const hasUnshareItems = nodes.some(node => node.owner !== getCurrentUser()?.uid)
+	const hasDeleteItems = nodes.some(node => node.owner === getCurrentUser()?.uid)
+	return hasUnshareItems && hasDeleteItems
+}
 
 export const action = new FileAction({
 	id: 'delete',
 	displayName(nodes: Node[], view: View) {
+		if (isMixedUnshareAndDelete(nodes)) {
+			return t('files', 'Delete and unshare')
+		}
+
+		if (isAllUnshare(nodes)) {
+			return t('files', 'Unshare')
+		}
+
 		return view.id === 'trashbin'
 			? t('files', 'Delete permanently')
 			: t('files', 'Delete')
 	},
-	iconSvgInline: () => TrashCanSvg,
+	iconSvgInline: (nodes: Node[]) => {
+		if (isAllUnshare(nodes)) {
+			return CloseSvg
+		}
+		return TrashCanSvg
+	},
 
 	enabled(nodes: Node[]) {
 		return nodes.length > 0 && nodes

apps/files/src/services/Files.ts

@@ -42,7 +42,7 @@ interface ResponseProps extends DAVResultResponseProps {
 export const resultToNode = function(node: FileStat): File | Folder {
 	const props = node.props as ResponseProps
 	const permissions = davParsePermissions(props?.permissions)
-	const owner = getCurrentUser()?.uid as string
+	const owner = (props['owner-id'] || getCurrentUser()?.uid) as string
 
 	const source = generateRemoteUrl('dav' + rootPath + node.filename)
 	const id = props?.fileid < 0

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
 		}

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,

package-lock.json

@@ -20,7 +20,7 @@
         "@nextcloud/capabilities": "^1.0.4",
         "@nextcloud/dialogs": "^5.0.3",
         "@nextcloud/event-bus": "^3.1.0",
-        "@nextcloud/files": "^3.0.0",
+        "@nextcloud/files": "^3.1.0",
         "@nextcloud/initial-state": "^2.0.0",
         "@nextcloud/l10n": "^2.1.0",
         "@nextcloud/logger": "^2.5.0",
@@ -3918,9 +3918,9 @@
       "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A=="
     },
     "node_modules/@nextcloud/files": {
-      "version": "3.0.0",
-      "resolved": "https://registry.npmjs.org/@nextcloud/files/-/files-3.0.0.tgz",
-      "integrity": "sha512-zk5oIuVDyk2gWBKCJ+0B1HE3VjhuGnz2iLNbTcbRuTjMYb6aYCAEn1LY0dXbUQG93ehndYJCOdaYri/TaGrlXw==",
+      "version": "3.1.0",
+      "resolved": "https://registry.npmjs.org/@nextcloud/files/-/files-3.1.0.tgz",
+      "integrity": "sha512-i0g9L5HRBJ2vr/gXYb0Gtg379u6nYZJFL30W50OV0F0qlf8OtkAlNpfOVOg3sJf9zklARE2lVY9g2Y9sv/iQ3g==",
       "dependencies": {
         "@nextcloud/auth": "^2.2.1",
         "@nextcloud/l10n": "^2.2.0",
@@ -3928,7 +3928,7 @@
         "@nextcloud/paths": "^2.1.0",
         "@nextcloud/router": "^2.2.0",
         "is-svg": "^5.0.0",
-        "webdav": "^5.3.0"
+        "webdav": "^5.3.1"
       },
       "engines": {
         "node": "^20.0.0",

package.json

@@ -47,7 +47,7 @@
     "@nextcloud/capabilities": "^1.0.4",
     "@nextcloud/dialogs": "^5.0.3",
     "@nextcloud/event-bus": "^3.1.0",
-    "@nextcloud/files": "^3.0.0",
+    "@nextcloud/files": "^3.1.0",
     "@nextcloud/initial-state": "^2.0.0",
     "@nextcloud/l10n": "^2.1.0",
     "@nextcloud/logger": "^2.5.0",