Merge pull request #50694 from nextcloud/backport/50655/stable30

[stable30] enh: Fix display default expire date, add tests & tiny refactors

Author: AndyScherzinger

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -277,7 +277,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
 				/** @var array{share_with_displayname: string, share_with_link: string, share_with?: string, token?: string} $roomShare */
 				$roomShare = $this->getRoomShareHelper()->formatShare($share);
 				$result = array_merge($result, $roomShare);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 			}
 		} elseif ($share->getShareType() === IShare::TYPE_DECK) {
 			$result['share_with'] = $share->getSharedWith();
@@ -287,7 +287,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
 				/** @var array{share_with: string, share_with_displayname: string, share_with_link: string} $deckShare */
 				$deckShare = $this->getDeckShareHelper()->formatShare($share);
 				$result = array_merge($result, $deckShare);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 			}
 		} elseif ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
 			$result['share_with'] = $share->getSharedWith();
@@ -297,7 +297,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
 				/** @var array{share_with: string, share_with_displayname: string, token: string} $scienceMeshShare */
 				$scienceMeshShare = $this->getSciencemeshShareHelper()->formatShare($share);
 				$result = array_merge($result, $scienceMeshShare);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 			}
 		}
 
@@ -632,7 +632,9 @@ public function createShare(
 			$share = $this->setShareAttributes($share, $attributes);
 		}
 
-		// Expire date
+		// Expire date checks
+		// Normally, null means no expiration date but we still set the default for backwards compatibility
+		// If the client sends an empty string, we set noExpirationDate to true
 		if ($expireDate !== null) {
 			if ($expireDate !== '') {
 				try {
@@ -746,7 +748,7 @@ public function createShare(
 			$share->setSharedWith($shareWith);
 			$share->setPermissions($permissions);
 		} elseif ($shareType === IShare::TYPE_CIRCLE) {
-			if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
+			if (!\OCP\Server::get(IAppManager::class)->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
 				throw new OCSNotFoundException($this->l->t('You cannot share to a Team if the app is not enabled'));
 			}
 
@@ -761,19 +763,19 @@ public function createShare(
 		} elseif ($shareType === IShare::TYPE_ROOM) {
 			try {
 				$this->getRoomShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
 			}
 		} elseif ($shareType === IShare::TYPE_DECK) {
 			try {
 				$this->getDeckShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
 			}
 		} elseif ($shareType === IShare::TYPE_SCIENCEMESH) {
 			try {
 				$this->getSciencemeshShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support ScienceMesh shares', [$node->getPath()]));
 			}
 		} else {
@@ -1765,10 +1767,10 @@ public function cleanup() {
 	 * Returns the helper of ShareAPIController for room shares.
 	 *
 	 * If the Talk application is not enabled or the helper is not available
-	 * a QueryException is thrown instead.
+	 * a ContainerExceptionInterface is thrown instead.
 	 *
 	 * @return \OCA\Talk\Share\Helper\ShareAPIController
-	 * @throws QueryException
+	 * @throws ContainerExceptionInterface
 	 */
 	private function getRoomShareHelper() {
 		if (!$this->appManager->isEnabledForUser('spreed')) {
@@ -1782,10 +1784,10 @@ private function getRoomShareHelper() {
 	 * Returns the helper of ShareAPIHelper for deck shares.
 	 *
 	 * If the Deck application is not enabled or the helper is not available
-	 * a QueryException is thrown instead.
+	 * a ContainerExceptionInterface is thrown instead.
 	 *
 	 * @return \OCA\Deck\Sharing\ShareAPIHelper
-	 * @throws QueryException
+	 * @throws ContainerExceptionInterface
 	 */
 	private function getDeckShareHelper() {
 		if (!$this->appManager->isEnabledForUser('deck')) {
@@ -1799,10 +1801,10 @@ private function getDeckShareHelper() {
 	 * Returns the helper of ShareAPIHelper for sciencemesh shares.
 	 *
 	 * If the sciencemesh application is not enabled or the helper is not available
-	 * a QueryException is thrown instead.
+	 * a ContainerExceptionInterface is thrown instead.
 	 *
 	 * @return \OCA\Deck\Sharing\ShareAPIHelper
-	 * @throws QueryException
+	 * @throws ContainerExceptionInterface
 	 */
 	private function getSciencemeshShareHelper() {
 		if (!$this->appManager->isEnabledForUser('sciencemesh')) {
@@ -1935,7 +1937,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no
 			return true;
 		}
 
-		if ($share->getShareType() === IShare::TYPE_CIRCLE && \OC::$server->getAppManager()->isEnabledForUser('circles')
+		if ($share->getShareType() === IShare::TYPE_CIRCLE && \OCP\Server::get(IAppManager::class)->isEnabledForUser('circles')
 			&& class_exists('\OCA\Circles\Api\v1\Circles')) {
 			$hasCircleId = (str_ends_with($share->getSharedWith(), ']'));
 			$shareWithStart = ($hasCircleId ? strrpos($share->getSharedWith(), '[') + 1 : 0);
@@ -1951,7 +1953,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no
 					return true;
 				}
 				return false;
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -85,12 +85,13 @@
 				:checked.sync="defaultExpirationDateEnabled"
 				:disabled="pendingEnforcedExpirationDate || saving"
 				class="share-link-expiration-date-checkbox"
-				@change="onDefaultExpirationDateEnabledChange">
+				@change="onExpirationDateToggleChange">
 				{{ config.isDefaultExpireDateEnforced ? t('files_sharing', 'Enable link expiration (enforced)') : t('files_sharing', 'Enable link expiration') }}
 			</NcActionCheckbox>
 
 			<!-- expiration date -->
 			<NcActionInput v-if="(pendingDefaultExpirationDate || pendingEnforcedExpirationDate) && defaultExpirationDateEnabled"
+				data-cy-files-sharing-expiration-date-input
 				class="share-link-expire-date"
 				:label="pendingEnforcedExpirationDate ? t('files_sharing', 'Enter expiration date (enforced)') : t('files_sharing', 'Enter expiration date')"
 				:disabled="saving"
@@ -100,7 +101,7 @@
 				type="date"
 				:min="dateTomorrow"
 				:max="maxExpirationDateEnforced"
-				@input="onExpirationChange /* let's not submit when picked, the user might want to still edit or copy the password */">
+				@change="expirationDateChanged($event)">
 				<template #icon>
 					<IconCalendarBlank :size="20" />
 				</template>
@@ -589,6 +590,9 @@ export default {
 	},
 	mounted() {
 		this.defaultExpirationDateEnabled = this.config.defaultExpirationDate instanceof Date
+		if (this.share && this.isNewShare) {
+			this.share.expireDate = this.defaultExpirationDateEnabled ? this.formatDateToString(this.config.defaultExpirationDate) : ''
+		}
 	},
 
 	methods: {
@@ -707,7 +711,7 @@ export default {
 					path,
 					shareType: ShareType.Link,
 					password: share.password,
-					expireDate: share.expireDate,
+					expireDate: share.expireDate ?? '',
 					attributes: JSON.stringify(this.fileInfo.shareAttributes),
 					// we do not allow setting the publicUpload
 					// before the share creation.
@@ -863,9 +867,14 @@ export default {
 			this.onPasswordSubmit()
 			this.onNoteSubmit()
 		},
-		onDefaultExpirationDateEnabledChange(enabled) {
+		onExpirationDateToggleChange(enabled) {
 			this.share.expireDate = enabled ? this.formatDateToString(this.config.defaultExpirationDate) : ''
 		},
+		expirationDateChanged(event) {
+			const date = event.target.value
+			this.onExpirationChange(date)
+			this.defaultExpirationDateEnabled = !!date
+		},
 
 		/**
 		 * Cancel the share creation

apps/files_sharing/src/components/SharingEntryQuickShareSelect.vue

@@ -29,6 +29,7 @@
 
 <script>
 import { ShareType } from '@nextcloud/sharing'
+import { subscribe, unsubscribe } from '@nextcloud/event-bus'
 import DropdownIcon from 'vue-material-design-icons/TriangleSmallDown.vue'
 import SharesMixin from '../mixins/SharesMixin.js'
 import ShareDetails from '../mixins/ShareDetails.js'
@@ -145,7 +146,17 @@ export default {
 	created() {
 		this.selectedOption = this.preSelectedOption
 	},
-
+	mounted() {
+		subscribe('update:share', (share) => {
+			if (share.id === this.share.id) {
+				this.share.permissions = share.permissions
+				this.selectedOption = this.preSelectedOption
+			}
+		})
+	},
+	unmounted() {
+		unsubscribe('update:share')
+	},
 	methods: {
 		selectOption(optionLabel) {
 			this.selectedOption = optionLabel

apps/files_sharing/src/mixins/SharesMixin.js

@@ -110,6 +110,9 @@ export default {
 				monthFormat: 'MMM',
 			}
 		},
+		isNewShare() {
+			return !this.share.id
+		},
 		isFolder() {
 			return this.fileInfo.type === 'dir'
 		},
@@ -210,17 +213,8 @@ export default {
 		 * @param {Date} date
 		 */
 		onExpirationChange(date) {
-			this.share.expireDate = this.formatDateToString(new Date(date))
-		},
-
-		/**
-		 * Uncheck expire date
-		 * We need this method because @update:checked
-		 * is ran simultaneously as @uncheck, so
-		 * so we cannot ensure data is up-to-date
-		 */
-		onExpirationDisable() {
-			this.share.expireDate = ''
+			const formattedDate = date ? this.formatDateToString(new Date(date)) : ''
+			this.share.expireDate = formattedDate
 		},
 
 		/**

apps/files_sharing/src/views/SharingDetailsTab.vue

@@ -526,9 +526,6 @@ export default {
 		isGroupShare() {
 			return this.share.type === ShareType.Group
 		},
-		isNewShare() {
-			return !this.share.id
-		},
 		allowsFileDrop() {
 			if (this.isFolder && this.config.isPublicUploadEnabled) {
 				if (this.share.type === ShareType.Link || this.share.type === ShareType.Email) {
@@ -921,6 +918,7 @@ export default {
 				this.$emit('add:share', this.share)
 			} else {
 				this.$emit('update:share', this.share)
+				emit('update:share', this.share)
 				this.queueUpdate(...permissionsAndAttributes)
 			}
 

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

@@ -51,9 +51,9 @@ export function setupData(user: User, shareName: string): void {
  */
 function checkPasswordState(enforced: boolean, alwaysAskForPassword: boolean) {
 	if (enforced) {
-	  cy.contains('Password protection (enforced)').should('exist')
+		cy.contains('Password protection (enforced)').should('exist')
 	} else if (alwaysAskForPassword) {
-	  cy.contains('Password protection').should('exist')
+		cy.contains('Password protection').should('exist')
 	}
 	cy.contains('Enter a password')
 		.should('exist')
@@ -68,13 +68,22 @@ function checkPasswordState(enforced: boolean, alwaysAskForPassword: boolean) {
  */
 function checkExpirationDateState(enforced: boolean, hasDefault: boolean) {
 	if (enforced) {
-	  cy.contains('Enable link expiration (enforced)').should('exist')
+		cy.contains('Enable link expiration (enforced)').should('exist')
 	} else if (hasDefault) {
-	  cy.contains('Enable link expiration').should('exist')
+		cy.contains('Enable link expiration').should('exist')
 	}
 	cy.contains('Enter expiration date')
 		.should('exist')
 		.and('not.be.disabled')
+	cy.get('input[data-cy-files-sharing-expiration-date-input]').should('exist')
+	cy.get('input[data-cy-files-sharing-expiration-date-input]')
+		.invoke('val')
+		.then((val) => {
+			const expectedDate = new Date()
+			expectedDate.setDate(expectedDate.getDate() + 2)
+			expect(new Date(val).toDateString()).to.eq(expectedDate.toDateString())
+		})
+
 }
 
 /**
@@ -90,7 +99,7 @@ export function createShare(context: ShareContext, shareName: string, options: S
 
 	cy.intercept('POST', '**/ocs/v2.php/apps/files_sharing/api/v1/shares').as('createShare')
 	cy.findByRole('button', { name: 'Create a new share link' }).click()
-	  // Conduct optional checks based on the provided options
+	// Conduct optional checks based on the provided options
 	if (options) {
 		cy.get('.sharing-entry__actions').should('be.visible') // Wait for the dialog to open
 		checkPasswordState(options.enforcePassword ?? false, options.alwaysAskForPassword ?? false)

dist/5661-5661.js

@@ -0,0 +1 @@
+5661-5661.js.license