From 44c41e9e4dbe31f071b8c87ffae77ad6737c2cbc Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 31 Jan 2021 00:23:15 +0000 Subject: [PATCH] Updated footer links to be a configurable list Made so footer link ordering, names and urls can be set. Cleaned up some of the setting-service and added support for array setting types, which are cleaned on entry and stored as json with a new type indicator column on the settings table for auto-decode. Also added testing to cover this feature. Related to #1973 and #854 --- app/Settings/SettingService.php | 130 ++++++++---------- ..._01_30_225441_add_settings_type_column.php | 32 +++++ resources/lang/en/common.php | 5 + resources/lang/en/settings.php | 7 +- resources/sass/_footer.scss | 2 +- resources/views/common/footer.blade.php | 11 +- .../views/settings/footer-links.blade.php | 34 +++++ resources/views/settings/index.blade.php | 13 +- tests/FooterLinksTest.php | 61 ++++++++ 9 files changed, 198 insertions(+), 97 deletions(-) create mode 100644 database/migrations/2021_01_30_225441_add_settings_type_column.php create mode 100644 resources/views/settings/footer-links.blade.php create mode 100644 tests/FooterLinksTest.php diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php index 1c053b384..042ae7aa4 100644 --- a/app/Settings/SettingService.php +++ b/app/Settings/SettingService.php @@ -1,5 +1,6 @@ localCache[$key]; } - $value = $this->getValueFromStore($key, $default); + $value = $this->getValueFromStore($key) ?? $default; $formatted = $this->formatValue($value, $default); $this->localCache[$key] = $formatted; return $formatted; @@ -52,25 +47,17 @@ class SettingService /** * Get a value from the session instead of the main store option. - * @param $key - * @param bool $default - * @return mixed */ - protected function getFromSession($key, $default = false) + protected function getFromSession(string $key, $default = false) { $value = session()->get($key, $default); - $formatted = $this->formatValue($value, $default); - return $formatted; + return $this->formatValue($value, $default); } /** * Get a user-specific setting from the database or cache. - * @param \BookStack\Auth\User $user - * @param $key - * @param bool $default - * @return bool|string */ - public function getUser($user, $key, $default = false) + public function getUser(User $user, string $key, $default = false) { if ($user->isDefault()) { return $this->getFromSession($key, $default); @@ -80,11 +67,8 @@ class SettingService /** * Get a value for the current logged-in user. - * @param $key - * @param bool $default - * @return bool|string */ - public function getForCurrentUser($key, $default = false) + public function getForCurrentUser(string $key, $default = false) { return $this->getUser(user(), $key, $default); } @@ -92,11 +76,9 @@ class SettingService /** * Gets a setting value from the cache or database. * Looks at the system defaults if not cached or in database. - * @param $key - * @param $default - * @return mixed + * Returns null if nothing is found. */ - protected function getValueFromStore($key, $default) + protected function getValueFromStore(string $key) { // Check the cache $cacheKey = $this->cachePrefix . $key; @@ -109,18 +91,22 @@ class SettingService $settingObject = $this->getSettingObjectByKey($key); if ($settingObject !== null) { $value = $settingObject->value; + + if ($settingObject->type === 'array') { + $value = json_decode($value, true) ?? []; + } + $this->cache->forever($cacheKey, $value); return $value; } - return $default; + return null; } /** * Clear an item from the cache completely. - * @param $key */ - protected function clearFromCache($key) + protected function clearFromCache(string $key) { $cacheKey = $this->cachePrefix . $key; $this->cache->forget($cacheKey); @@ -131,17 +117,13 @@ class SettingService /** * Format a settings value - * @param $value - * @param $default - * @return mixed */ protected function formatValue($value, $default) { // Change string booleans to actual booleans if ($value === 'true') { $value = true; - } - if ($value === 'false') { + } else if ($value === 'false') { $value = false; } @@ -154,36 +136,29 @@ class SettingService /** * Checks if a setting exists. - * @param $key - * @return bool */ - public function has($key) + public function has(string $key): bool { $setting = $this->getSettingObjectByKey($key); return $setting !== null; } - /** - * Check if a user setting is in the database. - * @param $key - * @return bool - */ - public function hasUser($key) - { - return $this->has($this->userKey($key)); - } - /** * Add a setting to the database. - * @param $key - * @param $value - * @return bool + * Values can be an array or a string. */ - public function put($key, $value) + public function put(string $key, $value): bool { - $setting = $this->setting->firstOrNew([ + $setting = $this->setting->newQuery()->firstOrNew([ 'setting_key' => $key ]); + $setting->type = 'string'; + + if (is_array($value)) { + $setting->type = 'array'; + $value = $this->formatArrayValue($value); + } + $setting->value = $value; $setting->save(); $this->clearFromCache($key); @@ -191,62 +166,67 @@ class SettingService } /** - * Put a user-specific setting into the database. - * @param \BookStack\Auth\User $user - * @param $key - * @param $value - * @return bool + * Format an array to be stored as a setting. + * Array setting types are expected to be a flat array of child key=>value array items. + * This filters out any child items that are empty. */ - public function putUser($user, $key, $value) + protected function formatArrayValue(array $value): string + { + $values = collect($value)->values()->filter(function(array $item) { + return count(array_filter($item)) > 0; + }); + return json_encode($values); + } + + /** + * Put a user-specific setting into the database. + */ + public function putUser(User $user, string $key, string $value): bool { if ($user->isDefault()) { - return session()->put($key, $value); + session()->put($key, $value); + return true; } + return $this->put($this->userKey($user->id, $key), $value); } /** * Convert a setting key into a user-specific key. - * @param $key - * @return string */ - protected function userKey($userId, $key = '') + protected function userKey(string $userId, string $key = ''): string { return 'user:' . $userId . ':' . $key; } /** * Removes a setting from the database. - * @param $key - * @return bool */ - public function remove($key) + public function remove(string $key): void { $setting = $this->getSettingObjectByKey($key); if ($setting) { $setting->delete(); } $this->clearFromCache($key); - return true; } /** * Delete settings for a given user id. - * @param $userId - * @return mixed */ - public function deleteUserSettings($userId) + public function deleteUserSettings(string $userId) { - return $this->setting->where('setting_key', 'like', $this->userKey($userId) . '%')->delete(); + return $this->setting->newQuery() + ->where('setting_key', 'like', $this->userKey($userId) . '%') + ->delete(); } /** * Gets a setting model from the database for the given key. - * @param $key - * @return mixed */ - protected function getSettingObjectByKey($key) + protected function getSettingObjectByKey(string $key): ?Setting { - return $this->setting->where('setting_key', '=', $key)->first(); + return $this->setting->newQuery() + ->where('setting_key', '=', $key)->first(); } } diff --git a/database/migrations/2021_01_30_225441_add_settings_type_column.php b/database/migrations/2021_01_30_225441_add_settings_type_column.php new file mode 100644 index 000000000..61d9bda41 --- /dev/null +++ b/database/migrations/2021_01_30_225441_add_settings_type_column.php @@ -0,0 +1,32 @@ +string('type', 50)->default('string'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('settings', function (Blueprint $table) { + $table->dropColumn('type'); + }); + } +} diff --git a/resources/lang/en/common.php b/resources/lang/en/common.php index e87bd11a5..e048db90f 100644 --- a/resources/lang/en/common.php +++ b/resources/lang/en/common.php @@ -77,4 +77,9 @@ return [ // Email Content 'email_action_help' => 'If you’re having trouble clicking the ":actionText" button, copy and paste the URL below into your web browser:', 'email_rights' => 'All rights reserved', + + // Footer Link Options + // Not directly used but available for convenience to users. + 'privacy_policy' => 'Privacy Policy', + 'terms_of_service' => 'Terms of Service', ]; diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 878a9991f..bd55668a2 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -38,9 +38,10 @@ return [ 'app_homepage_desc' => 'Select a view to show on the homepage instead of the default view. Page permissions are ignored for selected pages.', 'app_homepage_select' => 'Select a page', 'app_footer_links' => 'Footer Links', - 'app_footer_links_desc' => 'Certain countries may require that websites include a privacy policy or terms of service. You may provide links to those here, which will then be displayed at the bottom of each page.', - 'app_privacy_policy' => 'Privacy Policy', - 'app_terms_of_service' => 'Terms of Service', + 'app_footer_links_desc' => 'Add links to show within the site footer. These will be displayed at the bottom of most pages, including those that do not require login. You can use a label of "trans::" to use system-defined translations. For example: Using "trans::common.privacy_policy" will provide the translated text "Privacy Policy" and "trans::common.terms_of_service" will provide the translated text "Terms of Service".', + 'app_footer_links_label' => 'Link Label', + 'app_footer_links_url' => 'Link URL', + 'app_footer_links_add' => 'Add Footer Link', 'app_disable_comments' => 'Disable Comments', 'app_disable_comments_toggle' => 'Disable comments', 'app_disable_comments_desc' => 'Disables comments across all pages in the application.
Existing comments are not shown.', diff --git a/resources/sass/_footer.scss b/resources/sass/_footer.scss index 960911584..1c58bccd9 100644 --- a/resources/sass/_footer.scss +++ b/resources/sass/_footer.scss @@ -4,7 +4,7 @@ footer { flex-shrink: 0; - padding: .5em; + padding: 1rem 1rem 2rem 1rem; text-align: center; } diff --git a/resources/views/common/footer.blade.php b/resources/views/common/footer.blade.php index a056f6d99..67b52a609 100644 --- a/resources/views/common/footer.blade.php +++ b/resources/views/common/footer.blade.php @@ -1,10 +1,7 @@ -@if(setting('app-privacy-policy') | setting('app-terms-of-service')) +@if(count(setting('app-footer-links', [])) > 0) @endif \ No newline at end of file diff --git a/resources/views/settings/footer-links.blade.php b/resources/views/settings/footer-links.blade.php new file mode 100644 index 000000000..10bf756b5 --- /dev/null +++ b/resources/views/settings/footer-links.blade.php @@ -0,0 +1,34 @@ +{{-- +$value - Setting value +$name - Setting input name +--}} +
+ +
+ @foreach(array_merge($value, [['label' => '', 'url' => '']]) as $index => $link) +
last) refs="add-remove-rows@model" @endif> +
@icon('grip')
+ @foreach(['label', 'url'] as $prop) +
+ +
+ @endforeach + +
+ @endforeach +
+ + +
\ No newline at end of file diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index 34d3cad92..ad03b6c91 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -181,17 +181,8 @@
-

{{ trans('settings.app_footer_links_desc') }}

-
-
- - -
-
- - -
-
+

{{ trans('settings.app_footer_links_desc') }}

+ @include('settings.footer-links', ['name' => 'setting-app-footer-links', 'value' => setting('app-footer-links', [])])
diff --git a/tests/FooterLinksTest.php b/tests/FooterLinksTest.php new file mode 100644 index 000000000..f0ff0c40d --- /dev/null +++ b/tests/FooterLinksTest.php @@ -0,0 +1,61 @@ +asAdmin()->post("/settings", [ + 'setting-app-footer-links' => [ + ['label' => 'My custom link 1', 'url' => 'https://example.com/1'], + ['label' => 'My custom link 2', 'url' => 'https://example.com/2'], + ], + ]); + $resp->assertRedirect('/settings'); + + $result = setting('app-footer-links'); + $this->assertIsArray($result); + $this->assertCount(2, $result); + $this->assertEquals('My custom link 2', $result[1]['label']); + $this->assertEquals('https://example.com/1', $result[0]['url']); + } + + public function test_set_options_visible_on_settings_page() + { + $this->setSettings(['app-footer-links' => [ + ['label' => 'My custom link', 'url' => 'https://example.com/link-a'], + ['label' => 'Another Link', 'url' => 'https://example.com/link-b'], + ]]); + + $resp = $this->asAdmin()->get('/settings'); + $resp->assertSee('value="My custom link"'); + $resp->assertSee('value="Another Link"'); + $resp->assertSee('value="https://example.com/link-a"'); + $resp->assertSee('value="https://example.com/link-b"'); + } + + public function test_footer_links_show_on_pages() + { + $this->setSettings(['app-footer-links' => [ + ['label' => 'My custom link', 'url' => 'https://example.com/link-a'], + ['label' => 'Another Link', 'url' => 'https://example.com/link-b'], + ]]); + + $this->get('/login')->assertElementContains('footer a[href="https://example.com/link-a"]', 'My custom link'); + $this->asEditor()->get('/')->assertElementContains('footer a[href="https://example.com/link-b"]', 'Another link'); + } + + public function test_using_translation_system_for_labels() + { + $this->setSettings(['app-footer-links' => [ + ['label' => 'trans::common.privacy_policy', 'url' => 'https://example.com/privacy'], + ['label' => 'trans::common.terms_of_service', 'url' => 'https://example.com/terms'], + ]]); + + $resp = $this->get('/login'); + $resp->assertElementContains('footer a[href="https://example.com/privacy"]', 'Privacy Policy'); + $resp->assertElementContains('footer a[href="https://example.com/terms"]', 'Terms of Service'); + } +} \ No newline at end of file