Merge pull request #3994 from BookStackApp/app_icon_setting

ssddanbrown · web-flow · commit f6d3944b20cc · 2023-01-25T16:50:48.000Z
Added ability to control app icon (favicon) via settings
diff --git a/app/Http/Controllers/SettingController.php b/app/Http/Controllers/SettingController.php
@@ -4,20 +4,14 @@
 
 use BookStack\Actions\ActivityType;
 use BookStack\Auth\User;
+use BookStack\Settings\AppSettingsStore;
 use BookStack\Uploads\ImageRepo;
 use Illuminate\Http\Request;
 
 class SettingController extends Controller
 {
-    protected ImageRepo $imageRepo;
-
     protected array $settingCategories = ['features', 'customization', 'registration'];
 
-    public function __construct(ImageRepo $imageRepo)
-    {
-        $this->imageRepo = $imageRepo;
-    }
-
     /**
      * Handle requests to the settings index path.
      */
@@ -48,37 +42,17 @@ public function category(string $category)
     /**
      * Update the specified settings in storage.
      */
-    public function update(Request $request, string $category)
+    public function update(Request $request, AppSettingsStore $store, string $category)
     {
         $this->ensureCategoryExists($category);
         $this->preventAccessInDemoMode();
         $this->checkPermission('settings-manage');
         $this->validate($request, [
-            'app_logo' => array_merge(['nullable'], $this->getImageValidationRules()),
+            'app_logo' => ['nullable', ...$this->getImageValidationRules()],
+            'app_icon' => ['nullable', ...$this->getImageValidationRules()],
         ]);
 
-        // Cycles through posted settings and update them
-        foreach ($request->all() as $name => $value) {
-            $key = str_replace('setting-', '', trim($name));
-            if (strpos($name, 'setting-') !== 0) {
-                continue;
-            }
-            setting()->put($key, $value);
-        }
-
-        // Update logo image if set
-        if ($category === 'customization' && $request->hasFile('app_logo')) {
-            $logoFile = $request->file('app_logo');
-            $this->imageRepo->destroyByType('system');
-            $image = $this->imageRepo->saveNew($logoFile, 'system', 0, null, 86);
-            setting()->put('app-logo', $image->url);
-        }
-
-        // Clear logo image if requested
-        if ($category === 'customization' && $request->get('app_logo_reset', null)) {
-            $this->imageRepo->destroyByType('system');
-            setting()->remove('app-logo');
-        }
+        $store->storeFromUpdateRequest($request, $category);
 
         $this->logActivity(ActivityType::SETTINGS_UPDATE, $category);
         $this->showSuccessNotification(trans('settings.settings_save_success'));
diff --git a/app/Settings/AppSettingsStore.php b/app/Settings/AppSettingsStore.php
@@ -0,0 +1,91 @@
+<?php
+
+namespace BookStack\Settings;
+
+use BookStack\Uploads\ImageRepo;
+use Illuminate\Http\Request;
+
+class AppSettingsStore
+{
+    protected ImageRepo $imageRepo;
+
+    public function __construct(ImageRepo $imageRepo)
+    {
+        $this->imageRepo = $imageRepo;
+    }
+
+    public function storeFromUpdateRequest(Request $request, string $category)
+    {
+        $this->storeSimpleSettings($request);
+        if ($category === 'customization') {
+            $this->updateAppLogo($request);
+            $this->updateAppIcon($request);
+        }
+    }
+
+    protected function updateAppIcon(Request $request): void
+    {
+        $sizes = [180, 128, 64, 32];
+
+        // Update icon image if set
+        if ($request->hasFile('app_icon')) {
+            $iconFile = $request->file('app_icon');
+            $this->destroyExistingSettingImage('app-icon');
+            $image = $this->imageRepo->saveNew($iconFile, 'system', 0, 256, 256);
+            setting()->put('app-icon', $image->url);
+
+            foreach ($sizes as $size) {
+                $this->destroyExistingSettingImage('app-icon-' . $size);
+                $icon = $this->imageRepo->saveNew($iconFile, 'system', 0, $size, $size);
+                setting()->put('app-icon-' . $size, $icon->url);
+            }
+        }
+
+        // Clear icon image if requested
+        if ($request->get('app_icon_reset')) {
+            $this->destroyExistingSettingImage('app-icon');
+            setting()->remove('app-icon');
+            foreach ($sizes as $size) {
+                $this->destroyExistingSettingImage('app-icon-' . $size);
+                setting()->remove('app-icon-' . $size);
+            }
+        }
+    }
+
+    protected function updateAppLogo(Request $request): void
+    {
+        // Update logo image if set
+        if ($request->hasFile('app_logo')) {
+            $logoFile = $request->file('app_logo');
+            $this->destroyExistingSettingImage('app-logo');
+            $image = $this->imageRepo->saveNew($logoFile, 'system', 0, null, 86);
+            setting()->put('app-logo', $image->url);
+        }
+
+        // Clear logo image if requested
+        if ($request->get('app_logo_reset')) {
+            $this->destroyExistingSettingImage('app-logo');
+            setting()->remove('app-logo');
+        }
+    }
+
+    protected function storeSimpleSettings(Request $request): void
+    {
+        foreach ($request->all() as $name => $value) {
+            if (strpos($name, 'setting-') !== 0) {
+                continue;
+            }
+
+            $key = str_replace('setting-', '', trim($name));
+            setting()->put($key, $value);
+        }
+    }
+
+    protected function destroyExistingSettingImage(string $settingKey)
+    {
+        $existingVal = setting()->get($settingKey);
+        if ($existingVal) {
+            $this->imageRepo->destroyByUrlAndType($existingVal, 'system');
+        }
+    }
+}
diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php
@@ -12,15 +12,11 @@
  */
 class SettingService
 {
-    protected $setting;
-    protected $cache;
-    protected $localCache = [];
+    protected Setting $setting;
+    protected Cache $cache;
+    protected array $localCache = [];
+    protected string $cachePrefix = 'setting-';
 
-    protected $cachePrefix = 'setting-';
-
-    /**
-     * SettingService constructor.
-     */
     public function __construct(Setting $setting, Cache $cache)
     {
         $this->setting = $setting;
diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php
@@ -123,7 +123,10 @@ public function getEntityFiltered(
     public function saveNew(UploadedFile $uploadFile, string $type, int $uploadedTo = 0, int $resizeWidth = null, int $resizeHeight = null, bool $keepRatio = true): Image
     {
         $image = $this->imageService->saveNewFromUpload($uploadFile, $type, $uploadedTo, $resizeWidth, $resizeHeight, $keepRatio);
-        $this->loadThumbs($image);
+
+        if ($type !== 'system') {
+            $this->loadThumbs($image);
+        }
 
         return $image;
     }
@@ -180,13 +183,17 @@ public function destroyImage(Image $image = null): void
     }
 
     /**
-     * Destroy all images of a certain type.
+     * Destroy images that have a specific URL and type combination.
      *
      * @throws Exception
      */
-    public function destroyByType(string $imageType): void
+    public function destroyByUrlAndType(string $url, string $imageType): void
     {
-        $images = Image::query()->where('type', '=', $imageType)->get();
+        $images = Image::query()
+            ->where('url', '=', $url)
+            ->where('type', '=', $imageType)
+            ->get();
+
         foreach ($images as $image) {
             $this->destroyImage($image);
         }
diff --git a/public/icon-128.png b/public/icon-128.png
diff --git a/public/icon-32.png b/public/icon-32.png
diff --git a/public/icon-64.png b/public/icon-64.png
diff --git a/public/icon.png b/public/icon.png
diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php
@@ -33,7 +33,9 @@
     'app_custom_html_desc' => 'Any content added here will be inserted into the bottom of the <head> section of every page. This is handy for overriding styles or adding analytics code.',
     'app_custom_html_disabled_notice' => 'Custom HTML head content is disabled on this settings page to ensure any breaking changes can be reverted.',
     'app_logo' => 'Application Logo',
-    'app_logo_desc' => 'This image should be 43px in height. <br>Large images will be scaled down.',
+    'app_logo_desc' => 'This is used in the application header bar, among other areas. This image should be 86px in height. Large images will be scaled down.',
+    'app_icon' => 'Application Icon',
+    'app_icon_desc' => 'This icon is used for browser tabs and shortcut icons. This should be a 256px square PNG image.',
     'app_primary_color' => 'Application Primary Color',
     'app_primary_color_desc' => 'Sets the primary color for the application including the banner, buttons, and links.',
     'app_homepage' => 'Application Homepage',
diff --git a/resources/views/layouts/base.blade.php b/resources/views/layouts/base.blade.php
@@ -6,10 +6,11 @@ class="{{ setting()->getForCurrentUser('dark-mode-enabled') ? 'dark-mode ' : ''
     <title>{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}</title>
 
     <!-- Meta -->
+    <meta charset="utf-8">
     <meta name="viewport" content="width=device-width">
     <meta name="token" content="{{ csrf_token() }}">
     <meta name="base-url" content="{{ url('/') }}">
-    <meta charset="utf-8">
+    <meta name="theme-color" content="{{ setting('app-color') }}"/>
 
     <!-- Social Cards Meta -->
     <meta property="og:title" content="{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}">
@@ -20,6 +21,14 @@ class="{{ setting()->getForCurrentUser('dark-mode-enabled') ? 'dark-mode ' : ''
     <link rel="stylesheet" href="{{ versioned_asset('dist/styles.css') }}">
     <link rel="stylesheet" media="print" href="{{ versioned_asset('dist/print-styles.css') }}">
 
+    <!-- Icons -->
+    <link rel="icon" type="image/png" sizes="256x256" href="{{ setting('app-icon') ?: url('/icon.png') }}">
+    <link rel="icon" type="image/png" sizes="180x180" href="{{ setting('app-icon-180') ?: url('/icon-180.png') }}">
+    <link rel="apple-touch-icon" sizes="180x180" href="{{ setting('app-icon-180') ?: url('/icon-180.png') }}">
+    <link rel="icon" type="image/png" sizes="128x128" href="{{ setting('app-icon-128') ?: url('/icon-128.png') }}">
+    <link rel="icon" type="image/png" sizes="64x64" href="{{ setting('app-icon-64') ?: url('/icon-64.png') }}">
+    <link rel="icon" type="image/png" sizes="32x32" href="{{ setting('app-icon-32') ?: url('/icon-32.png') }}">
+
     @yield('head')
 
     <!-- Custom Styles & Head Content -->
diff --git a/resources/views/settings/customization.blade.php b/resources/views/settings/customization.blade.php
@@ -53,6 +53,22 @@
                 </div>
             </div>
 
+            <div class="grid half gap-xl">
+                <div>
+                    <label class="setting-list-label">{{ trans('settings.app_icon') }}</label>
+                    <p class="small">{{ trans('settings.app_icon_desc') }}</p>
+                </div>
+                <div class="pt-xs">
+                    @include('form.image-picker', [
+                             'removeValue' => 'none',
+                             'defaultImage' => url('/icon.png'),
+                             'currentImage' => setting('app-icon'),
+                             'name' => 'app_icon',
+                             'imageClass' => 'logo-image',
+                         ])
+                </div>
+            </div>
+
             <!-- Primary Color -->
             <div class="grid half gap-xl">
                 <div>
diff --git a/tests/Settings/SettingsTest.php b/tests/Settings/SettingsTest.php
@@ -2,10 +2,14 @@
 
 namespace Tests\Settings;
 
+use Illuminate\Support\Facades\Storage;
 use Tests\TestCase;
+use Tests\Uploads\UsesImages;
 
 class SettingsTest extends TestCase
 {
+    use UsesImages;
+
     public function test_settings_endpoint_redirects_to_settings_view()
     {
         $resp = $this->asAdmin()->get('/settings');
@@ -40,4 +44,46 @@ public function test_not_found_setting_category_throws_404()
         $resp->assertStatus(404);
         $resp->assertSee('Page Not Found');
     }
+
+    public function test_updating_and_removing_app_icon()
+    {
+        $this->asAdmin();
+        $galleryFile = $this->getTestImage('my-app-icon.png');
+        $expectedPath = public_path('uploads/images/system/' . date('Y-m') . '/my-app-icon.png');
+
+        $this->assertFalse(setting()->get('app-icon'));
+        $this->assertFalse(setting()->get('app-icon-180'));
+        $this->assertFalse(setting()->get('app-icon-128'));
+        $this->assertFalse(setting()->get('app-icon-64'));
+        $this->assertFalse(setting()->get('app-icon-32'));
+
+        $prevFileCount = count(glob(dirname($expectedPath) . DIRECTORY_SEPARATOR . '*.png'));
+
+        $upload = $this->call('POST', '/settings/customization', [], [], ['app_icon' => $galleryFile], []);
+        $upload->assertRedirect('/settings/customization');
+
+        $this->assertTrue(file_exists($expectedPath), 'Uploaded image not found at path: ' . $expectedPath);
+        $this->assertStringContainsString('my-app-icon', setting()->get('app-icon'));
+        $this->assertStringContainsString('my-app-icon', setting()->get('app-icon-180'));
+        $this->assertStringContainsString('my-app-icon', setting()->get('app-icon-128'));
+        $this->assertStringContainsString('my-app-icon', setting()->get('app-icon-64'));
+        $this->assertStringContainsString('my-app-icon', setting()->get('app-icon-32'));
+
+        $newFileCount = count(glob(dirname($expectedPath) . DIRECTORY_SEPARATOR . '*.png'));
+        $this->assertEquals(5, $newFileCount - $prevFileCount);
+
+        $resp = $this->get('/');
+        $this->withHtml($resp)->assertElementCount('link[sizes][href*="my-app-icon"]', 6);
+
+        $reset = $this->post('/settings/customization', ['app_icon_reset' => 'true']);
+        $reset->assertRedirect('/settings/customization');
+
+        $resetFileCount = count(glob(dirname($expectedPath) . DIRECTORY_SEPARATOR . '*.png'));
+        $this->assertEquals($prevFileCount, $resetFileCount);
+        $this->assertFalse(setting()->get('app-icon'));
+        $this->assertFalse(setting()->get('app-icon-180'));
+        $this->assertFalse(setting()->get('app-icon-128'));
+        $this->assertFalse(setting()->get('app-icon-64'));
+        $this->assertFalse(setting()->get('app-icon-32'));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -123,7 +123,10 @@ public function getEntityFiltered(`
`123`	`123`	`public function saveNew(UploadedFile $uploadFile, string $type, int $uploadedTo = 0, int $resizeWidth = null, int $resizeHeight = null, bool $keepRatio = true): Image`
`124`	`124`	`{`
`125`	`125`	`$image = $this->imageService->saveNewFromUpload($uploadFile, $type, $uploadedTo, $resizeWidth, $resizeHeight, $keepRatio);`
`126`		`- $this->loadThumbs($image);`
	`126`	`+`
	`127`	`+ if ($type !== 'system') {`
	`128`	`+ $this->loadThumbs($image);`
	`129`	`+ }`
`127`	`130`
`128`	`131`	`return $image;`
`129`	`132`	`}`
`@@ -180,13 +183,17 @@ public function destroyImage(Image $image = null): void`
`180`	`183`	`}`
`181`	`184`
`182`	`185`	`/**`
`183`		`- * Destroy all images of a certain type.`
	`186`	`+ * Destroy images that have a specific URL and type combination.`
`184`	`187`	`*`
`185`	`188`	`* @throws Exception`
`186`	`189`	`*/`
`187`		`- public function destroyByType(string $imageType): void`
	`190`	`+ public function destroyByUrlAndType(string $url, string $imageType): void`
`188`	`191`	`{`
`189`		`- $images = Image::query()->where('type', '=', $imageType)->get();`
	`192`	`+ $images = Image::query()`
	`193`	`+ ->where('url', '=', $url)`
	`194`	`+ ->where('type', '=', $imageType)`
	`195`	`+ ->get();`
	`196`	`+`
`190`	`197`	`foreach ($images as $image) {`
`191`	`198`	`$this->destroyImage($image);`
`192`	`199`	`}`