Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Make show forms to all users in sidebar an admin setting #1906

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/Constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ class Constants {
* Used AppConfig Keys
*/
public const CONFIG_KEY_ALLOWPERMITALL = 'allowPermitAll';
public const CONFIG_KEY_ALLOWSHOWTOALL = 'allowShowToAll';
public const CONFIG_KEY_ALLOWPUBLICLINK = 'allowPublicLink';
public const CONFIG_KEY_CREATIONALLOWEDGROUPS = 'creationAllowedGroups';
public const CONFIG_KEY_RESTRICTCREATION = 'restrictCreation';
public const CONFIG_KEYS = [
self::CONFIG_KEY_ALLOWPERMITALL,
self::CONFIG_KEY_ALLOWSHOWTOALL,
self::CONFIG_KEY_ALLOWPUBLICLINK,
self::CONFIG_KEY_CREATIONALLOWEDGROUPS,
self::CONFIG_KEY_RESTRICTCREATION
Expand Down
4 changes: 4 additions & 0 deletions lib/Service/ConfigService.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public function __construct(string $appName,
public function getAllowPermitAll(): bool {
return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWPERMITALL, "true"));
}
public function getAllowShowToAll() : bool {
return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWSHOWTOALL, "true"));
}
public function getAllowPublicLink(): bool {
return json_decode($this->config->getAppValue($this->appName, Constants::CONFIG_KEY_ALLOWPUBLICLINK, "true"));
}
Expand All @@ -88,6 +91,7 @@ public function getRestrictCreation(): bool {
public function getAppConfig(): array {
return [
Constants::CONFIG_KEY_ALLOWPERMITALL => $this->getAllowPermitAll(),
Constants::CONFIG_KEY_ALLOWSHOWTOALL => $this->getAllowShowToAll(),
Constants::CONFIG_KEY_ALLOWPUBLICLINK => $this->getAllowPublicLink(),
Constants::CONFIG_KEY_CREATIONALLOWEDGROUPS => $this->getCreationAllowedGroups(),
Constants::CONFIG_KEY_RESTRICTCREATION => $this->getRestrictCreation(),
Expand Down
3 changes: 2 additions & 1 deletion lib/Service/FormsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ public function isSharedFormShown(Form $form): bool {
// Shown if permitall and showntoall are both set.
if ($access['permitAllUsers'] &&
$access['showToAllUsers'] &&
$this->configService->getAllowPermitAll()) {
$this->configService->getAllowPermitAll() &&
$this->configService->getAllowShowToAll()) {
return true;
}

Expand Down
13 changes: 12 additions & 1 deletion src/FormsSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@
@update:checked="onAllowPermitAllChange">
{{ t('forms', 'Allow sharing to all logged in accounts') }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch ref="switchAllowShowToAll"
:checked.sync="appConfig.allowShowToAll"
type="switch"
@update:checked="onAllowShowToAllChange">
{{ t('forms', 'Allow showing form to all logged in accounts on sidebar') }}
</NcCheckboxRadioSwitch>
</NcSettingsSection>
</div>
</template>
Expand Down Expand Up @@ -117,7 +123,12 @@ export default {
await this.saveAppConfig('allowPermitAll', newVal)
el.loading = false
},

async onAllowShowToAllChange(newVal) {
const el = this.$refs.switchAllowShowToAll
el.loading = true
await this.saveAppConfig('allowShowToAll', newVal)
el.loading = false
},
/**
* Save a key-value pair to the appConfig.
*
Expand Down
2 changes: 1 addition & 1 deletion src/components/SidebarTabs/SharingSidebarTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
type="switch"
@update:checked="onPermitAllUsersChange" />
</div>
<div v-if="form.access.permitAllUsers" class="share-div share-div--indent">
<div v-if="appConfig.allowShowToAll && form.access.permitAllUsers" class="share-div share-div--indent">
<div class="share-div__avatar">
<FormsIcon :size="16" />
</div>
Expand Down
11 changes: 8 additions & 3 deletions tests/Unit/Controller/ConfigControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function testGetAppConfig() {
'allow' => 'someConfig',
'permit' => 'all'
]);

$this->assertEquals(new DataResponse([
'allow' => 'someConfig',
'permit' => 'all'
Expand All @@ -91,6 +91,11 @@ public function dataUpdateAppConfig() {
'configValue' => true,
'strConfig' => "true"
],
'booleanConfig' => [
'configKey' => 'allowShowToAll',
'configValue' => true,
'strConfig' => "true"
],
'arrayConfig' => [
'configKey' => 'allowPermitAll',
'configValue' => [
Expand All @@ -111,7 +116,7 @@ public function dataUpdateAppConfig() {
public function testUpdateAppConfig(string $configKey, $configValue, string $strConfig) {
$this->logger->expects($this->once())
->method('debug');

$this->config->expects($this->once())
->method('setAppValue')
->with('forms', $configKey, $strConfig);
Expand All @@ -122,7 +127,7 @@ public function testUpdateAppConfig(string $configKey, $configValue, string $str
public function testUpdateAppConfig_unknownKey() {
$this->logger->expects($this->once())
->method('debug');

$this->config->expects($this->never())
->method('setAppValue');

Expand Down
5 changes: 4 additions & 1 deletion tests/Unit/Service/ConfigServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public function dataGetAppConfig() {
'oneFullConfig' => [
'strConfig' => [
'allowPermitAll' => 'false',
'allowShowToAll' => 'false',
'allowPublicLink' => 'false',
'creationAllowedGroups' => '["group1", "group2", "nonExisting"]',
'restrictCreation' => 'true',
Expand All @@ -94,6 +95,7 @@ public function dataGetAppConfig() {
],
'expected' => [
'allowPermitAll' => false,
'allowShowToAll' => false,
'allowPublicLink' => false,
'creationAllowedGroups' => [
[
Expand Down Expand Up @@ -156,6 +158,7 @@ public function dataGetAppConfig_Default() {
'defaultValues' => [
'expected' => [
'allowPermitAll' => true,
'allowShowToAll' => true,
'allowPublicLink' => true,
'creationAllowedGroups' => [],
'restrictCreation' => false,
Expand Down Expand Up @@ -223,7 +226,7 @@ public function testCanCreateForms(array $config, bool $expected) {
->method('getUserGroupIds')
->with($this->currentUser)
->willReturn(["usersGroup"]);

$this->assertEquals($expected, $this->configService->canCreateForms());
}
}
34 changes: 34 additions & 0 deletions tests/Unit/Service/FormsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,10 @@ public function testIsSharedFormShown(string $ownerId, int $expires, array $acce
->method('getAllowPermitAll')
->willReturn(true);

$this->configService->expects($this->any())
->method('getAllowShowToAll')
->willReturn(true);

$share = new Share();
$share->setShareType($shareType);
$share->setShareWith('currentUser'); // Only relevant, if $shareType is TYPE_USER, otherwise it's just some 'hash'
Expand All @@ -1129,6 +1133,36 @@ public function testIsSharedFormShown_PermitAllNotAllowed() {
->method('getAllowPermitAll')
->willReturn(false);

$this->configService->expects($this->any())
->method('getAllowShowToAll')
->willReturn(false);

$this->shareMapper->expects($this->any())
->method('findByForm')
->with(42)
->willReturn([]);

$this->assertEquals(false, $this->formsService->isSharedFormShown($form));
}

public function testIsSharedFormShown_PermitShowNotAllowed() {
$form = new Form();
$form->setId(42);
$form->setOwnerId('notCurrentUser');
$form->setExpires(false);
$form->setAccess([
'permitAllUsers' => true,
'showToAllUsers' => true,
]);

$this->configService->expects($this->any())
->method('getAllowPermitAll')
->willReturn(true);

$this->configService->expects($this->any())
->method('getAllowShowToAll')
->willReturn(false);

$this->shareMapper->expects($this->any())
->method('findByForm')
->with(42)
Expand Down
Loading