From e3e425506fbe7a063f5f511e5ae0a2a0e82773bb Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sun, 19 Jan 2025 14:51:50 -0800 Subject: [PATCH 1/4] Make icon management dependent on local storage provider --- adm/style/acp_pwakit.html | 64 +++++++++++++++------------- controller/admin_controller.php | 3 ++ helper/helper.php | 10 +++++ language/en/acp_pwa.php | 1 + tests/unit/admin_controller_test.php | 2 + tests/unit/helper_test.php | 5 +++ 6 files changed, 55 insertions(+), 30 deletions(-) diff --git a/adm/style/acp_pwakit.html b/adm/style/acp_pwakit.html index afab5ff..93356b8 100644 --- a/adm/style/acp_pwakit.html +++ b/adm/style/acp_pwakit.html @@ -69,36 +69,40 @@

{{ lang('ACP_PWA_KIT_APP_ICONS') }}

{{ lang('ACP_PWA_KIT_APP_ICONS_EXPLAIN') }}

{{ lang('ACP_PWA_KIT_LEGEND_ICONS') }} -
-

{{ lang('ACP_PWA_IMG_UPLOAD_EXPLAIN', PWA_IMAGES_DIR, U_STORAGE_SETTINGS) }}
-
- - - -
-
-
-

{{ lang('ACP_PWA_KIT_ICONS_EXPLAIN') }}
-
- {% for icon in PWA_KIT_ICONS %} - {% set iconName = icon.src|split('/')|last %} -

- - {{ iconName|e( - -
- {{ iconName }}
{{ icon.sizes }} -

- {% else %} - {{ lang('ACP_PWA_KIT_NO_ICONS') }} - {% endfor %} -
-
+ {% if S_STORAGE_LOCAL %} +
+

{{ lang('ACP_PWA_IMG_UPLOAD_EXPLAIN', PWA_IMAGES_DIR, U_STORAGE_SETTINGS) }}
+
+ + + +
+
+
+

{{ lang('ACP_PWA_KIT_ICONS_EXPLAIN') }}
+
+ {% for icon in PWA_KIT_ICONS %} + {% set iconName = icon.src|split('/')|last %} +

+ + {{ iconName|e( + +
+ {{ iconName }}
{{ icon.sizes }} +

+ {% else %} + {{ lang('ACP_PWA_KIT_NO_ICONS') }} + {% endfor %} +
+
+ {% else %} +

{{ lang('ACP_PWA_STORAGE_INCOMPATIBLE', U_STORAGE_SETTINGS) }}

+ {% endif %}
diff --git a/controller/admin_controller.php b/controller/admin_controller.php index 9009b25..c5ea12c 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -189,6 +189,9 @@ protected function display_settings(): void 'PWA_IMAGES_DIR' => $this->helper->get_storage_path(), 'PWA_KIT_ICONS' => $this->helper->get_icons($this->phpbb_root_path), 'STYLES' => $this->get_styles(), + + 'S_STORAGE_LOCAL' => $this->helper->is_storage_compatible(), + 'U_BOARD_SETTINGS' => append_sid("{$this->phpbb_admin_path}index.$this->php_ext", 'i=acp_board&mode=settings'), 'U_STORAGE_SETTINGS'=> append_sid("{$this->phpbb_admin_path}index.$this->php_ext", 'i=acp_storage&mode=settings'), 'U_ACTION' => $this->u_action, diff --git a/helper/helper.php b/helper/helper.php index 8520cdc..5db4408 100644 --- a/helper/helper.php +++ b/helper/helper.php @@ -52,6 +52,16 @@ public function __construct(ext_manager $extension_manager, FastImageSize $image $this->root_path = $root_path; } + /** + * Storage compatibility requires the local provider + * + * @return bool + */ + public function is_storage_compatible(): bool + { + return str_ends_with($this->storage_helper->get_current_provider($this->storage->get_name()), '\local'); + } + /** * Get the storage path for the current storage definition * diff --git a/language/en/acp_pwa.php b/language/en/acp_pwa.php index 8f20f39..edab16f 100644 --- a/language/en/acp_pwa.php +++ b/language/en/acp_pwa.php @@ -54,6 +54,7 @@ 'ACP_PWA_KIT_APP_ICONS_EXPLAIN' => 'Used to specify the touch-icon for your web application. This icon will be used when your web application is added to the home screen. Multiple sizes are preferred for compatibility with various devices. Note that when adding or removing icons, you may need to clear your web app/browser cache to see them update.', 'ACP_PWA_KIT_ICONS' => 'Web application icons', 'ACP_PWA_KIT_ICONS_EXPLAIN' => 'PNG image files that represent your web application. Multiple sizes are preferred for compatibility with various devices.', + 'ACP_PWA_STORAGE_INCOMPATIBLE' => 'To manage icons you must set Web app icons storage to Local in General » Storage settings.', 'ACP_PWA_KIT_NO_ICONS' => 'No icons are available. Click Upload to add new icons or click Resync to find existing icons that were previously uploaded. Recommended PNG sizes include 180x180, 192x192 and 512x512.', 'ACP_PWA_IMG_UPLOAD' => 'Upload web application icons', 'ACP_PWA_IMG_UPLOAD_EXPLAIN' => 'Upload PNG images. Images are currently being stored in %1$s. This can be changed at any time to another location in General » Storage settings.', diff --git a/tests/unit/admin_controller_test.php b/tests/unit/admin_controller_test.php index e8a78be..e3f5740 100644 --- a/tests/unit/admin_controller_test.php +++ b/tests/unit/admin_controller_test.php @@ -91,6 +91,7 @@ protected function setUp(): void $this->helper = $this->getMockBuilder(helper::class) ->disableOriginalConstructor() ->getMock(); + $this->helper->method('is_storage_compatible')->willReturn(true); $this->helper->method('get_storage_path')->willReturn('images/site_icons'); $this->helper->method('get_icons')->willReturn([]); @@ -247,6 +248,7 @@ public function test_display_settings($configs, $expected) 'PWA_IMAGES_DIR' => 'images/site_icons', 'PWA_KIT_ICONS' => [], 'STYLES' => $expected_style_data, + 'S_STORAGE_LOCAL' => true, 'U_BOARD_SETTINGS' => "{$this->phpbb_root_path}adm/index.php?i=acp_board&mode=settings", 'U_STORAGE_SETTINGS'=> "{$this->phpbb_root_path}adm/index.php?i=acp_storage&mode=settings", 'U_ACTION' => '', diff --git a/tests/unit/helper_test.php b/tests/unit/helper_test.php index 7a3824d..fba936e 100644 --- a/tests/unit/helper_test.php +++ b/tests/unit/helper_test.php @@ -153,6 +153,11 @@ public function test_get_tracked_files() $this->assertEquals(['foo.png'], $this->storage->get_tracked_files()); } + public function test_is_storage_compatible() + { + $this->assertTrue($this->helper->is_storage_compatible()); + } + public function test_get_storage_path() { $this->assertEquals($this->storage_path, $this->helper->get_storage_path()); From a2151c22e18c950f6a2b292caa080c639bd3954d Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 20 Jan 2025 07:04:41 -0800 Subject: [PATCH 2/4] Test actual provider name --- config/services.yml | 1 + helper/helper.php | 14 ++++++++++++-- tests/unit/helper_test.php | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/config/services.yml b/config/services.yml index 065daca..1a5cce8 100644 --- a/config/services.yml +++ b/config/services.yml @@ -15,6 +15,7 @@ services: - '@upload_imagesize' - '@phpbb.pwakit.storage' - '@storage.helper' + - '@storage.provider_collection' - '%core.root_path%' phpbb.pwakit.upload: diff --git a/helper/helper.php b/helper/helper.php index 5db4408..35e9546 100644 --- a/helper/helper.php +++ b/helper/helper.php @@ -11,6 +11,7 @@ namespace phpbb\pwakit\helper; use FastImageSize\FastImageSize; +use phpbb\di\service_collection; use phpbb\exception\runtime_exception; use phpbb\extension\manager as ext_manager; use phpbb\pwakit\storage\storage; @@ -31,6 +32,9 @@ class helper /** @var storage_helper */ protected storage_helper $storage_helper; + /** @var service_collection $provider_collection */ + protected service_collection $provider_collection; + /** @var string */ protected string $root_path; @@ -41,14 +45,16 @@ class helper * @param FastImageSize $imagesize * @param storage $storage * @param storage_helper $storage_helper + * @param service_collection $provider_collection * @param string $root_path */ - public function __construct(ext_manager $extension_manager, FastImageSize $imagesize, storage $storage, storage_helper $storage_helper, string $root_path) + public function __construct(ext_manager $extension_manager, FastImageSize $imagesize, storage $storage, storage_helper $storage_helper, service_collection $provider_collection, string $root_path) { $this->extension_manager = $extension_manager; $this->imagesize = $imagesize; $this->storage = $storage; $this->storage_helper = $storage_helper; + $this->provider_collection = $provider_collection; $this->root_path = $root_path; } @@ -59,7 +65,11 @@ public function __construct(ext_manager $extension_manager, FastImageSize $image */ public function is_storage_compatible(): bool { - return str_ends_with($this->storage_helper->get_current_provider($this->storage->get_name()), '\local'); + $current_provider = $this->provider_collection->get_by_class( + $this->storage_helper->get_current_provider($this->storage->get_name()) + ); + + return $current_provider && $current_provider->get_name() === 'local'; } /** diff --git a/tests/unit/helper_test.php b/tests/unit/helper_test.php index fba936e..97f44be 100644 --- a/tests/unit/helper_test.php +++ b/tests/unit/helper_test.php @@ -128,6 +128,7 @@ protected function setUp(): void $provider_collection, $adapter_collection ), + $provider_collection, $phpbb_root_path ); From 8c5a6b5cb61f951832f0d9796f66eac0c750f8d5 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 20 Jan 2025 07:05:02 -0800 Subject: [PATCH 3/4] Rename method --- controller/admin_controller.php | 2 +- helper/helper.php | 2 +- tests/unit/admin_controller_test.php | 2 +- tests/unit/helper_test.php | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/controller/admin_controller.php b/controller/admin_controller.php index c5ea12c..3d2bdf4 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -190,7 +190,7 @@ protected function display_settings(): void 'PWA_KIT_ICONS' => $this->helper->get_icons($this->phpbb_root_path), 'STYLES' => $this->get_styles(), - 'S_STORAGE_LOCAL' => $this->helper->is_storage_compatible(), + 'S_STORAGE_LOCAL' => $this->helper->is_storage_local(), 'U_BOARD_SETTINGS' => append_sid("{$this->phpbb_admin_path}index.$this->php_ext", 'i=acp_board&mode=settings'), 'U_STORAGE_SETTINGS'=> append_sid("{$this->phpbb_admin_path}index.$this->php_ext", 'i=acp_storage&mode=settings'), diff --git a/helper/helper.php b/helper/helper.php index 35e9546..99669a8 100644 --- a/helper/helper.php +++ b/helper/helper.php @@ -63,7 +63,7 @@ public function __construct(ext_manager $extension_manager, FastImageSize $image * * @return bool */ - public function is_storage_compatible(): bool + public function is_storage_local(): bool { $current_provider = $this->provider_collection->get_by_class( $this->storage_helper->get_current_provider($this->storage->get_name()) diff --git a/tests/unit/admin_controller_test.php b/tests/unit/admin_controller_test.php index e3f5740..c2179f8 100644 --- a/tests/unit/admin_controller_test.php +++ b/tests/unit/admin_controller_test.php @@ -91,7 +91,7 @@ protected function setUp(): void $this->helper = $this->getMockBuilder(helper::class) ->disableOriginalConstructor() ->getMock(); - $this->helper->method('is_storage_compatible')->willReturn(true); + $this->helper->method('is_storage_local')->willReturn(true); $this->helper->method('get_storage_path')->willReturn('images/site_icons'); $this->helper->method('get_icons')->willReturn([]); diff --git a/tests/unit/helper_test.php b/tests/unit/helper_test.php index 97f44be..42106ea 100644 --- a/tests/unit/helper_test.php +++ b/tests/unit/helper_test.php @@ -154,9 +154,9 @@ public function test_get_tracked_files() $this->assertEquals(['foo.png'], $this->storage->get_tracked_files()); } - public function test_is_storage_compatible() + public function test_is_storage_local() { - $this->assertTrue($this->helper->is_storage_compatible()); + $this->assertTrue($this->helper->is_storage_local()); } public function test_get_storage_path() From eb068eb05c3f0bbe35d46f4fbb5b7c2f17aa7a06 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 20 Jan 2025 13:53:45 -0800 Subject: [PATCH 4/4] Catch exceptions --- helper/helper.php | 16 ++++++++++++---- tests/unit/helper_test.php | 6 ++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/helper/helper.php b/helper/helper.php index 99669a8..5d2a39e 100644 --- a/helper/helper.php +++ b/helper/helper.php @@ -17,6 +17,7 @@ use phpbb\pwakit\storage\storage; use phpbb\storage\exception\storage_exception; use phpbb\storage\helper as storage_helper; +use RuntimeException; class helper { @@ -65,11 +66,18 @@ public function __construct(ext_manager $extension_manager, FastImageSize $image */ public function is_storage_local(): bool { - $current_provider = $this->provider_collection->get_by_class( - $this->storage_helper->get_current_provider($this->storage->get_name()) - ); + try + { + $storage_name = $this->storage->get_name(); + $provider_class = $this->storage_helper->get_current_provider($storage_name); + $current_provider = $this->provider_collection->get_by_class($provider_class); - return $current_provider && $current_provider->get_name() === 'local'; + return $current_provider && $current_provider->get_name() === 'local'; + } + catch (RuntimeException) + { + return false; + } } /** diff --git a/tests/unit/helper_test.php b/tests/unit/helper_test.php index 42106ea..db65cbf 100644 --- a/tests/unit/helper_test.php +++ b/tests/unit/helper_test.php @@ -159,6 +159,12 @@ public function test_is_storage_local() $this->assertTrue($this->helper->is_storage_local()); } + public function test_is_storage_not_local() + { + $this->config->set('storage\phpbb_pwakit\provider', 'foo'); + $this->assertFalse($this->helper->is_storage_local()); + } + public function test_get_storage_path() { $this->assertEquals($this->storage_path, $this->helper->get_storage_path());