Merge pull request #51582 from nextcloud/backport/51250/stable31

[stable31] fix: pass hide download attribute while creating the share to fix github issue 50788

Author: susnux

apps/files_sharing/src/components/SharingInput.vue

@@ -5,13 +5,13 @@
 
 <template>
 	<div class="sharing-search">
-		<label class="hidden-visually" for="sharing-search-input">
+		<label class="hidden-visually" :for="shareInputId">
 			{{ isExternal ? t('files_sharing', 'Enter external recipients')
 				: t('files_sharing', 'Search for internal recipients') }}
 		</label>
 		<NcSelect ref="select"
 			v-model="value"
-			input-id="sharing-search-input"
+			:input-id="shareInputId"
 			class="sharing-search__input"
 			:disabled="!canReshare"
 			:loading="loading"
@@ -87,6 +87,12 @@ export default {
 		},
 	},
 
+	setup() {
+		return {
+			shareInputId: `share-input-${Math.random().toString(36).slice(2, 7)}`,
+		}
+	},
+
 	data() {
 		return {
 			config: new Config(),

apps/files_sharing/src/mixins/ShareDetails.js

@@ -51,6 +51,7 @@ export default {
 						scope: 'permissions',
 					},
 				],
+				hideDownload: false,
 				share_type: shareRequestObject.shareType,
 				share_with: shareRequestObject.shareWith,
 				is_no_user: shareRequestObject.isNoUser,

apps/files_sharing/src/mixins/SharesMixin.js

@@ -278,14 +278,16 @@ export default {
 				// force value to string because that is what our
 				// share api controller accepts
 				propertyNames.forEach(name => {
-					if ((typeof this.share[name]) === 'object') {
+					if (this.share[name] === null || this.share[name] === undefined) {
+						properties[name] = ''
+					} else if ((typeof this.share[name]) === 'object') {
 						properties[name] = JSON.stringify(this.share[name])
 					} else {
 						properties[name] = this.share[name].toString()
 					}
 				})
 
-				this.updateQueue.add(async () => {
+				return this.updateQueue.add(async () => {
 					this.saving = true
 					this.errors = {}
 					try {
@@ -317,7 +319,6 @@ export default {
 						this.saving = false
 					}
 				})
-				return
 			}
 
 			// This share does not exists on the server yet

apps/files_sharing/src/models/Share.ts

@@ -21,6 +21,10 @@ export default class Share {
 			ocsData = ocsData.ocs.data[0]
 		}
 
+		// string to int
+		if (typeof ocsData.id === 'string') {
+			ocsData.id = Number.parseInt(ocsData.id)
+		}
 		// convert int into boolean
 		ocsData.hide_download = !!ocsData.hide_download
 		ocsData.mail_send = !!ocsData.mail_send
@@ -77,7 +81,7 @@ export default class Share {
 	 * Get the share attributes
 	 */
 	get attributes(): Array<ShareAttribute> {
-		return this._share.attributes
+		return this._share.attributes || []
 	}
 
 	/**
@@ -241,6 +245,7 @@ export default class Share {
 	 */
 	get hideDownload(): boolean {
 		return this._share.hide_download === true
+			|| this.attributes.find?.(({ scope, key, value }) => scope === 'permissions' && key === 'download' && !value) !== undefined
 	}
 
 	/**

apps/files_sharing/src/views/SharingDetailsTab.vue

@@ -1005,10 +1005,24 @@ export default {
 
 				this.creating = true
 				const share = await this.addShare(incomingShare)
-				this.creating = false
+				// ugly hack to make code work - we need the id to be set but at the same time we need to keep values we want to update
+				this.share._share.id = share.id
+				await this.queueUpdate(...permissionsAndAttributes)
+				// Also a ugly hack to update the updated permissions
+				for (const prop of permissionsAndAttributes) {
+					if (prop in share && prop in this.share) {
+						try {
+							share[prop] = this.share[prop]
+						} catch {
+							share._share[prop] = this.share[prop]
+						}
+					}
+				}
 				this.share = share
+				this.creating = false
 				this.$emit('add:share', this.share)
 			} else {
+				// Let's update after creation as some attrs are only available after creation
 				this.$emit('update:share', this.share)
 				emit('update:share', this.share)
 				this.queueUpdate(...permissionsAndAttributes)

cypress/e2e/files_sharing/FilesSharingUtils.ts

@@ -19,9 +19,9 @@ export function createShare(fileName: string, username: string, shareSettings: P
 	openSharingPanel(fileName)
 
 	cy.get('#app-sidebar-vue').within(() => {
-		cy.get('#sharing-search-input').clear()
 		cy.intercept({ times: 1, method: 'GET', url: '**/apps/files_sharing/api/v1/sharees?*' }).as('userSearch')
-		cy.get('#sharing-search-input').type(username)
+		cy.findByRole('combobox', { name: /Search for internal recipients/i })
+			.type(`{selectAll}${username}`)
 		cy.wait('@userSearch')
 	})
 

…haring/public-share/download-files.cy.ts …iles_sharing/public-share/download.cy.tscypress/e2e/files_sharing/public-share/download-files.cy.ts renamed to cypress/e2e/files_sharing/public-share/download.cy.ts cypress/e2e/files_sharing/public-share/download-files.cy.ts renamed to cypress/e2e/files_sharing/public-share/download.cy.ts

@@ -4,9 +4,10 @@
  */
 // @ts-expect-error The package is currently broken - but works...
 import { deleteDownloadsFolderBeforeEach } from 'cypress-delete-downloads-folder'
-import { createShare, getShareUrl, setupPublicShare, type ShareContext } from './setup-public-share.ts'
+import { createShare, getShareUrl, openLinkShareDetails, setupPublicShare, type ShareContext } from './setup-public-share.ts'
 import { getRowForFile, getRowForFileId, triggerActionForFile, triggerActionForFileId } from '../../files/FilesUtils.ts'
 import { zipFileContains } from '../../../support/utils/assertions.ts'
+import type { User } from '@nextcloud/cypress'
 
 describe('files_sharing: Public share - downloading files', { testIsolation: true }, () => {
 
@@ -170,4 +171,98 @@ describe('files_sharing: Public share - downloading files', { testIsolation: tru
 			})
 		})
 	})
+
+	describe('download permission - link share', () => {
+		let context: ShareContext
+		beforeEach(() => {
+			cy.createRandomUser().then((user) => {
+				cy.mkdir(user, '/test')
+
+				context = { user }
+				createShare(context, 'test')
+				cy.login(context.user)
+				cy.visit('/apps/files')
+			})
+		})
+
+		deleteDownloadsFolderBeforeEach()
+
+		it('download permission is retained', () => {
+			getRowForFile('test').should('be.visible')
+			triggerActionForFile('test', 'details')
+
+			openLinkShareDetails(0)
+
+			cy.intercept('PUT', '**/ocs/v2.php/apps/files_sharing/api/v1/shares/*').as('update')
+
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('exist')
+				.and('not.be.checked')
+				.check({ force: true })
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('be.checked')
+			cy.findByRole('button', { name: /update share/i })
+				.click()
+
+			cy.wait('@update')
+
+			openLinkShareDetails(0)
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('be.checked')
+
+			cy.reload()
+
+			getRowForFile('test').should('be.visible')
+			triggerActionForFile('test', 'details')
+			openLinkShareDetails(0)
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('be.checked')
+		})
+	})
+
+	describe('download permission - mail share', () => {
+		let user: User
+
+		beforeEach(() => {
+			cy.createRandomUser().then(($user) => {
+				user = $user
+				cy.mkdir(user, '/test')
+				cy.login(user)
+				cy.visit('/apps/files')
+			})
+		})
+
+		it('download permission is retained', () => {
+			getRowForFile('test').should('be.visible')
+			triggerActionForFile('test', 'details')
+
+			cy.findByRole('combobox', { name: /Enter external recipients/i })
+				.type('test@example.com')
+
+			cy.get('.option[sharetype="4"][user="test@example.com"]')
+				.parent('li')
+				.click()
+			cy.findByRole('button', { name: /advanced settings/i })
+				.should('be.visible')
+				.click()
+
+			cy.intercept('PUT', '**/ocs/v2.php/apps/files_sharing/api/v1/shares/*').as('update')
+
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('exist')
+				.and('not.be.checked')
+				.check({ force: true })
+			cy.findByRole('button', { name: /save share/i })
+				.click()
+
+			cy.wait('@update')
+
+			openLinkShareDetails(1)
+			cy.findByRole('button', { name: /advanced settings/i })
+				.click()
+			cy.findByRole('checkbox', { name: /hide download/i })
+				.should('exist')
+				.and('be.checked')
+		})
+	})
 })

cypress/e2e/files_sharing/public-share/setup-public-share.ts

@@ -122,15 +122,24 @@ export function createShare(context: ShareContext, shareName: string, options: S
 }
 
 /**
- * Adjust share permissions to be editable
+ * open link share details for specific index
+ *
+ * @param index
  */
-function adjustSharePermission(): void {
+export function openLinkShareDetails(index: number) {
 	cy.findByRole('list', { name: 'Link shares' })
 		.findAllByRole('listitem')
-		.first()
+		.eq(index)
 		.findByRole('button', { name: /Actions/i })
 		.click()
 	cy.findByRole('menuitem', { name: /Customize link/i }).click()
+}
+
+/**
+ * Adjust share permissions to be editable
+ */
+function adjustSharePermission(): void {
+	openLinkShareDetails(0)
 
 	cy.get('[data-cy-files-sharing-share-permissions-bundle]').should('be.visible')
 	cy.get('[data-cy-files-sharing-share-permissions-bundle="upload-edit"]').click()