From ccb13f3282c109be5d2b35627ceee4a12e027bc9 Mon Sep 17 00:00:00 2001 From: Ilkka Tengvall Date: Sat, 4 Apr 2026 21:32:37 -0700 Subject: [PATCH] feat: move custom logo & favicon to configurable upload directory Support container deployments where static and persistent data should not share the same directory. Custom logo and favicon uploads now use the configured image upload directory (uploads.image.upload.directory) instead of Web/img/ and Web/. Files in the old locations (Web/img/custom-logo.* and Web/custom-favicon.*) continue to work as a fallback, so existing users are not affected. The new upload directory takes priority when files exist in both locations. Extract Page::findCustomFile() to search multiple directories in priority order, with unit tests covering fallback and precedence logic. Update ManageThemePresenter to use ImageUploadDirectory for uploads and to clean up both old and new locations on removal. Closes: #1049 --- Pages/Page.php | 81 +++++++--- Presenters/Admin/ManageThemePresenter.php | 43 +++-- tests/Pages/PageTest.php | 186 ++++++++++++++++++++++ tpl/Admin/Configuration/change_theme.tpl | 6 +- tpl/Reservation/pdf.tpl | 2 +- tpl/globalheader.tpl | 2 +- tpl/login.tpl | 2 +- tpl/maintenance.tpl | 2 +- 8 files changed, 282 insertions(+), 42 deletions(-) create mode 100644 tests/Pages/PageTest.php diff --git a/Pages/Page.php b/Pages/Page.php index 861ce6648..6cc521d90 100644 --- a/Pages/Page.php +++ b/Pages/Page.php @@ -8,6 +8,7 @@ require_once(ROOT_DIR . 'lib/Common/namespace.php'); require_once(ROOT_DIR . 'lib/Server/namespace.php'); require_once(ROOT_DIR . 'lib/Config/namespace.php'); +require_once(ROOT_DIR . 'lib/Application/Admin/ImageUploadDirectory.php'); use Detection\MobileDetect; @@ -94,16 +95,21 @@ protected function __construct($titleKey = '', $pageDepth = 0) $this->smarty->assign('cssTheme', (Configuration::Instance()->GetKey(ConfigKeys::CSS_THEME) ?? 'default')); - $this->smarty->assign('LogoUrl', 'librebooking.png'); - if (file_exists($this->path . 'img/custom-logo.png')) { - $this->smarty->assign('LogoUrl', 'custom-logo.png'); - } - if (file_exists($this->path . 'img/custom-logo.gif')) { - $this->smarty->assign('LogoUrl', 'custom-logo.gif'); - } - if (file_exists($this->path . 'img/custom-logo.jpg')) { - $this->smarty->assign('LogoUrl', 'custom-logo.jpg'); - } + $imageUploadDirectory = new ImageUploadDirectory(); + $uploadUrl = Configuration::Instance()->GetKey(ConfigKeys::UPLOAD_IMAGE_URL); + $customFileLocations = [ + ['dir' => $imageUploadDirectory->GetDirectory(), 'url' => $uploadUrl], + ['dir' => ROOT_DIR . 'Web/img', 'url' => 'img'], + ]; + + $this->smarty->assign( + 'LogoUrl', + self::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $customFileLocations, + ) ?? 'img/librebooking.png' + ); $this->smarty->assign('CssUrl', 'null-style.css'); if (file_exists($this->path . 'css/custom-style.css')) { @@ -115,19 +121,18 @@ protected function __construct($titleKey = '', $pageDepth = 0) $this->smarty->assign('CssStylingFile', 'styling-plugin.php'); } - $this->smarty->assign('FaviconUrl', 'favicon.ico'); - if (file_exists($this->path . 'custom-favicon.png')) { - $this->smarty->assign('FaviconUrl', 'custom-favicon.png'); - } - if (file_exists($this->path . 'custom-favicon.gif')) { - $this->smarty->assign('FaviconUrl', 'custom-favicon.gif'); - } - if (file_exists($this->path . 'custom-favicon.jpg')) { - $this->smarty->assign('FaviconUrl', 'custom-favicon.jpg'); - } - if (file_exists($this->path . 'custom-favicon.ico')) { - $this->smarty->assign('FaviconUrl', 'custom-favicon.ico'); - } + $faviconLocations = [ + ['dir' => $imageUploadDirectory->GetDirectory(), 'url' => $uploadUrl], + ['dir' => ROOT_DIR . 'Web', 'url' => ''], + ]; + $this->smarty->assign( + 'FaviconUrl', + self::findCustomFile( + baseName: 'custom-favicon', + extensions: ['png', 'gif', 'jpg', 'ico'], + locations: $faviconLocations, + ) ?? 'favicon.ico' + ); $logoUrl = Configuration::Instance()->GetKey(ConfigKeys::HOME_URL); if (empty($logoUrl)) { @@ -153,6 +158,36 @@ protected function __construct($titleKey = '', $pageDepth = 0) $this->Set('AutoScrollToday', Configuration::Instance()->GetKey(ConfigKeys::SCHEDULE_AUTO_SCROLL_TODAY, new BooleanConverter()) ?? true); } + /** + * Search for a custom file across multiple locations, returning the URL-relative path if found. + * + * Locations are checked in order (first match wins). Within each location, + * later extensions overwrite earlier ones. + * + * @param string $baseName File name without extension (e.g. 'custom-logo') + * @param string[] $extensions File extensions to check (e.g. ['png', 'gif', 'jpg']) + * @param array $locations Directories to search, + * each with 'dir' (filesystem path) and 'url' (URL-relative prefix) + * @return string|null URL-relative path to the file, or null if not found + */ + public static function findCustomFile(string $baseName, array $extensions, array $locations): ?string + { + foreach ($locations as $location) { + $result = null; + foreach ($extensions as $ext) { + if (file_exists($location['dir'] . '/' . $baseName . '.' . $ext)) { + $prefix = $location['url'] !== '' ? rtrim($location['url'], '/') . '/' : ''; + $result = $prefix . $baseName . '.' . $ext; + } + } + if ($result !== null) { + return $result; + } + } + + return null; + } + protected function SetTitle($title) { $this->smarty->assign('Title', $title); diff --git a/Presenters/Admin/ManageThemePresenter.php b/Presenters/Admin/ManageThemePresenter.php index 5feb887e1..8c011ab87 100644 --- a/Presenters/Admin/ManageThemePresenter.php +++ b/Presenters/Admin/ManageThemePresenter.php @@ -1,6 +1,7 @@ RemoveLogo(); - $target = ROOT_DIR . 'Web/img/custom-logo.' . $logoFile->Extension(); + $imageUploadDirectory = new ImageUploadDirectory(); + $uploadDir = $imageUploadDirectory->GetDirectory(); + $target = $uploadDir . '/custom-logo.' . $logoFile->Extension(); $copied = copy($logoFile->TemporaryName(), $target); if (!$copied) { Log::Error( @@ -57,7 +60,9 @@ public function UpdateTheme() $this->RemoveFavicon(); - $target = ROOT_DIR . 'Web/custom-favicon.' . $favicon->Extension(); + $imageUploadDirectory = new ImageUploadDirectory(); + $uploadDir = $imageUploadDirectory->GetDirectory(); + $target = $uploadDir . '/custom-favicon.' . $favicon->Extension(); $copied = copy($favicon->TemporaryName(), $target); if (!$copied) { Log::Error( @@ -72,11 +77,18 @@ public function UpdateTheme() public function RemoveLogo() { try { - $targets = glob(ROOT_DIR . 'Web/img/custom-logo.*'); - foreach ($targets as $target) { - $removed = unlink($target); - if (!$removed) { - Log::Error('Could not remove existing logo. Ensure %s is writable.', $target); + $imageUploadDirectory = new ImageUploadDirectory(); + $dirs = [ + $imageUploadDirectory->GetDirectory(), + ROOT_DIR . 'Web/img', + ]; + foreach ($dirs as $dir) { + $targets = glob($dir . '/custom-logo.*'); + foreach ($targets as $target) { + $removed = unlink($target); + if (!$removed) { + Log::Error('Could not remove existing logo. Ensure %s is writable.', $target); + } } } } catch (Exception $ex) { @@ -87,11 +99,18 @@ public function RemoveLogo() public function RemoveFavicon() { try { - $targets = glob(ROOT_DIR . 'Web/custom-favicon.*'); - foreach ($targets as $target) { - $removed = unlink($target); - if (!$removed) { - Log::Error('Could not remove existing favicon. Ensure %s is writable.', $target); + $imageUploadDirectory = new ImageUploadDirectory(); + $dirs = [ + $imageUploadDirectory->GetDirectory(), + ROOT_DIR . 'Web', + ]; + foreach ($dirs as $dir) { + $targets = glob($dir . '/custom-favicon.*'); + foreach ($targets as $target) { + $removed = unlink($target); + if (!$removed) { + Log::Error('Could not remove existing favicon. Ensure %s is writable.', $target); + } } } } catch (Exception $ex) { diff --git a/tests/Pages/PageTest.php b/tests/Pages/PageTest.php new file mode 100644 index 000000000..81e5a490e --- /dev/null +++ b/tests/Pages/PageTest.php @@ -0,0 +1,186 @@ +tempDir = sys_get_temp_dir() . '/librebooking_test_' . uniqid(); + mkdir($this->tempDir . '/new_upload', recursive: true); + mkdir($this->tempDir . '/old_img', recursive: true); + mkdir($this->tempDir . '/old_web', recursive: true); + } + + public function tearDown(): void + { + // Clean up temp files + $this->removeDirectory($this->tempDir); + parent::tearDown(); + } + + private function removeDirectory(string $dir): void + { + if (!is_dir($dir)) { + return; + } + $items = scandir($dir); + foreach ($items as $item) { + if ($item === '.' || $item === '..') { + continue; + } + $path = $dir . '/' . $item; + if (is_dir($path)) { + $this->removeDirectory($path); + } else { + unlink($path); + } + } + rmdir($dir); + } + + public function testFindCustomFileReturnsNullWhenNoFilesExist(): void + { + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ['dir' => $this->tempDir . '/old_img', 'url' => 'img'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $locations, + ); + + $this->assertNull($result); + } + + public function testFindCustomFileFindsFileInFirstLocation(): void + { + touch($this->tempDir . '/new_upload/custom-logo.png'); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ['dir' => $this->tempDir . '/old_img', 'url' => 'img'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $locations, + ); + + $this->assertSame('uploads/images/custom-logo.png', $result); + } + + public function testFindCustomFileFallsBackToSecondLocation(): void + { + touch($this->tempDir . '/old_img/custom-logo.gif'); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ['dir' => $this->tempDir . '/old_img', 'url' => 'img'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $locations, + ); + + $this->assertSame('img/custom-logo.gif', $result); + } + + public function testFindCustomFileFirstLocationWinsWhenBothHaveFiles(): void + { + touch($this->tempDir . '/new_upload/custom-logo.png'); + touch($this->tempDir . '/old_img/custom-logo.jpg'); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ['dir' => $this->tempDir . '/old_img', 'url' => 'img'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $locations, + ); + + $this->assertSame('uploads/images/custom-logo.png', $result); + } + + public function testFindCustomFileLaterExtensionWinsWithinSameLocation(): void + { + touch($this->tempDir . '/new_upload/custom-logo.png'); + touch($this->tempDir . '/new_upload/custom-logo.jpg'); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-logo', + extensions: ['png', 'gif', 'jpg'], + locations: $locations, + ); + + $this->assertSame('uploads/images/custom-logo.jpg', $result); + } + + public function testFindCustomFileHandlesEmptyUrlPrefix(): void + { + touch($this->tempDir . '/old_web/custom-favicon.ico'); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ['dir' => $this->tempDir . '/old_web', 'url' => ''], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-favicon', + extensions: ['png', 'gif', 'jpg', 'ico'], + locations: $locations, + ); + + $this->assertSame('custom-favicon.ico', $result); + } + + #[DataProvider('faviconExtensionProvider')] + public function testFindCustomFileFaviconExtensions(string $extension): void + { + touch($this->tempDir . '/new_upload/custom-favicon.' . $extension); + + $locations = [ + ['dir' => $this->tempDir . '/new_upload', 'url' => 'uploads/images'], + ]; + + $result = Page::findCustomFile( + baseName: 'custom-favicon', + extensions: ['png', 'gif', 'jpg', 'ico'], + locations: $locations, + ); + + $this->assertSame('uploads/images/custom-favicon.' . $extension, $result); + } + + /** + * @return array + */ + public static function faviconExtensionProvider(): array + { + return [ + 'png' => ['png'], + 'gif' => ['gif'], + 'jpg' => ['jpg'], + 'ico' => ['ico'], + ]; + } +} diff --git a/tpl/Admin/Configuration/change_theme.tpl b/tpl/Admin/Configuration/change_theme.tpl index 429bc35cc..30a396b75 100644 --- a/tpl/Admin/Configuration/change_theme.tpl +++ b/tpl/Admin/Configuration/change_theme.tpl @@ -26,9 +26,9 @@

  • {translate key="Logo"} (*.png, *.gif, *.jpg - Recommended height 75px)

    - +
    - {$LogoUrl}
    {$FaviconUrl}
    - +
    {if $CompanyName neq ''} diff --git a/tpl/login.tpl b/tpl/login.tpl index aaccd5320..2b36b178d 100644 --- a/tpl/login.tpl +++ b/tpl/login.tpl @@ -27,7 +27,7 @@
    {if $ShowLoginError} diff --git a/tpl/maintenance.tpl b/tpl/maintenance.tpl index efb4e9163..fe7e8305a 100644 --- a/tpl/maintenance.tpl +++ b/tpl/maintenance.tpl @@ -6,7 +6,7 @@
    - {$Title} + {$Title}