From d84be6b946f9dd5bd59ed220f3ee2e7830ed6e0a Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 4 Feb 2025 22:40:00 +0100 Subject: [PATCH 1/5] refactor: Remove some deprecated containers and exceptions Signed-off-by: nfebe --- .../lib/Controller/ShareAPIController.php | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index d959fad7560eb..b630bd602ce01 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -307,7 +307,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(); @@ -317,7 +317,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(); @@ -327,7 +327,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) { } } @@ -663,7 +663,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 { @@ -756,7 +758,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')); } @@ -771,19 +773,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 { @@ -1770,10 +1772,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')) { @@ -1787,10 +1789,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')) { @@ -1804,10 +1806,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')) { @@ -1940,7 +1942,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); @@ -1956,7 +1958,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no return true; } return false; - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { return false; } } From 2029f309704fb27633c03a3ec70cd0e02ab8e480 Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 4 Feb 2025 22:40:00 +0100 Subject: [PATCH 2/5] fix: Show default expiration date before create link share Since `ShareEntryLink` component is used to both create and display/list the share links, we should only set default expiration date on `share.expireDate` when we know is a new share. Otherwise, we overidding data from the backend. Signed-off-by: nfebe --- .../src/components/SharingEntryLink.vue | 16 ++++++++++++---- apps/files_sharing/src/mixins/SharesMixin.js | 16 +++++----------- .../src/views/SharingDetailsTab.vue | 3 --- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue index 0563cc82d814c..f334f3d37bb8d 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -102,7 +102,7 @@ :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') }} @@ -117,7 +117,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)"> @@ -592,6 +592,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: { @@ -710,7 +713,7 @@ export default { path, shareType: ShareTypes.SHARE_TYPE_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. @@ -866,9 +869,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 diff --git a/apps/files_sharing/src/mixins/SharesMixin.js b/apps/files_sharing/src/mixins/SharesMixin.js index a0170cd4b7ff8..5b2dd66b1ba53 100644 --- a/apps/files_sharing/src/mixins/SharesMixin.js +++ b/apps/files_sharing/src/mixins/SharesMixin.js @@ -131,6 +131,9 @@ export default { monthFormat: 'MMM', } }, + isNewShare() { + return !this.share.id + }, isFolder() { return this.fileInfo.type === 'dir' }, @@ -231,17 +234,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 }, /** diff --git a/apps/files_sharing/src/views/SharingDetailsTab.vue b/apps/files_sharing/src/views/SharingDetailsTab.vue index 6a7a63e1f45f6..0253b29e0c3f0 100644 --- a/apps/files_sharing/src/views/SharingDetailsTab.vue +++ b/apps/files_sharing/src/views/SharingDetailsTab.vue @@ -521,9 +521,6 @@ export default { isGroupShare() { return this.share.type === this.SHARE_TYPES.SHARE_TYPE_GROUP }, - isNewShare() { - return !this.share.id - }, allowsFileDrop() { if (this.isFolder && this.config.isPublicUploadEnabled) { if (this.share.type === this.SHARE_TYPES.SHARE_TYPE_LINK || this.share.type === this.SHARE_TYPES.SHARE_TYPE_EMAIL) { From f7cc372444ce98d76937374829166d7881a402c4 Mon Sep 17 00:00:00 2001 From: nfebe Date: Wed, 5 Feb 2025 16:40:08 +0100 Subject: [PATCH 3/5] test(files_sharing): Check that default expiration date is shown b4 create share Signed-off-by: nfebe --- .../src/components/SharingEntryLink.vue | 1 + .../public-share/setup-public-share.ts | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue index f334f3d37bb8d..da47af4b08b91 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -108,6 +108,7 @@