Skip to content

Commit 7af66ea

Browse files
nfebenfebe
authored
Merge pull request #50655 from nextcloud/fix/share-sidebar-bugs
enh: Fix display default expire date, add tests & tiny refactors
2 parents 9c3ef8e + a544252 commit 7af66ea

16 files changed

+86
-59
lines changed

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
use OCP\IL10N;
4646
use OCP\IPreview;
4747
use OCP\IRequest;
48+
use OCP\ITagManager;
4849
use OCP\IURLGenerator;
4950
use OCP\IUserManager;
5051
use OCP\Lock\ILockingProvider;
@@ -280,7 +281,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
280281
/** @var array{share_with_displayname: string, share_with_link: string, share_with?: string, token?: string} $roomShare */
281282
$roomShare = $this->getRoomShareHelper()->formatShare($share);
282283
$result = array_merge($result, $roomShare);
283-
} catch (QueryException $e) {
284+
} catch (ContainerExceptionInterface $e) {
284285
}
285286
} elseif ($share->getShareType() === IShare::TYPE_DECK) {
286287
$result['share_with'] = $share->getSharedWith();
@@ -290,7 +291,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
290291
/** @var array{share_with: string, share_with_displayname: string, share_with_link: string} $deckShare */
291292
$deckShare = $this->getDeckShareHelper()->formatShare($share);
292293
$result = array_merge($result, $deckShare);
293-
} catch (QueryException $e) {
294+
} catch (ContainerExceptionInterface $e) {
294295
}
295296
} elseif ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
296297
$result['share_with'] = $share->getSharedWith();
@@ -300,7 +301,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
300301
/** @var array{share_with: string, share_with_displayname: string, token: string} $scienceMeshShare */
301302
$scienceMeshShare = $this->getSciencemeshShareHelper()->formatShare($share);
302303
$result = array_merge($result, $scienceMeshShare);
303-
} catch (QueryException $e) {
304+
} catch (ContainerExceptionInterface $e) {
304305
}
305306
}
306307

@@ -470,7 +471,7 @@ public function getShare(string $id, bool $include_tags = false): DataResponse {
470471
$share = $this->formatShare($share);
471472

472473
if ($include_tags) {
473-
$share = Helper::populateTags([$share], \OC::$server->getTagManager());
474+
$share = Helper::populateTags([$share], \OCP\Server::get(ITagManager::class));
474475
} else {
475476
$share = [$share];
476477
}
@@ -637,7 +638,9 @@ public function createShare(
637638
$share = $this->setShareAttributes($share, $attributes);
638639
}
639640

640-
// Expire date
641+
// Expire date checks
642+
// Normally, null means no expiration date but we still set the default for backwards compatibility
643+
// If the client sends an empty string, we set noExpirationDate to true
641644
if ($expireDate !== null) {
642645
if ($expireDate !== '') {
643646
try {
@@ -751,7 +754,7 @@ public function createShare(
751754
$share->setSharedWith($shareWith);
752755
$share->setPermissions($permissions);
753756
} elseif ($shareType === IShare::TYPE_CIRCLE) {
754-
if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
757+
if (!\OCP\Server::get(IAppManager::class)->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
755758
throw new OCSNotFoundException($this->l->t('You cannot share to a Team if the app is not enabled'));
756759
}
757760

@@ -766,19 +769,19 @@ public function createShare(
766769
} elseif ($shareType === IShare::TYPE_ROOM) {
767770
try {
768771
$this->getRoomShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
769-
} catch (QueryException $e) {
772+
} catch (ContainerExceptionInterface $e) {
770773
throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
771774
}
772775
} elseif ($shareType === IShare::TYPE_DECK) {
773776
try {
774777
$this->getDeckShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
775-
} catch (QueryException $e) {
778+
} catch (ContainerExceptionInterface $e) {
776779
throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
777780
}
778781
} elseif ($shareType === IShare::TYPE_SCIENCEMESH) {
779782
try {
780783
$this->getSciencemeshShareHelper()->createShare($share, $shareWith, $permissions, $expireDate ?? '');
781-
} catch (QueryException $e) {
784+
} catch (ContainerExceptionInterface $e) {
782785
throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support ScienceMesh shares', [$node->getPath()]));
783786
}
784787
} else {
@@ -839,7 +842,7 @@ private function getSharedWithMe($node, bool $includeTags): array {
839842
}
840843

841844
if ($includeTags) {
842-
$formatted = Helper::populateTags($formatted, \OC::$server->getTagManager());
845+
$formatted = Helper::populateTags($formatted, \OCP\Server::get(ITagManager::class));
843846
}
844847

845848
return $formatted;
@@ -1093,7 +1096,7 @@ private function getFormattedShares(
10931096

10941097
if ($includeTags) {
10951098
$formatted =
1096-
Helper::populateTags($formatted, \OC::$server->getTagManager());
1099+
Helper::populateTags($formatted, \OCP\Server::get(ITagManager::class));
10971100
}
10981101

10991102
return $formatted;
@@ -1522,23 +1525,23 @@ protected function canAccessShare(IShare $share, bool $checkGroups = true): bool
15221525
if ($share->getShareType() === IShare::TYPE_ROOM) {
15231526
try {
15241527
return $this->getRoomShareHelper()->canAccessShare($share, $this->userId);
1525-
} catch (QueryException $e) {
1528+
} catch (ContainerExceptionInterface $e) {
15261529
return false;
15271530
}
15281531
}
15291532

15301533
if ($share->getShareType() === IShare::TYPE_DECK) {
15311534
try {
15321535
return $this->getDeckShareHelper()->canAccessShare($share, $this->userId);
1533-
} catch (QueryException $e) {
1536+
} catch (ContainerExceptionInterface $e) {
15341537
return false;
15351538
}
15361539
}
15371540

15381541
if ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
15391542
try {
15401543
return $this->getSciencemeshShareHelper()->canAccessShare($share, $this->userId);
1541-
} catch (QueryException $e) {
1544+
} catch (ContainerExceptionInterface $e) {
15421545
return false;
15431546
}
15441547
}
@@ -1656,23 +1659,23 @@ protected function canDeleteShareFromSelf(IShare $share): bool {
16561659
if ($share->getShareType() === IShare::TYPE_ROOM) {
16571660
try {
16581661
return $this->getRoomShareHelper()->canAccessShare($share, $this->userId);
1659-
} catch (QueryException $e) {
1662+
} catch (ContainerExceptionInterface $e) {
16601663
return false;
16611664
}
16621665
}
16631666

16641667
if ($share->getShareType() === IShare::TYPE_DECK) {
16651668
try {
16661669
return $this->getDeckShareHelper()->canAccessShare($share, $this->userId);
1667-
} catch (QueryException $e) {
1670+
} catch (ContainerExceptionInterface $e) {
16681671
return false;
16691672
}
16701673
}
16711674

16721675
if ($share->getShareType() === IShare::TYPE_SCIENCEMESH) {
16731676
try {
16741677
return $this->getSciencemeshShareHelper()->canAccessShare($share, $this->userId);
1675-
} catch (QueryException $e) {
1678+
} catch (ContainerExceptionInterface $e) {
16761679
return false;
16771680
}
16781681
}
@@ -1798,10 +1801,10 @@ public function cleanup() {
17981801
* Returns the helper of ShareAPIController for room shares.
17991802
*
18001803
* If the Talk application is not enabled or the helper is not available
1801-
* a QueryException is thrown instead.
1804+
* a ContainerExceptionInterface is thrown instead.
18021805
*
18031806
* @return \OCA\Talk\Share\Helper\ShareAPIController
1804-
* @throws QueryException
1807+
* @throws ContainerExceptionInterface
18051808
*/
18061809
private function getRoomShareHelper() {
18071810
if (!$this->appManager->isEnabledForUser('spreed')) {
@@ -1815,10 +1818,10 @@ private function getRoomShareHelper() {
18151818
* Returns the helper of ShareAPIHelper for deck shares.
18161819
*
18171820
* If the Deck application is not enabled or the helper is not available
1818-
* a QueryException is thrown instead.
1821+
* a ContainerExceptionInterface is thrown instead.
18191822
*
18201823
* @return \OCA\Deck\Sharing\ShareAPIHelper
1821-
* @throws QueryException
1824+
* @throws ContainerExceptionInterface
18221825
*/
18231826
private function getDeckShareHelper() {
18241827
if (!$this->appManager->isEnabledForUser('deck')) {
@@ -1832,10 +1835,10 @@ private function getDeckShareHelper() {
18321835
* Returns the helper of ShareAPIHelper for sciencemesh shares.
18331836
*
18341837
* If the sciencemesh application is not enabled or the helper is not available
1835-
* a QueryException is thrown instead.
1838+
* a ContainerExceptionInterface is thrown instead.
18361839
*
18371840
* @return \OCA\Deck\Sharing\ShareAPIHelper
1838-
* @throws QueryException
1841+
* @throws ContainerExceptionInterface
18391842
*/
18401843
private function getSciencemeshShareHelper() {
18411844
if (!$this->appManager->isEnabledForUser('sciencemesh')) {
@@ -1968,7 +1971,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no
19681971
return true;
19691972
}
19701973

1971-
if ($share->getShareType() === IShare::TYPE_CIRCLE && \OC::$server->getAppManager()->isEnabledForUser('circles')
1974+
if ($share->getShareType() === IShare::TYPE_CIRCLE && \OCP\Server::get(IAppManager::class)->isEnabledForUser('circles')
19721975
&& class_exists('\OCA\Circles\Api\v1\Circles')) {
19731976
$hasCircleId = (str_ends_with($share->getSharedWith(), ']'));
19741977
$shareWithStart = ($hasCircleId ? strrpos($share->getSharedWith(), '[') + 1 : 0);
@@ -1984,7 +1987,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no
19841987
return true;
19851988
}
19861989
return false;
1987-
} catch (QueryException $e) {
1990+
} catch (ContainerExceptionInterface $e) {
19881991
return false;
19891992
}
19901993
}

‎apps/files_sharing/src/components/SharingEntryLink.vue‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,13 @@
8686
:checked.sync="defaultExpirationDateEnabled"
8787
:disabled="pendingEnforcedExpirationDate || saving"
8888
class="share-link-expiration-date-checkbox"
89-
@change="onDefaultExpirationDateEnabledChange">
89+
@change="onExpirationDateToggleChange">
9090
{{ config.isDefaultExpireDateEnforced ? t('files_sharing', 'Enable link expiration (enforced)') : t('files_sharing', 'Enable link expiration') }}
9191
</NcActionCheckbox>
9292

9393
<!-- expiration date -->
9494
<NcActionInput v-if="(pendingDefaultExpirationDate || pendingEnforcedExpirationDate) && defaultExpirationDateEnabled"
95+
data-cy-files-sharing-expiration-date-input
9596
class="share-link-expire-date"
9697
:label="pendingEnforcedExpirationDate ? t('files_sharing', 'Enter expiration date (enforced)') : t('files_sharing', 'Enter expiration date')"
9798
:disabled="saving"
@@ -101,7 +102,7 @@
101102
type="date"
102103
:min="dateTomorrow"
103104
:max="maxExpirationDateEnforced"
104-
@input="onExpirationChange /* let's not submit when picked, the user might want to still edit or copy the password */">
105+
@change="expirationDateChanged($event)">
105106
<template #icon>
106107
<IconCalendarBlank :size="20" />
107108
</template>
@@ -597,6 +598,9 @@ export default {
597598
},
598599
mounted() {
599600
this.defaultExpirationDateEnabled = this.config.defaultExpirationDate instanceof Date
601+
if (this.share && this.isNewShare) {
602+
this.share.expireDate = this.defaultExpirationDateEnabled ? this.formatDateToString(this.config.defaultExpirationDate) : ''
603+
}
600604
},
601605
602606
methods: {
@@ -715,7 +719,7 @@ export default {
715719
path,
716720
shareType: ShareType.Link,
717721
password: share.password,
718-
expireDate: share.expireDate,
722+
expireDate: share.expireDate ?? '',
719723
attributes: JSON.stringify(this.fileInfo.shareAttributes),
720724
// we do not allow setting the publicUpload
721725
// before the share creation.
@@ -871,9 +875,14 @@ export default {
871875
this.onPasswordSubmit()
872876
this.onNoteSubmit()
873877
},
874-
onDefaultExpirationDateEnabledChange(enabled) {
878+
onExpirationDateToggleChange(enabled) {
875879
this.share.expireDate = enabled ? this.formatDateToString(this.config.defaultExpirationDate) : ''
876880
},
881+
expirationDateChanged(event) {
882+
const date = event.target.value
883+
this.onExpirationChange(date)
884+
this.defaultExpirationDateEnabled = !!date
885+
},
877886
878887
/**
879888
* Cancel the share creation

‎apps/files_sharing/src/components/SharingEntryQuickShareSelect.vue‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
<script>
3131
import { ShareType } from '@nextcloud/sharing'
32+
import { subscribe, unsubscribe } from '@nextcloud/event-bus'
3233
import DropdownIcon from 'vue-material-design-icons/TriangleSmallDown.vue'
3334
import SharesMixin from '../mixins/SharesMixin.js'
3435
import ShareDetails from '../mixins/ShareDetails.js'
@@ -145,7 +146,17 @@ export default {
145146
created() {
146147
this.selectedOption = this.preSelectedOption
147148
},
148-
149+
mounted() {
150+
subscribe('update:share', (share) => {
151+
if (share.id === this.share.id) {
152+
this.share.permissions = share.permissions
153+
this.selectedOption = this.preSelectedOption
154+
}
155+
})
156+
},
157+
unmounted() {
158+
unsubscribe('update:share')
159+
},
149160
methods: {
150161
selectOption(optionLabel) {
151162
this.selectedOption = optionLabel

‎apps/files_sharing/src/components/SharingInput.vue‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ export default {
149149
},
150150
151151
mounted() {
152-
this.getRecommendations()
152+
if (!this.isExternal) {
153+
// We can only recommend users, groups etc for internal shares
154+
this.getRecommendations()
155+
}
153156
},
154157
155158
methods: {

‎apps/files_sharing/src/mixins/SharesMixin.js‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ export default {
110110
monthFormat: 'MMM',
111111
}
112112
},
113+
isNewShare() {
114+
return !this.share.id
115+
},
113116
isFolder() {
114117
return this.fileInfo.type === 'dir'
115118
},
@@ -210,17 +213,8 @@ export default {
210213
* @param {Date} date
211214
*/
212215
onExpirationChange(date) {
213-
this.share.expireDate = this.formatDateToString(new Date(date))
214-
},
215-
216-
/**
217-
* Uncheck expire date
218-
* We need this method because @update:checked
219-
* is ran simultaneously as @uncheck, so
220-
* so we cannot ensure data is up-to-date
221-
*/
222-
onExpirationDisable() {
223-
this.share.expireDate = ''
216+
const formattedDate = date ? this.formatDateToString(new Date(date)) : ''
217+
this.share.expireDate = formattedDate
224218
},
225219

226220
/**

‎apps/files_sharing/src/views/SharingDetailsTab.vue‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@
115115
: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.')"
116116
show-trailing-button
117117
:trailing-button-label="loadingToken ? t('files_sharing', 'Generating…') : t('files_sharing', 'Generate new token')"
118-
@trailing-button-click="generateNewToken"
119-
:value.sync="share.token">
118+
:value.sync="share.token"
119+
@trailing-button-click="generateNewToken">
120120
<template #trailing-button-icon>
121121
<NcLoadingIcon v-if="loadingToken" />
122122
<Refresh v-else :size="20" />
@@ -556,9 +556,6 @@ export default {
556556
isGroupShare() {
557557
return this.share.type === ShareType.Group
558558
},
559-
isNewShare() {
560-
return !this.share.id
561-
},
562559
allowsFileDrop() {
563560
if (this.isFolder && this.config.isPublicUploadEnabled) {
564561
if (this.share.type === ShareType.Link || this.share.type === ShareType.Email) {
@@ -972,6 +969,7 @@ export default {
972969
this.$emit('add:share', this.share)
973970
} else {
974971
this.$emit('update:share', this.share)
972+
emit('update:share', this.share)
975973
this.queueUpdate(...permissionsAndAttributes)
976974
}
977975

0 commit comments

Comments
 (0)