From 08426e0a8fa9913b26a9da39f69e68fef19e62f8 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 dc9aa2d998b..b78ea32f3a2 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -277,7 +277,7 @@ class ShareAPIController extends OCSController { /** @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 @@ class ShareAPIController extends OCSController { /** @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 @@ class ShareAPIController extends OCSController { /** @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 @@ class ShareAPIController extends OCSController { $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 @@ class ShareAPIController extends OCSController { $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 @@ class ShareAPIController extends OCSController { } 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 @@ class ShareAPIController extends OCSController { * 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 @@ class ShareAPIController extends OCSController { * 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 @@ class ShareAPIController extends OCSController { * 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 @@ class ShareAPIController extends OCSController { 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 @@ class ShareAPIController extends OCSController { return true; } return false; - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { return false; } } From b4cc4efaf291fd808d29b2121f2961f0dbaf2ad7 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 d4b909e65ee..a3b8de16e59 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -85,7 +85,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') }} @@ -100,7 +100,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)"> @@ -589,6 +589,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 +710,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 +866,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 ab84a6f0e19..a18f35378c3 100644 --- a/apps/files_sharing/src/mixins/SharesMixin.js +++ b/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 }, /** diff --git a/apps/files_sharing/src/views/SharingDetailsTab.vue b/apps/files_sharing/src/views/SharingDetailsTab.vue index 95d6d234264..a155f6ed2ae 100644 --- a/apps/files_sharing/src/views/SharingDetailsTab.vue +++ b/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) { From 23e70a574e2a2ee1adf6727f18baeb3fd1b5fef8 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 a3b8de16e59..e07aec73b08 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -91,6 +91,7 @@