From 975fe1e153c35d7079d06655c43303b04a6502a7 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Wed, 27 Mar 2019 12:09:55 +0100 Subject: [PATCH 1/3] [ticket/15257] Provide extension not enableable messages PHPBB3-15257 --- phpBB/includes/acp/acp_extensions.php | 22 +++++++++++------- .../console/command/extension/enable.php | 5 +++- phpBB/phpbb/extension/extension_interface.php | 3 ++- tests/extension/ext/vendor5/foo/composer.json | 23 +++++++++++++++++++ tests/extension/ext/vendor5/foo/ext.php | 19 +++++++++++++++ tests/extension/manager_test.php | 2 +- tests/functional/extension_acp_test.php | 9 ++++++-- 7 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 tests/extension/ext/vendor5/foo/composer.json create mode 100644 tests/extension/ext/vendor5/foo/ext.php diff --git a/phpBB/includes/acp/acp_extensions.php b/phpBB/includes/acp/acp_extensions.php index 2929de3c4f..da03b576bf 100644 --- a/phpBB/includes/acp/acp_extensions.php +++ b/phpBB/includes/acp/acp_extensions.php @@ -172,10 +172,8 @@ class acp_extensions } $extension = $this->ext_manager->get_extension($ext_name); - if (!$extension->is_enableable()) - { - trigger_error($this->user->lang['EXTENSION_NOT_ENABLEABLE'] . adm_back_link($this->u_action), E_USER_WARNING); - } + + $this->check_is_enableable($extension->is_enableable()); if ($this->ext_manager->is_enabled($ext_name)) { @@ -209,10 +207,8 @@ class acp_extensions } $extension = $this->ext_manager->get_extension($ext_name); - if (!$extension->is_enableable()) - { - trigger_error($this->user->lang['EXTENSION_NOT_ENABLEABLE'] . adm_back_link($this->u_action), E_USER_WARNING); - } + + $this->check_is_enableable($extension->is_enableable()); try { @@ -727,4 +723,14 @@ class acp_extensions )); } } + + protected function check_is_enableable($enableable) + { + if ($enableable !== true) + { + $message = !empty($enableable) ? $enableable : $this->user->lang('EXTENSION_NOT_ENABLEABLE'); + $message = is_array($message) ? implode('
', $message) : $message; + trigger_error($message . adm_back_link($this->u_action), E_USER_WARNING); + } + } } diff --git a/phpBB/phpbb/console/command/extension/enable.php b/phpBB/phpbb/console/command/extension/enable.php index a6f5b10e86..f007009aa0 100644 --- a/phpBB/phpbb/console/command/extension/enable.php +++ b/phpBB/phpbb/console/command/extension/enable.php @@ -69,7 +69,10 @@ class enable extends command } else { - $io->error($this->user->lang('CLI_EXTENSION_ENABLE_FAILURE', $name)); + $enableable = $this->manager->get_extension($name)->is_enableable(); + $message = !empty($enableable) ? $enableable : $this->user->lang('CLI_EXTENSION_ENABLE_FAILURE'); + $message = is_array($message) ? implode(PHP_EOL, $message) : $message; + $io->error($message, $name); return 1; } } diff --git a/phpBB/phpbb/extension/extension_interface.php b/phpBB/phpbb/extension/extension_interface.php index 6a6b6adb8f..46072d420c 100644 --- a/phpBB/phpbb/extension/extension_interface.php +++ b/phpBB/phpbb/extension/extension_interface.php @@ -22,7 +22,8 @@ interface extension_interface /** * Indicate whether or not the extension can be enabled. * - * @return bool + * @return bool|array True if extension is enableable, array of reasons + * if not, false for generic reason. */ public function is_enableable(); diff --git a/tests/extension/ext/vendor5/foo/composer.json b/tests/extension/ext/vendor5/foo/composer.json new file mode 100644 index 0000000000..ddaee68dc4 --- /dev/null +++ b/tests/extension/ext/vendor5/foo/composer.json @@ -0,0 +1,23 @@ +{ + "name": "vendor5/foo", + "type": "phpbb-extension", + "description": "An example/sample extension to be used for testing purposes in phpBB Development.", + "version": "1.0.0", + "time": "2012-02-15 01:01:01", + "license": "GPL-2.0", + "authors": [{ + "name": "John Smith", + "email": "email@phpbb.com", + "homepage": "http://phpbb.com", + "role": "N/A" + }], + "require": { + "php": ">=5.3" + }, + "extra": { + "display-name": "phpBB Bar Extension", + "soft-require": { + "phpbb/phpbb": "3.3.*@dev" + } + } +} \ No newline at end of file diff --git a/tests/extension/ext/vendor5/foo/ext.php b/tests/extension/ext/vendor5/foo/ext.php new file mode 100644 index 0000000000..729e9d2a33 --- /dev/null +++ b/tests/extension/ext/vendor5/foo/ext.php @@ -0,0 +1,19 @@ +assertEquals(array('vendor/moo', 'vendor2/bar', 'vendor2/foo', 'vendor3/foo', 'vendor4/bar'), array_keys($this->extension_manager->all_available())); + $this->assertEquals(array('vendor/moo', 'vendor2/bar', 'vendor2/foo', 'vendor3/foo', 'vendor4/bar', 'vendor5/foo'), array_keys($this->extension_manager->all_available())); } public function test_all_enabled() diff --git a/tests/functional/extension_acp_test.php b/tests/functional/extension_acp_test.php index b398a73e3f..9a326dba68 100644 --- a/tests/functional/extension_acp_test.php +++ b/tests/functional/extension_acp_test.php @@ -86,7 +86,7 @@ class phpbb_functional_extension_acp_test extends phpbb_functional_test_case $crawler = self::request('GET', 'adm/index.php?i=acp_extensions&mode=main&sid=' . $this->sid); $this->assertCount(1, $crawler->filter('.ext_enabled')); - $this->assertCount(6, $crawler->filter('.ext_disabled')); + $this->assertCount(7, $crawler->filter('.ext_disabled')); $this->assertContains('phpBB Foo Extension', $crawler->filter('.ext_enabled')->eq(0)->text()); $this->assertContainsLang('EXTENSION_DISABLE', $crawler->filter('.ext_enabled')->eq(0)->text()); @@ -165,9 +165,14 @@ class phpbb_functional_extension_acp_test extends phpbb_functional_test_case $crawler = self::request('GET', 'adm/index.php?i=acp_extensions&mode=main&action=enable_pre&ext_name=vendor%2Fmoo&sid=' . $this->sid); $this->assertContains($this->lang('EXTENSION_ENABLE_CONFIRM', 'phpBB Moo Extension'), $crawler->filter('#main')->text()); - // Correctly submit the enable form + // Correctly submit the enable form, default not enableable message $crawler = self::request('GET', 'adm/index.php?i=acp_extensions&mode=main&action=enable_pre&ext_name=vendor3%2Ffoo&sid=' . $this->sid); $this->assertContainsLang('EXTENSION_NOT_ENABLEABLE', $crawler->filter('.errorbox')->text()); + + // Custom reason messages returned by not enableable extension + $crawler = self::request('GET', 'adm/index.php?i=acp_extensions&mode=main&action=enable_pre&ext_name=vendor5%2Ffoo&sid=' . $this->sid); + $this->assertContains('Reason 1', $crawler->filter('.errorbox')->text()); + $this->assertContains('Reason 2', $crawler->filter('.errorbox')->text()); } public function test_disable_pre() From d7c644a7923783f875ed7820d52388c8d9f765fb Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Fri, 12 Apr 2019 12:36:37 +0200 Subject: [PATCH 2/3] [ticket/15257] Address validation issues PHPBB3-15257 --- phpBB/includes/acp/acp_extensions.php | 7 +++++++ tests/extension/ext/vendor5/foo/composer.json | 2 +- tests/extension/ext/vendor5/foo/ext.php | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/acp/acp_extensions.php b/phpBB/includes/acp/acp_extensions.php index da03b576bf..a9cbb8956b 100644 --- a/phpBB/includes/acp/acp_extensions.php +++ b/phpBB/includes/acp/acp_extensions.php @@ -724,6 +724,13 @@ class acp_extensions } } + /** + * Checks whether the extension can be enabled. Triggers error if not. + * Error message can be set by the extension. + * + * @param bool|array $enableable True if extension is enableable, array of reasons + * if not, false for generic reason. + */ protected function check_is_enableable($enableable) { if ($enableable !== true) diff --git a/tests/extension/ext/vendor5/foo/composer.json b/tests/extension/ext/vendor5/foo/composer.json index ddaee68dc4..0fa052e188 100644 --- a/tests/extension/ext/vendor5/foo/composer.json +++ b/tests/extension/ext/vendor5/foo/composer.json @@ -20,4 +20,4 @@ "phpbb/phpbb": "3.3.*@dev" } } -} \ No newline at end of file +} diff --git a/tests/extension/ext/vendor5/foo/ext.php b/tests/extension/ext/vendor5/foo/ext.php index 729e9d2a33..6bf03f001c 100644 --- a/tests/extension/ext/vendor5/foo/ext.php +++ b/tests/extension/ext/vendor5/foo/ext.php @@ -16,4 +16,4 @@ class ext extends \phpbb\extension\base self::$enabled = true; return self::$enabled; } -} \ No newline at end of file +} From e54aa94e7714483f0f5ef8c18e21317a1f0524f6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 30 Sep 2019 21:21:50 +0200 Subject: [PATCH 3/3] [ticket/15257] Improve readability of check_is_enableable PHPBB3-15257 --- phpBB/includes/acp/acp_extensions.php | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/phpBB/includes/acp/acp_extensions.php b/phpBB/includes/acp/acp_extensions.php index a9cbb8956b..6ac70ce3a8 100644 --- a/phpBB/includes/acp/acp_extensions.php +++ b/phpBB/includes/acp/acp_extensions.php @@ -173,7 +173,7 @@ class acp_extensions $extension = $this->ext_manager->get_extension($ext_name); - $this->check_is_enableable($extension->is_enableable()); + $this->check_is_enableable($extension); if ($this->ext_manager->is_enabled($ext_name)) { @@ -208,7 +208,7 @@ class acp_extensions $extension = $this->ext_manager->get_extension($ext_name); - $this->check_is_enableable($extension->is_enableable()); + $this->check_is_enableable($extension); try { @@ -728,15 +728,22 @@ class acp_extensions * Checks whether the extension can be enabled. Triggers error if not. * Error message can be set by the extension. * - * @param bool|array $enableable True if extension is enableable, array of reasons - * if not, false for generic reason. + * @param \phpbb\extension\extension_interface $extension Extension to check */ - protected function check_is_enableable($enableable) + protected function check_is_enableable(\phpbb\extension\extension_interface $extension) { - if ($enableable !== true) + $message = $extension->is_enableable(); + if ($message !== true) { - $message = !empty($enableable) ? $enableable : $this->user->lang('EXTENSION_NOT_ENABLEABLE'); - $message = is_array($message) ? implode('
', $message) : $message; + if (empty($message)) + { + $message = $this->user->lang('EXTENSION_NOT_ENABLEABLE'); + } + else if (is_array($message)) + { + $message = implode('
', $message); + } + trigger_error($message . adm_back_link($this->u_action), E_USER_WARNING); } }