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

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -45,6 +45,7 @@
 use OCP\IL10N;
 use OCP\IPreview;
 use OCP\IRequest;
+use OCP\ITagManager;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
@@ -280,7 +281,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();
@@ -290,7 +291,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();
@@ -300,7 +301,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) {
 			}
 		}
 
@@ -470,7 +471,7 @@ public function getShare(string $id, bool $include_tags = false): DataResponse {
 				$share = $this->formatShare($share);
 
 				if ($include_tags) {
-					$share = Helper::populateTags([$share], \OC::$server->getTagManager());
+					$share = Helper::populateTags([$share], \OCP\Server::get(ITagManager::class));
 				} else {
 					$share = [$share];
 				}
@@ -637,7 +638,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 {
@@ -751,7 +754,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'));
 			}
 
@@ -766,19 +769,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 {
@@ -839,7 +842,7 @@ private function getSharedWithMe($node, bool $includeTags): array {
 		}
 
 		if ($includeTags) {
-			$formatted = Helper::populateTags($formatted, \OC::$server->getTagManager());
+			$formatted = Helper::populateTags($formatted, \OCP\Server::get(ITagManager::class));
 		}
 
 		return $formatted;
@@ -1093,7 +1096,7 @@ private function getFormattedShares(
 
 		if ($includeTags) {
 			$formatted =
-				Helper::populateTags($formatted, \OC::$server->getTagManager());
+				Helper::populateTags($formatted, \OCP\Server::get(ITagManager::class));
 		}
 
 		return $formatted;
@@ -1522,23 +1525,23 @@ protected function canAccessShare(IShare $share, bool $checkGroups = true): bool
 		if ($share->getShareType() === IShare::TYPE_ROOM) {
 			try {
 				return $this->getRoomShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
 
 		if ($share->getShareType() === IShare::TYPE_DECK) {
 			try {
 				return $this->getDeckShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
 
 		if ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
 			try {
 				return $this->getSciencemeshShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
@@ -1656,23 +1659,23 @@ protected function canDeleteShareFromSelf(IShare $share): bool {
 		if ($share->getShareType() === IShare::TYPE_ROOM) {
 			try {
 				return $this->getRoomShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
 
 		if ($share->getShareType() === IShare::TYPE_DECK) {
 			try {
 				return $this->getDeckShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
 
 		if ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
 			try {
 				return $this->getSciencemeshShareHelper()->canAccessShare($share, $this->userId);
-			} catch (QueryException $e) {
+			} catch (ContainerExceptionInterface $e) {
 				return false;
 			}
 		}
@@ -1798,10 +1801,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')) {
@@ -1815,10 +1818,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')) {
@@ -1832,10 +1835,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')) {
@@ -1968,7 +1971,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);
@@ -1984,7 +1987,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

@@ -86,12 +86,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"
@@ -101,7 +102,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>
@@ -597,6 +598,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: {
@@ -715,7 +719,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.
@@ -871,9 +875,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/components/SharingInput.vue

@@ -149,7 +149,10 @@ export default {
 	},
 
 	mounted() {
-		this.getRecommendations()
+		if (!this.isExternal) {
+			// We can only recommend users, groups etc for internal shares
+			this.getRecommendations()
+		}
 	},
 
 	methods: {

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

@@ -115,8 +115,8 @@
 						:helper-text="t('files_sharing', 'Set the public share link token to something easy to remember or generate a new token. It is not recommended to use a guessable token for shares which contain sensitive information.')"
 						show-trailing-button
 						:trailing-button-label="loadingToken ? t('files_sharing', 'Generating…') : t('files_sharing', 'Generate new token')"
-						@trailing-button-click="generateNewToken"
-						:value.sync="share.token">
+						:value.sync="share.token"
+						@trailing-button-click="generateNewToken">
 						<template #trailing-button-icon>
 							<NcLoadingIcon v-if="loadingToken" />
 							<Refresh v-else :size="20" />
@@ -556,9 +556,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) {
@@ -972,6 +969,7 @@ export default {
 				this.$emit('add:share', this.share)
 			} else {
 				this.$emit('update:share', this.share)
+				emit('update:share', this.share)
 				this.queueUpdate(...permissionsAndAttributes)
 			}