Skip to content

Commit 2926055

Browse files
committed
fix review comments
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
1 parent 92160cb commit 2926055

7 files changed

Lines changed: 29 additions & 54 deletions

File tree

lib/Constants.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ class Constants {
3636
public const CONFIG_KEY_TYPES = [
3737
self::CONFIG_KEY_ALLOWPERMITALL => 'bool',
3838
self::CONFIG_KEY_ALLOWPUBLICLINK => 'bool',
39+
self::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN => 'bool',
3940
self::CONFIG_KEY_ALLOWSHOWTOALL => 'bool',
4041
self::CONFIG_KEY_RESTRICTCREATION => 'bool',
4142
self::CONFIG_KEY_ALLOWCONFIRMATIONEMAIL => 'bool',
4243
self::CONFIG_KEY_CREATIONALLOWEDGROUPS => 'array',
4344
self::CONFIG_KEY_CONFIRMATIONEMAILRATELIMIT => 'int',
4445
];
4546

46-
public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 8;
47+
public const PUBLIC_SHARE_TOKEN_MIN_LENGTH = 1;
4748
public const PUBLIC_SHARE_TOKEN_MAX_LENGTH = 256;
4849
public const PUBLIC_SHARE_HASH_REQUIREMENT = '[a-zA-Z0-9]{' . self::PUBLIC_SHARE_TOKEN_MIN_LENGTH . ',' . self::PUBLIC_SHARE_TOKEN_MAX_LENGTH . '}';
4950

lib/Controller/ShareApiController.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar
213213
* @throws OCSBadRequestException Share doesn't belong to given Form
214214
* @throws OCSBadRequestException Invalid permission given
215215
* @throws OCSBadRequestException Invalid share token
216-
* @throws OCSBadRequestException Share hash exists, please retry
217216
* @throws OCSForbiddenException Custom public share tokens are not allowed
218217
* @throws OCSForbiddenException This form is not owned by the current user
219218
* @throws OCSForbiddenException Empty keyValuePairs, will not update
@@ -296,7 +295,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da
296295
$existingShare = $this->shareMapper->findPublicShareByHash($token);
297296
if ($existingShare->getId() !== $formShare->getId()) {
298297
$this->logger->debug('Share hash already exists.');
299-
throw new OCSBadRequestException('Share hash exists, please retry.');
298+
throw new OCSBadRequestException('Invalid share token');
300299
}
301300
} catch (DoesNotExistException $e) {
302301
// Just continue, this is what we expect to happen (share hash not existing yet).

lib/Service/ConfigService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public function getAllowPublicLink(): bool {
3939
return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWPUBLICLINK, true);
4040
}
4141
public function getAllowCustomPublicToken(): bool {
42-
return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, 'false'));
42+
return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWCUSTOMPUBLICTOKEN, false);
4343
}
4444
public function getAllowShowToAll(): bool {
4545
return $this->appConfig->getAppValueBool(Constants::CONFIG_KEY_ALLOWSHOWTOALL, true);

openapi.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5293,7 +5293,7 @@
52935293
}
52945294
},
52955295
"400": {
5296-
"description": "Share hash exists, please retry",
5296+
"description": "Invalid share token",
52975297
"content": {
52985298
"application/json": {
52995299
"schema": {

src/FormsSettings.vue

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,13 @@
7373
{{ t('forms', 'Allow sharing by link') }}
7474
</NcCheckboxRadioSwitch>
7575
<NcCheckboxRadioSwitch
76-
ref="switchAllowCustomPublicShareTokens"
7776
v-model="appConfig.allowCustomPublicShareTokens"
77+
:loading="loading.allowCustomPublicShareTokens"
7878
type="switch"
7979
@update:modelValue="onAllowCustomPublicShareTokensChange">
8080
{{ t('forms', 'Allow custom public share tokens') }}
8181
</NcCheckboxRadioSwitch>
8282
<NcCheckboxRadioSwitch
83-
ref="switchAllowPermitAll"
8483
v-model="appConfig.allowPermitAll"
8584
:loading="loading.allowPermitAll"
8685
type="switch"
@@ -176,14 +175,13 @@ export default {
176175
async onAllowPublicLinkChange(newVal) {
177176
this.loading.allowPublicLink = true
178177
await this.saveAppConfig('allowPublicLink', newVal)
179-
el.loading = false
178+
this.loading.allowPublicLink = false
180179
},
181180
182181
async onAllowCustomPublicShareTokensChange(newVal) {
183-
const el = this.$refs.switchAllowCustomPublicShareTokens
184-
el.loading = true
182+
this.loading.allowCustomPublicShareTokens = true
185183
await this.saveAppConfig('allowCustomPublicShareTokens', newVal)
186-
el.loading = false
184+
this.loading.allowCustomPublicShareTokens = false
187185
},
188186
189187
async onAllowPermitAllChange(newVal) {

src/components/SidebarTabs/SharingSidebarTab.vue

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
"
9898
@click="updateShareToken(share)">
9999
<template #icon>
100-
<IconCheck :size="20" />
100+
<NcIconSvgWrapper :svg="IconCheck" />
101101
</template>
102102
{{ t('forms', 'Save token') }}
103103
</NcActionButton>
@@ -231,6 +231,7 @@
231231

232232
<script>
233233
import IconPlus from '@material-symbols/svg-400/outlined/add.svg?raw'
234+
import IconCheck from '@material-symbols/svg-400/outlined/check.svg?raw'
234235
import IconCodeBrackets from '@material-symbols/svg-400/outlined/code.svg?raw'
235236
import IconCopyAll from '@material-symbols/svg-400/outlined/copy_all.svg?raw'
236237
import IconDelete from '@material-symbols/svg-400/outlined/delete.svg?raw'
@@ -250,16 +251,6 @@ import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwit
250251
import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper'
251252
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
252253
import NcTextField from '@nextcloud/vue/components/NcTextField'
253-
import IconAccountMultiple from 'vue-material-design-icons/AccountMultipleOutline.vue'
254-
import IconCheck from 'vue-material-design-icons/Check.vue'
255-
import IconCodeBrackets from 'vue-material-design-icons/CodeBrackets.vue'
256-
import IconLinkVariant from 'vue-material-design-icons/Link.vue'
257-
import IconLinkBoxVariantOutline from 'vue-material-design-icons/LinkBoxOutline.vue'
258-
import IconPlus from 'vue-material-design-icons/Plus.vue'
259-
import IconQr from 'vue-material-design-icons/Qrcode.vue'
260-
import IconDelete from 'vue-material-design-icons/TrashCanOutline.vue'
261-
import FormsIcon from '../Icons/FormsIcon.vue'
262-
import IconCopyAll from '../Icons/IconCopyAll.vue'
263254
import QRDialog from '../QRDialog.vue'
264255
import SharingSearchDiv from './SharingSearchDiv.vue'
265256
import SharingShareDiv from './SharingShareDiv.vue'
@@ -273,16 +264,6 @@ import OcsResponse2Data from '../../utils/OcsResponse2Data.js'
273264
export default {
274265
components: {
275266
NcIconSvgWrapper,
276-
FormsIcon,
277-
IconAccountMultiple,
278-
IconCheck,
279-
IconCodeBrackets,
280-
IconCopyAll,
281-
IconDelete,
282-
IconLinkBoxVariantOutline,
283-
IconLinkVariant,
284-
IconPlus,
285-
IconQr,
286267
NcActions,
287268
NcActionButton,
288269
NcActionLink,
@@ -318,6 +299,7 @@ export default {
318299
setup() {
319300
return {
320301
FormsIcon,
302+
IconCheck,
321303
IconCopyAll,
322304
IconPlus,
323305
IconCodeBrackets,
@@ -339,21 +321,6 @@ export default {
339321
}
340322
},
341323
342-
watch: {
343-
publicLinkShares: {
344-
immediate: true,
345-
handler(shares) {
346-
const nextShareTokens = {}
347-
for (const share of shares) {
348-
nextShareTokens[share.id] =
349-
this.shareTokens[share.id] ?? share.shareWith
350-
}
351-
352-
this.shareTokens = nextShareTokens
353-
},
354-
},
355-
},
356-
357324
computed: {
358325
isCurrentUserOwner() {
359326
return getCurrentUser().uid === this.form.ownerId
@@ -383,6 +350,21 @@ export default {
383350
},
384351
},
385352
353+
watch: {
354+
publicLinkShares: {
355+
immediate: true,
356+
handler(shares) {
357+
const nextShareTokens = {}
358+
for (const share of shares) {
359+
nextShareTokens[share.id] =
360+
this.shareTokens[share.id] ?? share.shareWith
361+
}
362+
363+
this.shareTokens = nextShareTokens
364+
},
365+
},
366+
},
367+
386368
methods: {
387369
/**
388370
* Add share

tests/Unit/Controller/ConfigControllerTest.php

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,25 +61,21 @@ public static function dataUpdateAppConfig() {
6161
'booleanAllowPermitAll' => [
6262
'configKey' => 'allowPermitAll',
6363
'configValue' => true,
64-
'strConfig' => 'true'
6564
],
6665
'booleanAllowShowToAll' => [
6766
'configKey' => 'allowShowToAll',
6867
'configValue' => true,
69-
'strConfig' => 'true'
7068
],
71-
'booleanConfigAllowCustomPublicShareTokens' => [
69+
'booleanAllowCustomPublicShareTokens' => [
7270
'configKey' => 'allowCustomPublicShareTokens',
7371
'configValue' => true,
74-
'strConfig' => 'true'
7572
],
7673
'arrayCreationAllowedGroups' => [
7774
'configKey' => 'creationAllowedGroups',
7875
'configValue' => [
7976
'admin',
8077
'group1'
8178
],
82-
'strConfig' => '["admin","group1"]'
8379
]
8480
];
8581
}
@@ -88,9 +84,8 @@ public static function dataUpdateAppConfig() {
8884
*
8985
* @param string $configKey
9086
* @param mixed $configValue
91-
* @param string $strConfig The configValue as json-string
9287
*/
93-
public function testUpdateAppConfig(string $configKey, $configValue, string $strConfig) {
88+
public function testUpdateAppConfig(string $configKey, $configValue) {
9489
$this->logger->expects($this->once())
9590
->method('debug');
9691

0 commit comments

Comments
 (0)