[stable34] Fix cancelling the customization of a share

apps/files_sharing/src/components/SharingEntry.vue

@@ -128,15 +128,6 @@ export default {
 			return (typeof this.share.status === 'object' && !Array.isArray(this.share.status))
 		},
 	},
-
-	methods: {
-		/**
-		 * Save potential changed data on menu close
-		 */
-		onMenuClose() {
-			this.onNoteSubmit()
-		},
-	},
 }
 </script>
 

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -147,8 +147,7 @@
 			class="sharing-entry__actions"
 			:aria-label="actionsTooltip"
 			menu-align="right"
-			:open.sync="open"
-			@close="onMenuClose">
+			:open.sync="open">
 			<template v-if="share">
 				<template v-if="share.canEdit && canReshare">
 					<NcActionButton
@@ -452,30 +451,6 @@ export default {
 			return OC.appswebroots.spreed !== undefined
 		},
 
-		/**
-		 * Is it possible to protect the password by Talk?
-		 *
-		 * @return {boolean}
-		 */
-		isPasswordProtectedByTalkAvailable() {
-			return this.isPasswordProtected && this.isTalkEnabled
-		},
-
-		/**
-		 * Is the current share password protected by Talk?
-		 *
-		 * @return {boolean}
-		 */
-		isPasswordProtectedByTalk: {
-			get() {
-				return this.share.sendPasswordByTalk
-			},
-
-			async set(enabled) {
-				this.share.sendPasswordByTalk = enabled
-			},
-		},
-
 		/**
 		 * Is the current share an email share ?
 		 *
@@ -487,20 +462,6 @@ export default {
 				: false
 		},
 
-		canTogglePasswordProtectedByTalkAvailable() {
-			if (!this.isPasswordProtected) {
-				// Makes no sense
-				return false
-			} else if (this.isEmailShareType && !this.hasUnsavedPassword) {
-				// For email shares we need a new password in order to enable or
-				// disable
-				return false
-			}
-
-			// Anything else should be fine
-			return true
-		},
-
 		/**
 		 * Pending data.
 		 * If the share still doesn't have an id, it is not synced
@@ -863,46 +824,6 @@ export default {
 			}
 		},
 
-		/**
-		 * Menu have been closed or password has been submitted.
-		 * The only property that does not get
-		 * synced automatically is the password
-		 * So let's check if we have an unsaved
-		 * password.
-		 * expireDate is saved on datepicker pick
-		 * or close.
-		 */
-		onPasswordSubmit() {
-			if (this.hasUnsavedPassword) {
-				this.share.newPassword = this.share.newPassword.trim()
-				this.queueUpdate('password')
-			}
-		},
-
-		/**
-		 * Update the password along with "sendPasswordByTalk".
-		 *
-		 * If the password was modified the new password is sent; otherwise
-		 * updating a mail share would fail, as in that case it is required that
-		 * a new password is set when enabling or disabling
-		 * "sendPasswordByTalk".
-		 */
-		onPasswordProtectedByTalkChange() {
-			if (this.hasUnsavedPassword) {
-				this.share.newPassword = this.share.newPassword.trim()
-			}
-
-			this.queueUpdate('sendPasswordByTalk', 'password')
-		},
-
-		/**
-		 * Save potential changed data on menu close
-		 */
-		onMenuClose() {
-			this.onPasswordSubmit()
-			this.onNoteSubmit()
-		},
-
 		/**
 		 * @param {boolean} enabled True if expiration is enabled
 		 */

apps/files_sharing/src/mixins/SharesMixin.js

@@ -261,27 +261,6 @@ export default {
 			this.share.expireDate = this.formatDateToString(parsedDate)
 		},
 
-		/**
-		 * Note changed, let's save it to a different key
-		 *
-		 * @param {string} note the share note
-		 */
-		onNoteChange(note) {
-			this.$set(this.share, 'newNote', note.trim())
-		},
-
-		/**
-		 * When the note change, we trim, save and dispatch
-		 *
-		 */
-		onNoteSubmit() {
-			if (this.share.newNote) {
-				this.share.note = this.share.newNote
-				this.$delete(this.share, 'newNote')
-				this.queueUpdate('note')
-			}
-		},
-
 		/**
 		 * Delete share button handler
 		 */
@@ -348,7 +327,7 @@ export default {
 						if (propertyNames.includes('password')) {
 							// reset password state after sync
 							this.share.password = this.share.newPassword || undefined
-							this.$delete(this.share, 'newPassword')
+							this.$set(this.share, 'newPassword', undefined)
 
 							// updates password expiration time after sync
 							this.share.passwordExpirationTime = updatedShare.password_expiration_time
@@ -419,7 +398,7 @@ export default {
 				if (this.share.newPassword === this.share.password) {
 					this.share.password = ''
 				}
-				this.$delete(this.share, 'newPassword')
+				this.$set(this.share, 'newPassword', undefined)
 			}
 
 			// re-open menu if closed

apps/files_sharing/src/models/Share.ts

@@ -422,7 +422,7 @@ export default class Share {
 		const hasDisabledDownload = (attribute) => {
 			return attribute.scope === 'permissions' && attribute.key === 'download' && attribute.value === false
 		}
-		return this.attributes.some(hasDisabledDownload)
+		return !this.attributes.some(hasDisabledDownload)
 	}
 
 	/**

apps/files_sharing/src/views/SharingDetailsTab.vue

@@ -155,8 +155,7 @@
 					</template>
 					<NcCheckboxRadioSwitch
 						v-if="canTogglePasswordProtectedByTalkAvailable"
-						v-model="isPasswordProtectedByTalk"
-						@update:modelValue="onPasswordProtectedByTalkChange">
+						v-model="isPasswordProtectedByTalk">
 						{{ t('files_sharing', 'Video verification') }}
 					</NcCheckboxRadioSwitch>
 					<NcCheckboxRadioSwitch v-model="hasExpirationDate" :disabled="isExpiryDateEnforced">
@@ -178,8 +177,7 @@
 					<NcCheckboxRadioSwitch
 						v-if="isPublicShare"
 						v-model="share.hideDownload"
-						:disabled="canChangeHideDownload"
-						@update:modelValue="queueUpdate('hideDownload')">
+						:disabled="canChangeHideDownload">
 						{{ t('files_sharing', 'Hide download') }}
 					</NcCheckboxRadioSwitch>
 					<NcCheckboxRadioSwitch
@@ -400,6 +398,13 @@ export default {
 			creating: false,
 			initialToken: this.share.token,
 			loadingToken: false,
+			initialPermissions: undefined,
+			initialExpireDate: undefined,
+			initialNote: undefined,
+			initialLabel: undefined,
+			initialHideDownload: undefined,
+			initialSendPasswordByTalk: undefined,
+			initialHasDownloadPermission: undefined,
 
 			externalShareActions: getSidebarActions(),
 			// legacy
@@ -867,6 +872,14 @@ export default {
 	},
 
 	beforeMount() {
+		this.initialPermissions = this.share.permissions
+		this.initialExpireDate = this.share.expireDate
+		this.initialNote = this.share.note
+		this.initialLabel = this.share.label
+		this.initialHideDownload = this.share.hideDownload
+		this.initialSendPasswordByTalk = this.share.sendPasswordByTalk
+		this.initialHasDownloadPermission = this.share.hasDownloadPermission
+
 		this.initializePermissions()
 		this.initializeAttributes()
 		logger.debug('Share object received', { share: this.share })
@@ -931,6 +944,16 @@ export default {
 
 		cancel() {
 			this.share.token = this.initialToken
+			this.share.permissions = this.initialPermissions
+			this.share.expireDate = this.initialExpireDate
+			this.share.note = this.initialNote
+			this.share.label = this.initialLabel
+			this.share.hideDownload = this.initialHideDownload
+			this.share.sendPasswordByTalk = this.initialSendPasswordByTalk
+			this.share.hasDownloadPermission = this.initialHasDownloadPermission
+
+			this.$set(this.share, 'newPassword', undefined)
+
 			this.$emit('close-sharing-details')
 		},
 
@@ -1065,7 +1088,7 @@ export default {
 
 		async saveShare() {
 			const permissionsAndAttributes = ['permissions', 'attributes', 'note', 'expireDate']
-			const publicShareAttributes = ['label', 'hideDownload']
+			const publicShareAttributes = ['label', 'hideDownload', 'sendPasswordByTalk']
 			// Only include password if it's being actively changed
 			if (this.hasUnsavedPassword) {
 				publicShareAttributes.push('password')
@@ -1099,6 +1122,13 @@ export default {
 				this.share.password = ''
 			}
 
+			// "Video verification" must be disabled if the password was
+			// disabled, as it does not make sense and would also prevent
+			// saving if it is still enabled.
+			if (this.isPasswordProtectedByTalk && !this.isPasswordProtected) {
+				this.isPasswordProtectedByTalk = false
+			}
+
 			if (!this.hasExpirationDate) {
 				this.share.expireDate = ''
 			}
@@ -1228,30 +1258,14 @@ export default {
 		 */
 		onPasswordChange(password) {
 			if (password === '') {
-				this.$delete(this.share, 'newPassword')
+				this.$set(this.share, 'newPassword', undefined)
 				this.passwordError = this.isNewShare && this.isPasswordEnforced
 				return
 			}
 			this.passwordError = !this.isValidShareAttribute(password)
 			this.$set(this.share, 'newPassword', password)
 		},
 
-		/**
-		 * Update the password along with "sendPasswordByTalk".
-		 *
-		 * If the password was modified the new password is sent; otherwise
-		 * updating a mail share would fail, as in that case it is required that
-		 * a new password is set when enabling or disabling
-		 * "sendPasswordByTalk".
-		 */
-		onPasswordProtectedByTalkChange() {
-			if (this.isEmailShareType || this.hasUnsavedPassword) {
-				this.queueUpdate('sendPasswordByTalk', 'password')
-			} else {
-				this.queueUpdate('sendPasswordByTalk')
-			}
-		},
-
 		isValidShareAttribute(value) {
 			if ([null, undefined].includes(value)) {
 				return false

build/eslint-baseline-legacy.json

@@ -16,7 +16,7 @@
   },
   "apps/files_sharing/src/views/SharingDetailsTab.vue": {
     "vue/no-mutating-props": {
-      "count": 24
+      "count": 31
     }
   },
   "apps/files_sharing/src/views/SharingLinkList.vue": {

tests/playwright/e2e/files_sharing/public-share/share-editor.spec.ts

@@ -41,8 +41,8 @@ test.describe('files_sharing: Link share editor', () => {
 		await sharingTab.openLinkShareDetails()
 		await sharingTab.openAdvancedSettings()
 		await expect(sharingTab.checkbox('Hide download')).not.toBeChecked()
-		await sharingTab.setCheckbox('Hide download', true, { persists: true })
-		await sharingTab.closeDetails()
+		await sharingTab.setCheckbox('Hide download', true)
+		await sharingTab.save()
 
 		// Still set when the editor is opened again …
 		await sharingTab.openLinkShareDetails()
@@ -56,6 +56,74 @@ test.describe('files_sharing: Link share editor', () => {
 		await sharingTab.openAdvancedSettings()
 		await expect(sharingTab.checkbox('Hide download')).toBeChecked()
 	})
+
+	test('cancelling the edition resets to the previous state', async ({ page, filesListPage, sharingTab }) => {
+		await openSharingPanel(filesListPage, sharingTab, 'test')
+
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.labelInput()).toHaveValue('')
+		await sharingTab.labelInput().fill('The label')
+		await expect(sharingTab.checkbox('Set password')).not.toBeChecked()
+		await sharingTab.setCheckbox('Set password', true)
+		// A password is automatically generated and added to the input
+		await expect(sharingTab.checkbox('Set expiration date')).not.toBeChecked()
+		await sharingTab.setCheckbox('Set expiration date', true)
+		// A default expiration date is automatically added to the input
+		await expect(sharingTab.checkbox('Hide download')).not.toBeChecked()
+		await sharingTab.setCheckbox('Hide download', true)
+		await expect(sharingTab.checkbox('Note to recipient')).not.toBeChecked()
+		await sharingTab.setCheckbox('Note to recipient', true)
+		await sharingTab.noteInput().fill('The note')
+		await expect(sharingTab.checkbox('Custom permissions')).not.toBeChecked()
+		await sharingTab.setCheckbox('Custom permissions', true)
+		await expect(sharingTab.checkbox('Edit')).not.toBeChecked()
+		await sharingTab.setCheckbox('Edit', true)
+		await sharingTab.cancel()
+
+		// Back to the original state when the editor is opened again …
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.labelInput()).toHaveValue('')
+		await expect(sharingTab.checkbox('Set password')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Set expiration date')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Hide download')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Note to recipient')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Custom permissions')).not.toBeChecked()
+
+		// … and after a reload, i.e. it was not stored
+		await page.reload()
+		await openSharingPanel(filesListPage, sharingTab, 'test')
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.labelInput()).toHaveValue('')
+		await expect(sharingTab.checkbox('Set password')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Set expiration date')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Hide download')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Note to recipient')).not.toBeChecked()
+		await expect(sharingTab.checkbox('Custom permissions')).not.toBeChecked()
+	})
+
+	test('the password is unchecked after clearing and saving it', async ({ filesListPage, sharingTab }) => {
+		await openSharingPanel(filesListPage, sharingTab, 'test')
+
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.checkbox('Set password')).not.toBeChecked()
+		await sharingTab.setCheckbox('Set password', true)
+		// A password is automatically generated and added to the input
+		await sharingTab.save()
+
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.checkbox('Set password')).toBeChecked()
+		await sharingTab.setCheckbox('Set password', false)
+		await sharingTab.save()
+
+		await sharingTab.openLinkShareDetails()
+		await sharingTab.openAdvancedSettings()
+		await expect(sharingTab.checkbox('Set password')).not.toBeChecked()
+	})
 })
 
 test.describe('files_sharing: Email share editor', () => {