From 975fe1e153c35d7079d06655c43303b04a6502a7 Mon Sep 17 00:00:00 2001
From: Jakub Senko <jakubsenko@gmail.com>
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('<br />', $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 @@
+<?php
+
+namespace vendor5\foo;
+
+class ext extends \phpbb\extension\base
+{
+ static public $enabled;
+
+ public function is_enableable()
+ {
+ return array('Reason 1', 'Reason 2');
+ }
+
+ public function enable_step($old_state)
+ {
+ self::$enabled = true;
+ return self::$enabled;
+ }
+}
\ No newline at end of file
diff --git a/tests/extension/manager_test.php b/tests/extension/manager_test.php
index 231af81a39..3ab0f608d1 100644
--- a/tests/extension/manager_test.php
+++ b/tests/extension/manager_test.php
@@ -36,7 +36,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
public function test_all_available()
{
// barfoo and vendor3/bar should not listed due to missing composer.json. barfoo also has incorrect dir structure.
- $this->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 <jakubsenko@gmail.com>
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 <admin@m-a-styles.de>
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('<br />', $message) : $message;
+ if (empty($message))
+ {
+ $message = $this->user->lang('EXTENSION_NOT_ENABLEABLE');
+ }
+ else if (is_array($message))
+ {
+ $message = implode('<br>', $message);
+ }
+
trigger_error($message . adm_back_link($this->u_action), E_USER_WARNING);
}
}