From ba2a1ec72fddd609607788d67fb6ef0597a19998 Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 4 Feb 2025 22:40:00 +0100 Subject: [PATCH 1/6] refactor: Remove some deprecated containers and exceptions Signed-off-by: nfebe --- .../lib/Controller/ShareAPIController.php | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 9716e8f2772ec..f6fc584529b49 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/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,7 +1525,7 @@ 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; } } @@ -1530,7 +1533,7 @@ protected function canAccessShare(IShare $share, bool $checkGroups = true): bool if ($share->getShareType() === IShare::TYPE_DECK) { try { return $this->getDeckShareHelper()->canAccessShare($share, $this->userId); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { return false; } } @@ -1538,7 +1541,7 @@ protected function canAccessShare(IShare $share, bool $checkGroups = true): bool if ($share->getShareType() === IShare::TYPE_SCIENCEMESH) { try { return $this->getSciencemeshShareHelper()->canAccessShare($share, $this->userId); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { return false; } } @@ -1656,7 +1659,7 @@ 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; } } @@ -1664,7 +1667,7 @@ protected function canDeleteShareFromSelf(IShare $share): bool { if ($share->getShareType() === IShare::TYPE_DECK) { try { return $this->getDeckShareHelper()->canAccessShare($share, $this->userId); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { return false; } } @@ -1672,7 +1675,7 @@ protected function canDeleteShareFromSelf(IShare $share): bool { 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; } } From 84602522f768711b43073c8e4e745bd85d5fcf75 Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 4 Feb 2025 22:40:00 +0100 Subject: [PATCH 2/6] 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 | 7 ++----- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue index 28c72bfc98a90..3b75028c0838f 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -86,7 +86,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') }} @@ -101,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)"> @@ -597,6 +597,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 +718,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 +874,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 fdd8bbd72fdee..c809b650fdf0b 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 f50a533eeeb44..404a8fe25e9ed 100644 --- a/apps/files_sharing/src/views/SharingDetailsTab.vue +++ b/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">