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

diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php
index 1c053b3848ea779d480adea01834dce0059d629d..042ae7aa4bf8e4aa86b5e02a50951c0574d23431 100644 (file)
--- a/app/Settings/SettingService.php
+++ b/app/Settings/SettingService.php
@@ -1,5 +1,6 @@
 <?php namespace BookStack\Settings;
 
 <?php namespace BookStack\Settings;
 

+use BookStack\Auth\User;

 use Illuminate\Contracts\Cache\Repository as Cache;
 
 /**
 use Illuminate\Contracts\Cache\Repository as Cache;
 
 /**


@@ -9,7 +10,6 @@ use Illuminate\Contracts\Cache\Repository as Cache;
  */
 class SettingService
 {
  */
 class SettingService
 {

-

     protected $setting;
     protected $cache;
     protected $localCache = [];
     protected $setting;
     protected $cache;
     protected $localCache = [];


@@ -18,8 +18,6 @@ class SettingService
 
     /**
      * SettingService constructor.
 
     /**
      * SettingService constructor.

-     * @param Setting $setting
-     * @param Cache   $cache

      */
     public function __construct(Setting $setting, Cache $cache)
     {
      */
     public function __construct(Setting $setting, Cache $cache)
     {


@@ -30,11 +28,8 @@ class SettingService
     /**
      * Gets a setting from the database,
      * If not found, Returns default, Which is false by default.
     /**
      * Gets a setting from the database,
      * If not found, Returns default, Which is false by default.

-     * @param             $key
-     * @param string|bool $default
-     * @return bool|string

      */
      */

-    public function get($key, $default = false)
+    public function get(string $key, $default = false)

     {
         if ($default === false) {
             $default = config('setting-defaults.' . $key, false);
     {
         if ($default === false) {
             $default = config('setting-defaults.' . $key, false);


@@ -44,7 +39,7 @@ class SettingService
             return $this->localCache[$key];
         }
 
             return $this->localCache[$key];
         }
 

-        $value = $this->getValueFromStore($key, $default);
+        $value = $this->getValueFromStore($key) ?? $default;

         $formatted = $this->formatValue($value, $default);
         $this->localCache[$key] = $formatted;
         return $formatted;
         $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.
 
     /**
      * 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);
     {
         $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.
     }
 
     /**
      * 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);
     {
         if ($user->isDefault()) {
             return $this->getFromSession($key, $default);


@@ -80,11 +67,8 @@ class SettingService
 
     /**
      * Get a value for the current logged-in user.
 
     /**
      * 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);
     }
     {
         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.
     /**
      * 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;
     {
         // Check the cache
         $cacheKey = $this->cachePrefix . $key;


@@ -109,18 +91,22 @@ class SettingService
         $settingObject = $this->getSettingObjectByKey($key);
         if ($settingObject !== null) {
             $value = $settingObject->value;
         $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;
         }
 
             $this->cache->forever($cacheKey, $value);
             return $value;
         }
 

-        return $default;
+        return null;

     }
 
     /**
      * Clear an item from the cache completely.
     }
 
     /**
      * 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);
     {
         $cacheKey = $this->cachePrefix . $key;
         $this->cache->forget($cacheKey);


@@ -131,17 +117,13 @@ class SettingService
 
     /**
      * Format a settings value
 
     /**
      * 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;
      */
     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;
         }
 
             $value = false;
         }
 


@@ -154,99 +136,97 @@ class SettingService
 
     /**
      * Checks if a setting exists.
 
     /**
      * 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;
     }
 
     {
         $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.
     /**
      * 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_key' => $key
         ]);

+        $setting->type = 'string';
+
+        if (is_array($value)) {
+            $setting->type = 'array';
+            $value = $this->formatArrayValue($value);
+        }
+

         $setting->value = $value;
         $setting->save();
         $this->clearFromCache($key);
         return true;
     }
 
         $setting->value = $value;
         $setting->save();
         $this->clearFromCache($key);
         return true;
     }
 

+    /**
+     * 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.
+     */
+    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.
     /**
      * Put a user-specific setting into the database.

-     * @param \BookStack\Auth\User $user
-     * @param $key
-     * @param $value
-     * @return bool

      */
      */

-    public function putUser($user, $key, $value)
+    public function putUser(User $user, string $key, string $value): bool

     {
         if ($user->isDefault()) {
     {
         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.
         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.
     {
         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);
     {
         $setting = $this->getSettingObjectByKey($key);
         if ($setting) {
             $setting->delete();
         }
         $this->clearFromCache($key);

-        return true;

     }
 
     /**
      * Delete settings for a given user id.
     }
 
     /**
      * 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.
     }
 
     /**
      * 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 (file)
index 0000000..61d9bda
--- /dev/null
+++ b/database/migrations/2021_01_30_225441_add_settings_type_column.php
@@ -0,0 +1,32 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class AddSettingsTypeColumn extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('settings', function (Blueprint $table) {
+            $table->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 e87bd11a5e343173fadf78e62a042e1ca0579a4a..e048db90f4c2ff13d17a5fe29d449dac302c95e2 100644 (file)
--- 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',
     // 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 878a9991ffd09dd1559cd4bbebdb542680157118..bd55668a2e94e59b13f395b9fe0655fb28790889 100755 (executable)
--- 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_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::<key>" 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. <br> Existing comments are not shown.',
     'app_disable_comments' => 'Disable Comments',
     'app_disable_comments_toggle' => 'Disable comments',
     'app_disable_comments_desc' => 'Disables comments across all pages in the application. <br> Existing comments are not shown.',

diff --git a/resources/sass/_footer.scss b/resources/sass/_footer.scss
index 9609115846dc0e6cb5ef45ae4f8352eaded4b5f5..1c58bccd97696739151c0f8dbb72b3f336fd6741 100644 (file)
--- a/resources/sass/_footer.scss
+++ b/resources/sass/_footer.scss
@@ -4,7 +4,7 @@
 
  footer {
     flex-shrink: 0;
 
  footer {
     flex-shrink: 0;

-    padding: .5em;
+    padding: 1rem 1rem 2rem 1rem;

     text-align: center;
   }
   
     text-align: center;
   }
   

diff --git a/resources/views/common/footer.blade.php b/resources/views/common/footer.blade.php
index a056f6d99ec34e52a59d9e2060d58647f1826d96..67b52a609e686881e6286c9f7ad028ab6f8b494a 100644 (file)
--- 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)

 <footer>
 <footer>

-    @if(setting('app-privacy-policy'))
-        <a href="{{ setting('app-privacy-policy') }}">{{ trans('settings.app_privacy_policy') }}</a>
-    @endif
-    @if(setting('app-terms-of-service'))
-        <a href="{{ setting('app-terms-of-service') }}">{{ trans('settings.app_terms_of_service') }}</a>
-    @endif
+    @foreach(setting('app-footer-links', []) as $link)
+        <a href="{{ $link['url'] }}" target="_blank">{{ strpos($link['label'], 'trans::') === 0 ? trans(str_replace('trans::', '', $link['label'])) : $link['label'] }}</a>
+    @endforeach

 </footer>
 @endif
\ No newline at end of file
 </footer>
 @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 (file)
index 0000000..10bf756
--- /dev/null
+++ b/resources/views/settings/footer-links.blade.php
@@ -0,0 +1,34 @@
+{{--
+$value - Setting value
+$name - Setting input name
+--}}
+<div components="add-remove-rows"
+     option:add-remove-rows:row-selector=".card"
+     option:add-remove-rows:remove-selector="button.text-neg">
+
+    <div component="sortable-list"
+         option:sortable-list:handle-selector=".handle">
+        @foreach(array_merge($value, [['label' => '', 'url' => '']]) as $index => $link)
+            <div class="card drag-card {{ $loop->last ? 'hidden' : '' }}" @if($loop->last) refs="add-remove-rows@model" @endif>
+                <div class="handle">@icon('grip')</div>
+                @foreach(['label', 'url'] as $prop)
+                    <div class="outline">
+                        <input value="{{ $link[$prop] ?? '' }}"
+                               placeholder="{{ trans('settings.app_footer_links_' . $prop) }}"
+                               aria-label="{{ trans('settings.app_footer_links_' . $prop) }}"
+                               name="{{ $name }}[{{ $loop->parent->last ? 'randrowid' : $index }}][{{$prop}}]"
+                               type="text"
+                               autocomplete="off"/>
+                    </div>
+                @endforeach
+                <button type="button"
+                        aria-label="{{ trans('common.remove') }}"
+                        class="text-center drag-card-action text-neg">
+                    @icon('close')
+                </button>
+            </div>
+        @endforeach
+    </div>
+
+    <button refs="add-remove-rows@add" type="button" class="text-button">{{ trans('settings.app_footer_links_add') }}</button>
+</div>
\ No newline at end of file

diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php
index 34d3cad9293c882ed789cfb91a4a35321487e313..ad03b6c917fdfecc670a3950650bfa092f7c263a 100644 (file)
--- a/resources/views/settings/index.blade.php
+++ b/resources/views/settings/index.blade.php
@@ -181,17 +181,8 @@
 
                     <div>
                         <label for="setting-app-privacy-link" class="setting-list-label">{{ trans('settings.app_footer_links') }}</label>
 
                     <div>
                         <label for="setting-app-privacy-link" class="setting-list-label">{{ trans('settings.app_footer_links') }}</label>

-                        <p class="small">{{ trans('settings.app_footer_links_desc') }}</p>
-                        <div class="grid half pt-m">
-                            <div>
-                                <label for="setting-app-privacy-policy" class="text-dark">{{ trans('settings.app_privacy_policy') }}</label>
-                                <input type="text" value="{{ setting('app-privacy-policy', '') }}" name="setting-app-privacy-policy" id="setting-app-privacy-policy">
-                            </div>
-                            <div>
-                                <label for="setting-app-terms-of-service" class="text-dark">{{ trans('settings.app_terms_of_service') }}</label>
-                                <input type="text" value="{{ setting('app-terms-of-service', '') }}" name="setting-app-terms-of-service" id="setting-app-terms-of-service">
-                            </div>
-                        </div>
+                        <p class="small mb-m">{{ trans('settings.app_footer_links_desc') }}</p>
+                        @include('settings.footer-links', ['name' => 'setting-app-footer-links', 'value' => setting('app-footer-links', [])])

                     </div>
 
 
                     </div>
 
 

diff --git a/tests/FooterLinksTest.php b/tests/FooterLinksTest.php
new file mode 100644 (file)
index 0000000..f0ff0c4
--- /dev/null
+++ b/tests/FooterLinksTest.php
@@ -0,0 +1,61 @@
+<?php
+
+use Tests\TestCase;
+
+class FooterLinksTest extends TestCase
+{
+
+    public function test_saving_setting()
+    {
+        $resp = $this->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