From 04d825ec2dd24f2e486f11eae5fe2c1846bbcd7c Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Thu, 29 Nov 2018 01:50:46 +0100 Subject: [PATCH 01/14] [ticket/15886] Enhance group helper PHPBB3-15886 --- phpBB/config/default/container/services.yml | 8 + phpBB/phpbb/group/helper.php | 241 +++++++++++++++++++- 2 files changed, 247 insertions(+), 2 deletions(-) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index 9bb1d673f4..cd1d872ae0 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -122,7 +122,15 @@ services: group_helper: class: phpbb\group\helper arguments: + - '@auth' + - '@cache' + - '@config' - '@language' + - '@dispatcher' + - '@path_helper' + - '@user' + - '%core.root_path%' + - '%core.php_ext%' log: class: phpbb\log\log diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 5befddfc53..14c7891c82 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -15,17 +15,68 @@ namespace phpbb\group; class helper { + /** @var \phpbb\auth\auth */ + protected $auth; + + /** @var \phpbb\cache\service */ + protected $cache; + + /** @var \phpbb\config\config */ + protected $config; + /** @var \phpbb\language\language */ protected $language; + /** @var \phpbb\event\dispatcher_interface */ + protected $phpbb_dispatcher; + + /** @var \phpbb\path_helper */ + protected $phpbb_path_helper; + + /** @var \phpbb\user */ + protected $user; + + /** @var string phpBB root path */ + protected $phpbb_root_path; + + /** @var string PHP file extension */ + protected $php_ext; + + /** @var array Default group name string templates */ + protected $default_templates = array(); + /** * Constructor * - * @param \phpbb\language\language $language Language object + * @param \phpbb\auth\auth $auth Authentication object + * @param \phpbb\cache\service $cache Cache object + * @param \phpbb\config\config $config Configuration object + * @param \phpbb\language\language $language Language object + * @param \phpbb\event\dispatcher_interface $phpbb_dispatcher Event dispatcher object + * @param \phpbb\path_helper $phpbb_path_helper Path helper object + * @param \phpbb\user $user User object + * @param string $phpbb_root_path phpBB root path + * @param string $php_ext PHP file extension */ - public function __construct(\phpbb\language\language $language) + public function __construct(\phpbb\auth\auth $auth, \phpbb\cache\service $cache, \phpbb\config\config $config, \phpbb\language\language $language, \phpbb\event\dispatcher_interface $phpbb_dispatcher, \phpbb\path_helper $phpbb_path_helper, \phpbb\user $user, $phpbb_root_path, $php_ext) { + $this->auth = $auth; + $this->cache = $cache; + $this->config = $config; $this->language = $language; + $this->phpbb_dispatcher = $phpbb_dispatcher; + $this->phpbb_path_helper = $phpbb_path_helper; + $this->user = $user; + + $this->phpbb_root_path = $phpbb_root_path; + + $this->default_templates = array( + 'base_url' => append_sid("{$phpbb_root_path}memberlist.$php_ext", 'mode=group&g={GROUP_ID}'), + 'tpl_noprofile' => '{GROUP_NAME}', + 'tpl_noprofile_colour' => '{GROUP_NAME}', + 'tpl_profile' => '{GROUP_NAME}', + 'tpl_profile_colour' => '{GROUP_NAME}', + ); } /** @@ -37,4 +88,190 @@ class helper { return $this->language->is_set('G_' . utf8_strtoupper($group_name)) ? $this->language->lang('G_' . utf8_strtoupper($group_name)) : $group_name; } + + /** + * Get group name details for placing into templates. + * + * @param string $mode Can be profile (for getting an url to the profile), group_name (for obtaining the group name), colour (for obtaining the group colour), full (for obtaining a html string representing a coloured link to the group's profile) or no_profile (the same as full but forcing no profile link) + * @param int $group_id The group id + * @param string $group_name The group name + * @param string $group_colour The group colour + * @param mixed $custom_profile_url optional parameter to specify a profile url. The group id gets appended to this url as &g={group_id} + * + * @return string A string consisting of what is wanted based on $mode. + */ + public function get_name_string($mode, $group_id, $group_name, $group_colour = '', $custom_profile_url = false) + { + // This switch makes sure we only run code required for the mode + switch ($mode) + { + case 'full': + case 'no_profile': + case 'colour': + + // Build correct group colour + $group_colour = ($group_colour) ? '#' . $group_colour : ''; + + // Return colour + if ($mode === 'colour') + { + $group_name_string = $group_colour; + break; + } + + // no break; + + case 'group_name': + + // Build correct group name + $group_name = $this->get_name($group_name); + + // Return group name + if ($mode === 'group_name') + { + $group_name_string = $group_name; + break; + } + + // no break; + + case 'profile': + + // Build correct profile url - only show if not anonymous and permission to view profile if registered user + // For anonymous the link leads to a login page. + if ($group_id && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) + { + $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $this->default_templates['base_url']); + } + else + { + $profile_url = ''; + } + + // Return profile + if ($mode === 'profile') + { + $group_name_string = $profile_url; + break; + } + + // no break; + } + + if (!isset($group_name_string)) + { + if (($mode === 'full' && !$profile_url) || $mode === 'no_profile') + { + $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $this->default_templates['tpl_noprofile'] : $this->default_templates['tpl_noprofile_colour']); + } + else + { + $group_name_string = str_replace(array('{PROFILE_URL}', '{GROUP_COLOUR}', '{GROUP_NAME}'), array($profile_url, $group_colour, $group_name), (!$group_colour) ? $this->default_templates['tpl_profile'] : $this->default_templates['tpl_profile_colour']); + } + } + + /** + * Use this event to change the output of the group name + * + * @event core.modify_group_name_string + * @var string mode profile|group_name|colour|full|no_profile + * @var int group_id The group identifier + * @var string group_name The group name + * @var string group_colour The group colour + * @var string custom_profile_url Optional parameter to specify a profile url. + * @var string group_name_string The string that has been generated + * @var array _profile_cache Array of original return templates + * @since 3.2.5-RC1 + */ + $vars = array( + 'mode', + 'group_id', + 'group_name', + 'group_colour', + 'custom_profile_url', + 'group_name_string', + '_profile_cache', + ); + extract($this->phpbb_dispatcher->trigger_event('core.modify_group_name_string', compact($vars))); + + return $group_name_string; + } + + /** + * Get group rank title and image + * + * @param array $group_data the current stored group data + * + * @return array An associative array containing the rank title (title), + * the rank image as full img tag (img) and the rank image source (img_src) + */ + public function get_rank($group_data) + { + $group_rank_data = array( + 'title' => null, + 'img' => null, + 'img_src' => null, + ); + + /** + * Preparing a group's rank before displaying + * + * @event core.get_group_rank_before + * @var array group_data Array with group's data + * @since 3.2.5-RC1 + */ + + $vars = array('group_data'); + extract($this->phpbb_dispatcher->trigger_event('core.get_group_rank_before', compact($vars))); + + if (!empty($group_data['group_rank'])) + { + // Only obtain ranks if group rank is set + $ranks = $this->cache->obtain_ranks(); + + if (isset($ranks['special'][$group_data['group_rank']])) + { + $rank = $ranks['special'][$group_data['group_rank']]; + + $group_rank_data['title'] = $rank['rank_title']; + + $group_rank_data['img_src'] = (!empty($rank['rank_image'])) ? $this->phpbb_path_helper->update_web_root_path($this->phpbb_root_path . $this->config['ranks_path'] . '/' . $rank['rank_image']) : ''; + + $group_rank_data['img'] = (!empty($rank['rank_image'])) ? '' . $rank['rank_title'] . '' : ''; + } + } + + /** + * Modify a group's rank before displaying + * + * @event core.get_group_rank_after + * @var array group_data Array with group's data + * @var array group_rank_data Group rank data + * @since 3.2.5-RC1 + */ + + $vars = array( + 'group_data', + 'group_rank_data', + ); + extract($this->phpbb_dispatcher->trigger_event('core.get_group_rank_after', compact($vars))); + + return $group_rank_data; + } + + /** + * Get group avatar. + * Wrapper function for phpbb_get_group_avatar() + * + * @param array $group_row Row from the groups table + * @param string $alt Optional language string for alt tag within image, can be a language key or text + * @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP + * @param bool $lazy If true, will be lazy loaded (requires JS) + * + * @return string Avatar html + */ + function get_avatar($group_row, $alt = 'GROUP_AVATAR', $ignore_config = false, $lazy = false) + { + return phpbb_get_group_avatar($group_row, $alt, $ignore_config, $lazy); + } } From b6f6930eb56b9fd64f200a7b0e6d8cd28f0e73f6 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Thu, 29 Nov 2018 19:56:24 +0100 Subject: [PATCH 02/14] [ticket/15886] Group helper functions PHPBB3-15886 --- phpBB/phpbb/group/helper.php | 44 ++++--- tests/group/helper_get_name_string_test.php | 114 ++++++++++++++++++ tests/group/helper_get_name_test.php | 31 +++++ tests/group/helper_get_rank_test.php | 43 +++++++ tests/group/helper_test.php | 68 ----------- tests/group/helper_test_case.php | 123 ++++++++++++++++++++ 6 files changed, 332 insertions(+), 91 deletions(-) create mode 100644 tests/group/helper_get_name_string_test.php create mode 100644 tests/group/helper_get_name_test.php create mode 100644 tests/group/helper_get_rank_test.php delete mode 100644 tests/group/helper_test.php create mode 100644 tests/group/helper_test_case.php diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 14c7891c82..fc657a59a2 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -15,25 +15,25 @@ namespace phpbb\group; class helper { - /** @var \phpbb\auth\auth */ + /** @var \phpbb\auth\auth */ protected $auth; - /** @var \phpbb\cache\service */ + /** @var \phpbb\cache\service */ protected $cache; - /** @var \phpbb\config\config */ + /** @var \phpbb\config\config */ protected $config; - /** @var \phpbb\language\language */ + /** @var \phpbb\language\language */ protected $language; - /** @var \phpbb\event\dispatcher_interface */ + /** @var \phpbb\event\dispatcher_interface */ protected $phpbb_dispatcher; - /** @var \phpbb\path_helper */ + /** @var \phpbb\path_helper */ protected $phpbb_path_helper; - /** @var \phpbb\user */ + /** @var \phpbb\user */ protected $user; /** @var string phpBB root path */ @@ -42,9 +42,6 @@ class helper /** @var string PHP file extension */ protected $php_ext; - /** @var array Default group name string templates */ - protected $default_templates = array(); - /** * Constructor * @@ -58,7 +55,7 @@ class helper * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP file extension */ - public function __construct(\phpbb\auth\auth $auth, \phpbb\cache\service $cache, \phpbb\config\config $config, \phpbb\language\language $language, \phpbb\event\dispatcher_interface $phpbb_dispatcher, \phpbb\path_helper $phpbb_path_helper, \phpbb\user $user, $phpbb_root_path, $php_ext) + public function __construct(\phpbb\auth\auth $auth, \phpbb\cache\service $cache, \phpbb\config\config $config, \phpbb\language\language $language, \phpbb\event\dispatcher_interface $phpbb_dispatcher, \phpbb\path_helper $phpbb_path_helper, \phpbb\user $user, $phpbb_root_path, $php_ext) { $this->auth = $auth; $this->cache = $cache; @@ -69,14 +66,7 @@ class helper $this->user = $user; $this->phpbb_root_path = $phpbb_root_path; - - $this->default_templates = array( - 'base_url' => append_sid("{$phpbb_root_path}memberlist.$php_ext", 'mode=group&g={GROUP_ID}'), - 'tpl_noprofile' => '{GROUP_NAME}', - 'tpl_noprofile_colour' => '{GROUP_NAME}', - 'tpl_profile' => '{GROUP_NAME}', - 'tpl_profile_colour' => '{GROUP_NAME}', - ); + $this->php_ext = $php_ext; } /** @@ -102,6 +92,14 @@ class helper */ public function get_name_string($mode, $group_id, $group_name, $group_colour = '', $custom_profile_url = false) { + $_profile_cache = array( + 'base_url' => append_sid("{$this->phpbb_root_path}memberlist.{$this->php_ext}", 'mode=group&g={GROUP_ID}'), + 'tpl_noprofile' => '{GROUP_NAME}', + 'tpl_noprofile_colour' => '{GROUP_NAME}', + 'tpl_profile' => '{GROUP_NAME}', + 'tpl_profile_colour' => '{GROUP_NAME}', + ); + // This switch makes sure we only run code required for the mode switch ($mode) { @@ -141,7 +139,7 @@ class helper // For anonymous the link leads to a login page. if ($group_id && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) { - $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $this->default_templates['base_url']); + $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $_profile_cache['base_url']); } else { @@ -162,11 +160,11 @@ class helper { if (($mode === 'full' && !$profile_url) || $mode === 'no_profile') { - $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $this->default_templates['tpl_noprofile'] : $this->default_templates['tpl_noprofile_colour']); + $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $_profile_cache['tpl_noprofile'] : $_profile_cache['tpl_noprofile_colour']); } else { - $group_name_string = str_replace(array('{PROFILE_URL}', '{GROUP_COLOUR}', '{GROUP_NAME}'), array($profile_url, $group_colour, $group_name), (!$group_colour) ? $this->default_templates['tpl_profile'] : $this->default_templates['tpl_profile_colour']); + $group_name_string = str_replace(array('{PROFILE_URL}', '{GROUP_COLOUR}', '{GROUP_NAME}'), array($profile_url, $group_colour, $group_name), (!$group_colour) ? $_profile_cache['tpl_profile'] : $_profile_cache['tpl_profile_colour']); } } @@ -202,7 +200,7 @@ class helper * * @param array $group_data the current stored group data * - * @return array An associative array containing the rank title (title), + * @return array An associative array containing the rank title (title), * the rank image as full img tag (img) and the rank image source (img_src) */ public function get_rank($group_data) diff --git a/tests/group/helper_get_name_string_test.php b/tests/group/helper_get_name_string_test.php new file mode 100644 index 0000000000..7ea0f156e4 --- /dev/null +++ b/tests/group/helper_get_name_string_test.php @@ -0,0 +1,114 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +require_once dirname(__FILE__) . '/helper_test_case.php'; + +class phpbb_group_helper_get_name_string_test extends phpbb_group_helper_test_case +{ + + public function get_name_string_profile_data() + { + global $phpbb_root_path, $phpEx; + + return array( + array(0, 'Non existing group', '', false, ''), + array(2, 'Administrators', 'AA0000', false, "{$phpbb_root_path}memberlist.$phpEx?mode=group&g=2"), + array(42, 'Example Group', '', 'http://www.example.org/group.php?mode=show', 'http://www.example.org/group.php?mode=show&g=42'), + ); + } + + /** + * @dataProvider get_name_string_profile_data + */ + public function test_get_name_string_profile($group_id, $group_name, $group_colour, $custom_profile_url, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_name_string('profile', $group_id, $group_name, $group_colour, $custom_profile_url)); + } + + public function get_name_string_group_name_data() + { + return array( + // Should be fine + array(0, 'Bots', 'AA0000', false, 'Bots'), + array(1, 'new_group', '', false, 'Some new group'), + array(2, 'group_with_ümlauts', '', 'http://www.example.org/group.php?mode=show', 'Should work'), + + // Should fail and thus return the same + array(3, 'not_uppercase', 'FFFFFF', false, 'not_uppercase'), + array(4, 'Awesome group', '', false, 'Awesome group'), + ); + } + + /** + * @dataProvider get_name_string_group_name_data + */ + public function test_get_name_string_group_name($group_id, $group_name, $group_colour, $custom_profile_url, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_name_string('group_name', $group_id, $group_name, $group_colour, $custom_profile_url)); + } + + public function get_name_string_colour_data() + { + return array( + array(0, '', '', false, ''), + array(0, '', 'F0F0F0', false, '#F0F0F0'), + array(1, 'Guests', '000000', false, '#000000'), + array(2, 'Administrators', '', false, ''), + ); + } + + /** + * @dataProvider get_name_string_colour_data + */ + public function test_get_name_string_colour($group_id, $group_name, $group_colour, $custom_profile_url, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_name_string('colour', $group_id, $group_name, $group_colour, $custom_profile_url)); + } + + public function get_name_string_full_data() + { + global $phpbb_root_path, $phpEx; + + return array( + array(0, 'Bots', '000000', false, 'Bots'), + array(7, 'new_group', 'FFA500', false, 'Some new group'), + array(14, 'Awesome group', '', 'http://www.example.org/group.php?mode=show', 'Awesome group'), + ); + } + + /** + * @dataProvider get_name_string_full_data + */ + public function test_get_name_string_full($group_id, $group_name, $group_colour, $custom_profile_url, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_name_string('full', $group_id, $group_name, $group_colour, $custom_profile_url)); + } + + public function get_name_string_no_profile_data() + { + return array( + array(0, 'Bots', '000000', false, 'Bots'), + array(1, 'new_group', '', false, 'Some new group'), + arraY(2, 'not_uppercase', 'FF0000', false, 'not_uppercase'), + array(5, 'Awesome group', '', 'http://www.example.org/group.php?mode=show', 'Awesome group'), + ); + } + + /** + * @dataProvider get_name_string_no_profile_data + */ + public function test_get_name_string_no_profile($group_id, $group_name, $group_colour, $custom_profile_url, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_name_string('no_profile', $group_id, $group_name, $group_colour, $custom_profile_url)); + } +} diff --git a/tests/group/helper_get_name_test.php b/tests/group/helper_get_name_test.php new file mode 100644 index 0000000000..b39b2cbedd --- /dev/null +++ b/tests/group/helper_get_name_test.php @@ -0,0 +1,31 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +require_once dirname(__FILE__) . '/helper_test_case.php'; + +class phpbb_group_helper_get_name_test extends phpbb_group_helper_test_case +{ + public function test_get_name() + { + // They should be totally fine + $this->assertEquals('Bots', $this->group_helper->get_name('Bots')); + $this->assertEquals('Some new group', $this->group_helper->get_name('new_group')); + $this->assertEquals('Should work', $this->group_helper->get_name('group_with_ümlauts')); + + // This should fail (obviously) + $this->assertNotEquals('The key does not contain uppercase letters', $this->group_helper->get_name('not_uppercase')); + + // The key doesn't exist so just return group name... + $this->assertEquals('Awesome group', $this->group_helper->get_name('Awesome group')); + } +} diff --git a/tests/group/helper_get_rank_test.php b/tests/group/helper_get_rank_test.php new file mode 100644 index 0000000000..5efd8ad95e --- /dev/null +++ b/tests/group/helper_get_rank_test.php @@ -0,0 +1,43 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +require_once dirname(__FILE__) . '/helper_test_case.php'; + +class phpbb_group_helper_get_rank_test extends phpbb_group_helper_test_case +{ + public function get_rank_data() + { + global $phpbb_root_path; + + return array( + array( + array('group_id' => 0, 'group_rank' => 1), + array( + 'title' => 'Site admin', + 'img' => 'Site admin', + 'img_src' => $phpbb_root_path . 'images/ranks/siteadmin.png', + ) + ), + array(array('group_id' => 1, 'group_rank' => 0), array('title' => null, 'img' => null, 'img_src' => null)), + array(array('group_id' => 2, 'group_rank' => 2), array('title' => 'Test member', 'img' => '', 'img_src' => '')), + ); + } + + /** + * @dataProvider get_rank_data + */ + public function test_get_rank($group_data, $expected) + { + $this->assertEquals($expected, $this->group_helper->get_rank($group_data)); + } +} diff --git a/tests/group/helper_test.php b/tests/group/helper_test.php deleted file mode 100644 index 2377a6f47c..0000000000 --- a/tests/group/helper_test.php +++ /dev/null @@ -1,68 +0,0 @@ - - * @license GNU General Public License, version 2 (GPL-2.0) - * - * For full copyright and license information, please see - * the docs/CREDITS.txt file. - * - */ - -class phpbb_group_helper_test extends phpbb_test_case -{ - /** @var \phpbb\group\helper */ - protected $group_helper; - - public function setUp() - { - global $phpbb_root_path, $phpEx; - - // Set up language service - $lang = new \phpbb\language\language( - new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx) - ); - - // Set up language data for testing - $reflection_class = new ReflectionClass('\phpbb\language\language'); - - // Set default language files loaded flag to true - $loaded_flag = $reflection_class->getProperty('common_language_files_loaded'); - $loaded_flag->setAccessible(true); - $loaded_flag->setValue($lang, true); - - // Set up test language data - $lang_array = $reflection_class->getProperty('lang'); - $lang_array->setAccessible(true); - $lang_array->setValue($lang, $this->get_test_language_data_set()); - - // Set up group helper - $this->group_helper = new \phpbb\group\helper($lang); - } - - public function test_get_name() - { - // They should be totally fine - $this->assertEquals('Bots', $this->group_helper->get_name('Bots')); - $this->assertEquals('Some new group', $this->group_helper->get_name('new_group')); - $this->assertEquals('Should work', $this->group_helper->get_name('group_with_ümlauts')); - - // This should fail (obviously) - $this->assertNotEquals('They key does not contain uppercase letters', $this->group_helper->get_name('not_uppercase')); - - // The key doesn't exist so just return group name... - $this->assertEquals('Awesome group', $this->group_helper->get_name('Awesome group')); - } - - protected function get_test_language_data_set() - { - return array( - 'G_BOTS' => 'Bots', - 'G_NEW_GROUP' => 'Some new group', - 'G_not_uppercase' => 'The key does not contain uppercase letters', - 'G_GROUP_WITH_ÜMLAUTS' => 'Should work', - ); - } -} diff --git a/tests/group/helper_test_case.php b/tests/group/helper_test_case.php new file mode 100644 index 0000000000..1318cf9040 --- /dev/null +++ b/tests/group/helper_test_case.php @@ -0,0 +1,123 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class phpbb_group_helper_test_case extends phpbb_test_case +{ + /** @var \phpbb\group\helper */ + protected $group_helper; + + protected function config_defaults() + { + $defaults = array( + 'ranks_path' => 'images/ranks' + ); + return $defaults; + } + + protected function get_test_language_data_set() + { + return array( + 'G_BOTS' => 'Bots', + 'G_NEW_GROUP' => 'Some new group', + 'G_not_uppercase' => 'The key does not contain uppercase letters', + 'G_GROUP_WITH_ÜMLAUTS' => 'Should work', + ); + } + + protected function get_test_rank_data_set() + { + return array( + 'special' => array( + 1 => array( + 'rank_id' => 1, + 'rank_title' => 'Site admin', + 'rank_special' => 1, + 'rank_image' => 'siteadmin.png', + ), + 2 => array( + 'rank_id' => 2, + 'rank_title' => 'Test member', + 'rank_special' => 1, + 'rank_image' => '', + ) + ) + ); + } + + protected function setup_engine(array $new_config = array()) + { + global $cache, $phpbb_dispatcher, $phpbb_root_path, $phpEx; + + // Set up authentication data for testing + $auth = $this->getMock('\phpbb\auth\auth'); + $auth->expects($this->any()) + ->method('acl_get') + ->with($this->stringContains('_'), $this->anything()) + ->will($this->returnValueMap(array( + array('u_viewprofile', true), + ))); + + // Set up cache service + $cache_service = $this->getMockBuilder('\phpbb\cache\service')->disableOriginalConstructor()->getMock(); + $cache_service->expects($this->any()) + ->method('obtain_ranks') + ->will($this->returnValue($this->get_test_rank_data_set())); + + // Set up configuration + $defaults = $this->config_defaults(); + $config = new \phpbb\config\config(array_merge($defaults, $new_config)); + + // Set up language service + $lang = new \phpbb\language\language( + new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx) + ); + + // Set up language data for testing + $reflection_class = new ReflectionClass('\phpbb\language\language'); + + // Set default language files loaded flag to true + $loaded_flag = $reflection_class->getProperty('common_language_files_loaded'); + $loaded_flag->setAccessible(true); + $loaded_flag->setValue($lang, true); + + // Set up test language data + $lang_array = $reflection_class->getProperty('lang'); + $lang_array->setAccessible(true); + $lang_array->setValue($lang, $this->get_test_language_data_set()); + + // Set up event dispatcher + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + + // Set up path helper + $filesystem = new \phpbb\filesystem\filesystem(); + $path_helper = new \phpbb\path_helper( + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + $filesystem, + $this->getMock('\phpbb\request\request'), + $phpbb_root_path, + $phpEx + ); + + $user = new \phpbb\user($lang, '\phpbb\datetime'); + $user->data['user_id'] = ANONYMOUS; + + $this->group_helper = new \phpbb\group\helper($auth, $cache_service, $config, $lang, $phpbb_dispatcher, $path_helper, $user, $phpbb_root_path, $phpEx); + } + + public function setUp() + { + $this->setup_engine(); + } +} From a1baf106d68259daa0c6b1cd1bec8e870aef94f6 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Thu, 29 Nov 2018 20:27:04 +0100 Subject: [PATCH 03/14] [ticket/15886] Fix notification_group_request_test PHPBB3-15886 --- tests/notification/group_request_test.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/notification/group_request_test.php b/tests/notification/group_request_test.php index 92e758a336..36017945ec 100644 --- a/tests/notification/group_request_test.php +++ b/tests/notification/group_request_test.php @@ -49,9 +49,17 @@ class phpbb_notification_group_request_test extends phpbb_tests_notification_bas $this->cache->get_driver() )); $this->container->set('group_helper', new \phpbb\group\helper( + $this->getMock('\phpbb\auth\auth'), + $this->cache, + $this->config, new \phpbb\language\language( new phpbb\language\language_file_loader($phpbb_root_path, $phpEx) - ) + ), + new phpbb_mock_event_dispatcher(), + $this->getMock('\phpbb\path_helper'), + $this->user, + $phpbb_root_path, + $phpEx )); $phpbb_dispatcher = new phpbb_mock_event_dispatcher; $phpbb_log = new \phpbb\log\dummy(); From d356fa9f97986d7d31aa77d44c08ae07fbbe1579 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Thu, 29 Nov 2018 20:46:52 +0100 Subject: [PATCH 04/14] [ticket/15886] Arguments for path_helper PHPBB3-15886 --- tests/notification/group_request_test.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/notification/group_request_test.php b/tests/notification/group_request_test.php index 36017945ec..f53fe32f2a 100644 --- a/tests/notification/group_request_test.php +++ b/tests/notification/group_request_test.php @@ -56,7 +56,15 @@ class phpbb_notification_group_request_test extends phpbb_tests_notification_bas new phpbb\language\language_file_loader($phpbb_root_path, $phpEx) ), new phpbb_mock_event_dispatcher(), - $this->getMock('\phpbb\path_helper'), + new \phpbb\path_helper( + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + new \phpbb\filesystem\filesystem(), + $this->getMock('\phpbb\request\request'), + $phpbb_root_path, + $phpEx + ), $this->user, $phpbb_root_path, $phpEx From d79eb72fc13667db9ab3953fe4a243829efaed53 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sat, 29 Dec 2018 13:11:51 +0100 Subject: [PATCH 05/14] [ticket/15886] Class variable, comment splitting, events since tag, class names PHPBB3-15886 --- phpBB/phpbb/group/helper.php | 87 +++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index fc657a59a2..3b4796f3b2 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -13,27 +13,35 @@ namespace phpbb\group; +use phpbb\auth\auth; +use phpbb\cache; +use phpbb\config\config; +use phpbb\language\language; +use phpbb\event\dispatcher_interface; +use phpbb\path_helper; +use phpbb\user; + class helper { - /** @var \phpbb\auth\auth */ + /** @var auth */ protected $auth; - /** @var \phpbb\cache\service */ + /** @var cache\service */ protected $cache; - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var \phpbb\language\language */ + /** @var language */ protected $language; - /** @var \phpbb\event\dispatcher_interface */ + /** @var dispatcher_interface */ protected $phpbb_dispatcher; - /** @var \phpbb\path_helper */ + /** @var path_helper */ protected $phpbb_path_helper; - /** @var \phpbb\user */ + /** @var user */ protected $user; /** @var string phpBB root path */ @@ -42,20 +50,23 @@ class helper /** @var string PHP file extension */ protected $php_ext; + /** @var array Return templates for a group name string */ + protected $name_strings; + /** * Constructor * - * @param \phpbb\auth\auth $auth Authentication object - * @param \phpbb\cache\service $cache Cache object - * @param \phpbb\config\config $config Configuration object - * @param \phpbb\language\language $language Language object - * @param \phpbb\event\dispatcher_interface $phpbb_dispatcher Event dispatcher object - * @param \phpbb\path_helper $phpbb_path_helper Path helper object - * @param \phpbb\user $user User object - * @param string $phpbb_root_path phpBB root path - * @param string $php_ext PHP file extension + * @param auth $auth Authentication object + * @param cache\service $cache Cache service object + * @param config $config Configuration object + * @param language $language Language object + * @param dispatcher_interface $phpbb_dispatcher Event dispatcher object + * @param path_helper $phpbb_path_helper Path helper object + * @param user $user User object + * @param string $phpbb_root_path phpBB root path + * @param string $php_ext PHP file extension */ - public function __construct(\phpbb\auth\auth $auth, \phpbb\cache\service $cache, \phpbb\config\config $config, \phpbb\language\language $language, \phpbb\event\dispatcher_interface $phpbb_dispatcher, \phpbb\path_helper $phpbb_path_helper, \phpbb\user $user, $phpbb_root_path, $php_ext) + public function __construct(auth $auth, cache\service $cache, config $config, language $language, dispatcher_interface $phpbb_dispatcher, path_helper $phpbb_path_helper, user $user, $phpbb_root_path, $php_ext) { $this->auth = $auth; $this->cache = $cache; @@ -67,6 +78,14 @@ class helper $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; + + $this->name_strings = array( + 'base_url' => append_sid("{$phpbb_root_path}memberlist.{$php_ext}", 'mode=group&g={GROUP_ID}'), + 'tpl_noprofile' => '{GROUP_NAME}', + 'tpl_noprofile_colour' => '{GROUP_NAME}', + 'tpl_profile' => '{GROUP_NAME}', + 'tpl_profile_colour' => '{GROUP_NAME}', + ); } /** @@ -82,7 +101,11 @@ class helper /** * Get group name details for placing into templates. * - * @param string $mode Can be profile (for getting an url to the profile), group_name (for obtaining the group name), colour (for obtaining the group colour), full (for obtaining a html string representing a coloured link to the group's profile) or no_profile (the same as full but forcing no profile link) + * @param string $mode profile (for getting an url to the profile), + * group_name (for obtaining the group name), + * colour (for obtaining the group colour), + * full (for obtaining a coloured group name link to the group's profile), + * no_profile (the same as full but forcing no profile link) * @param int $group_id The group id * @param string $group_name The group name * @param string $group_colour The group colour @@ -92,14 +115,6 @@ class helper */ public function get_name_string($mode, $group_id, $group_name, $group_colour = '', $custom_profile_url = false) { - $_profile_cache = array( - 'base_url' => append_sid("{$this->phpbb_root_path}memberlist.{$this->php_ext}", 'mode=group&g={GROUP_ID}'), - 'tpl_noprofile' => '{GROUP_NAME}', - 'tpl_noprofile_colour' => '{GROUP_NAME}', - 'tpl_profile' => '{GROUP_NAME}', - 'tpl_profile_colour' => '{GROUP_NAME}', - ); - // This switch makes sure we only run code required for the mode switch ($mode) { @@ -139,7 +154,7 @@ class helper // For anonymous the link leads to a login page. if ($group_id && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) { - $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $_profile_cache['base_url']); + $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $this->name_strings['base_url']); } else { @@ -158,16 +173,18 @@ class helper if (!isset($group_name_string)) { - if (($mode === 'full' && !$profile_url) || $mode === 'no_profile') + if (($mode === 'full' && empty($profile_url)) || $mode === 'no_profile') { - $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $_profile_cache['tpl_noprofile'] : $_profile_cache['tpl_noprofile_colour']); + $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $this->name_strings['tpl_noprofile'] : $this->name_strings['tpl_noprofile_colour']); } else { - $group_name_string = str_replace(array('{PROFILE_URL}', '{GROUP_COLOUR}', '{GROUP_NAME}'), array($profile_url, $group_colour, $group_name), (!$group_colour) ? $_profile_cache['tpl_profile'] : $_profile_cache['tpl_profile_colour']); + $group_name_string = str_replace(array('{PROFILE_URL}', '{GROUP_COLOUR}', '{GROUP_NAME}'), array($profile_url, $group_colour, $group_name), (!$group_colour) ? $this->name_strings['tpl_profile'] : $this->name_strings['tpl_profile_colour']); } } + $name_strings = $this->name_strings; + /** * Use this event to change the output of the group name * @@ -178,8 +195,8 @@ class helper * @var string group_colour The group colour * @var string custom_profile_url Optional parameter to specify a profile url. * @var string group_name_string The string that has been generated - * @var array _profile_cache Array of original return templates - * @since 3.2.5-RC1 + * @var array name_strings Array of original return templates + * @since 3.2.6-RC1 */ $vars = array( 'mode', @@ -188,7 +205,7 @@ class helper 'group_colour', 'custom_profile_url', 'group_name_string', - '_profile_cache', + 'name_strings', ); extract($this->phpbb_dispatcher->trigger_event('core.modify_group_name_string', compact($vars))); @@ -216,7 +233,7 @@ class helper * * @event core.get_group_rank_before * @var array group_data Array with group's data - * @since 3.2.5-RC1 + * @since 3.2.6-RC1 */ $vars = array('group_data'); @@ -245,7 +262,7 @@ class helper * @event core.get_group_rank_after * @var array group_data Array with group's data * @var array group_rank_data Group rank data - * @since 3.2.5-RC1 + * @since 3.2.6-RC1 */ $vars = array( From e2e3d402a25b6203f2c29296eddf2dae002053da Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sat, 29 Dec 2018 13:28:00 +0100 Subject: [PATCH 06/14] [ticket/15886] Clean up services PHPBB3-15886 --- phpBB/config/default/container/services.yml | 2 - phpBB/phpbb/group/helper.php | 46 ++-- tests/group/helper_test_case.php | 246 ++++++++++---------- tests/notification/group_request_test.php | 4 +- 4 files changed, 144 insertions(+), 154 deletions(-) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index cd1d872ae0..3ead1e6181 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -129,8 +129,6 @@ services: - '@dispatcher' - '@path_helper' - '@user' - - '%core.root_path%' - - '%core.php_ext%' log: class: phpbb\log\log diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 3b4796f3b2..9a53df68d3 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -14,7 +14,7 @@ namespace phpbb\group; use phpbb\auth\auth; -use phpbb\cache; +use phpbb\cache\service as cache; use phpbb\config\config; use phpbb\language\language; use phpbb\event\dispatcher_interface; @@ -26,7 +26,7 @@ class helper /** @var auth */ protected $auth; - /** @var cache\service */ + /** @var cache */ protected $cache; /** @var config */ @@ -36,10 +36,10 @@ class helper protected $language; /** @var dispatcher_interface */ - protected $phpbb_dispatcher; + protected $dispatcher; /** @var path_helper */ - protected $phpbb_path_helper; + protected $path_helper; /** @var user */ protected $user; @@ -47,40 +47,34 @@ class helper /** @var string phpBB root path */ protected $phpbb_root_path; - /** @var string PHP file extension */ - protected $php_ext; - /** @var array Return templates for a group name string */ protected $name_strings; /** * Constructor * - * @param auth $auth Authentication object - * @param cache\service $cache Cache service object - * @param config $config Configuration object - * @param language $language Language object - * @param dispatcher_interface $phpbb_dispatcher Event dispatcher object - * @param path_helper $phpbb_path_helper Path helper object - * @param user $user User object - * @param string $phpbb_root_path phpBB root path - * @param string $php_ext PHP file extension + * @param auth $auth Authentication object + * @param cache $cache Cache service object + * @param config $config Configuration object + * @param language $language Language object + * @param dispatcher_interface $dispatcher Event dispatcher object + * @param path_helper $path_helper Path helper object + * @param user $user User object */ - public function __construct(auth $auth, cache\service $cache, config $config, language $language, dispatcher_interface $phpbb_dispatcher, path_helper $phpbb_path_helper, user $user, $phpbb_root_path, $php_ext) + public function __construct(auth $auth, cache $cache, config $config, language $language, dispatcher_interface $dispatcher, path_helper $path_helper, user $user) { $this->auth = $auth; $this->cache = $cache; $this->config = $config; $this->language = $language; - $this->phpbb_dispatcher = $phpbb_dispatcher; - $this->phpbb_path_helper = $phpbb_path_helper; + $this->dispatcher = $dispatcher; + $this->path_helper = $path_helper; $this->user = $user; - $this->phpbb_root_path = $phpbb_root_path; - $this->php_ext = $php_ext; + $this->phpbb_root_path = $path_helper->get_phpbb_root_path(); $this->name_strings = array( - 'base_url' => append_sid("{$phpbb_root_path}memberlist.{$php_ext}", 'mode=group&g={GROUP_ID}'), + 'base_url' => append_sid("{$path_helper->get_phpbb_root_path()}memberlist.{$path_helper->get_php_ext()}", 'mode=group&g={GROUP_ID}'), 'tpl_noprofile' => '{GROUP_NAME}', 'tpl_noprofile_colour' => '{GROUP_NAME}', 'tpl_profile' => '{GROUP_NAME}', @@ -207,7 +201,7 @@ class helper 'group_name_string', 'name_strings', ); - extract($this->phpbb_dispatcher->trigger_event('core.modify_group_name_string', compact($vars))); + extract($this->dispatcher->trigger_event('core.modify_group_name_string', compact($vars))); return $group_name_string; } @@ -237,7 +231,7 @@ class helper */ $vars = array('group_data'); - extract($this->phpbb_dispatcher->trigger_event('core.get_group_rank_before', compact($vars))); + extract($this->dispatcher->trigger_event('core.get_group_rank_before', compact($vars))); if (!empty($group_data['group_rank'])) { @@ -250,7 +244,7 @@ class helper $group_rank_data['title'] = $rank['rank_title']; - $group_rank_data['img_src'] = (!empty($rank['rank_image'])) ? $this->phpbb_path_helper->update_web_root_path($this->phpbb_root_path . $this->config['ranks_path'] . '/' . $rank['rank_image']) : ''; + $group_rank_data['img_src'] = (!empty($rank['rank_image'])) ? $this->path_helper->update_web_root_path($this->phpbb_root_path . $this->config['ranks_path'] . '/' . $rank['rank_image']) : ''; $group_rank_data['img'] = (!empty($rank['rank_image'])) ? '' . $rank['rank_title'] . '' : ''; } @@ -269,7 +263,7 @@ class helper 'group_data', 'group_rank_data', ); - extract($this->phpbb_dispatcher->trigger_event('core.get_group_rank_after', compact($vars))); + extract($this->dispatcher->trigger_event('core.get_group_rank_after', compact($vars))); return $group_rank_data; } diff --git a/tests/group/helper_test_case.php b/tests/group/helper_test_case.php index 1318cf9040..cfbcd5890c 100644 --- a/tests/group/helper_test_case.php +++ b/tests/group/helper_test_case.php @@ -1,123 +1,123 @@ - - * @license GNU General Public License, version 2 (GPL-2.0) - * - * For full copyright and license information, please see - * the docs/CREDITS.txt file. - * - */ - -class phpbb_group_helper_test_case extends phpbb_test_case -{ - /** @var \phpbb\group\helper */ - protected $group_helper; - - protected function config_defaults() - { - $defaults = array( - 'ranks_path' => 'images/ranks' - ); - return $defaults; - } - - protected function get_test_language_data_set() - { - return array( - 'G_BOTS' => 'Bots', - 'G_NEW_GROUP' => 'Some new group', - 'G_not_uppercase' => 'The key does not contain uppercase letters', - 'G_GROUP_WITH_ÜMLAUTS' => 'Should work', - ); - } - - protected function get_test_rank_data_set() - { - return array( - 'special' => array( - 1 => array( - 'rank_id' => 1, - 'rank_title' => 'Site admin', - 'rank_special' => 1, - 'rank_image' => 'siteadmin.png', - ), - 2 => array( - 'rank_id' => 2, - 'rank_title' => 'Test member', - 'rank_special' => 1, - 'rank_image' => '', - ) - ) - ); - } - - protected function setup_engine(array $new_config = array()) - { - global $cache, $phpbb_dispatcher, $phpbb_root_path, $phpEx; - - // Set up authentication data for testing - $auth = $this->getMock('\phpbb\auth\auth'); - $auth->expects($this->any()) - ->method('acl_get') - ->with($this->stringContains('_'), $this->anything()) - ->will($this->returnValueMap(array( - array('u_viewprofile', true), - ))); - - // Set up cache service - $cache_service = $this->getMockBuilder('\phpbb\cache\service')->disableOriginalConstructor()->getMock(); - $cache_service->expects($this->any()) - ->method('obtain_ranks') - ->will($this->returnValue($this->get_test_rank_data_set())); - - // Set up configuration - $defaults = $this->config_defaults(); - $config = new \phpbb\config\config(array_merge($defaults, $new_config)); - - // Set up language service - $lang = new \phpbb\language\language( - new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx) - ); - - // Set up language data for testing - $reflection_class = new ReflectionClass('\phpbb\language\language'); - - // Set default language files loaded flag to true - $loaded_flag = $reflection_class->getProperty('common_language_files_loaded'); - $loaded_flag->setAccessible(true); - $loaded_flag->setValue($lang, true); - - // Set up test language data - $lang_array = $reflection_class->getProperty('lang'); - $lang_array->setAccessible(true); - $lang_array->setValue($lang, $this->get_test_language_data_set()); - - // Set up event dispatcher - $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - - // Set up path helper - $filesystem = new \phpbb\filesystem\filesystem(); - $path_helper = new \phpbb\path_helper( - new \phpbb\symfony_request( - new phpbb_mock_request() - ), - $filesystem, - $this->getMock('\phpbb\request\request'), - $phpbb_root_path, - $phpEx - ); - - $user = new \phpbb\user($lang, '\phpbb\datetime'); - $user->data['user_id'] = ANONYMOUS; - - $this->group_helper = new \phpbb\group\helper($auth, $cache_service, $config, $lang, $phpbb_dispatcher, $path_helper, $user, $phpbb_root_path, $phpEx); - } - - public function setUp() - { - $this->setup_engine(); - } -} + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class phpbb_group_helper_test_case extends phpbb_test_case +{ + /** @var \phpbb\group\helper */ + protected $group_helper; + + protected function config_defaults() + { + $defaults = array( + 'ranks_path' => 'images/ranks' + ); + return $defaults; + } + + protected function get_test_language_data_set() + { + return array( + 'G_BOTS' => 'Bots', + 'G_NEW_GROUP' => 'Some new group', + 'G_not_uppercase' => 'The key does not contain uppercase letters', + 'G_GROUP_WITH_ÜMLAUTS' => 'Should work', + ); + } + + protected function get_test_rank_data_set() + { + return array( + 'special' => array( + 1 => array( + 'rank_id' => 1, + 'rank_title' => 'Site admin', + 'rank_special' => 1, + 'rank_image' => 'siteadmin.png', + ), + 2 => array( + 'rank_id' => 2, + 'rank_title' => 'Test member', + 'rank_special' => 1, + 'rank_image' => '', + ) + ) + ); + } + + protected function setup_engine(array $new_config = array()) + { + global $phpbb_dispatcher, $phpbb_root_path, $phpEx; + + // Set up authentication data for testing + $auth = $this->getMock('\phpbb\auth\auth'); + $auth->expects($this->any()) + ->method('acl_get') + ->with($this->stringContains('_'), $this->anything()) + ->will($this->returnValueMap(array( + array('u_viewprofile', true), + ))); + + // Set up cache service + $cache_service = $this->getMockBuilder('\phpbb\cache\service')->disableOriginalConstructor()->getMock(); + $cache_service->expects($this->any()) + ->method('obtain_ranks') + ->will($this->returnValue($this->get_test_rank_data_set())); + + // Set up configuration + $defaults = $this->config_defaults(); + $config = new \phpbb\config\config(array_merge($defaults, $new_config)); + + // Set up language service + $lang = new \phpbb\language\language( + new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx) + ); + + // Set up language data for testing + $reflection_class = new ReflectionClass('\phpbb\language\language'); + + // Set default language files loaded flag to true + $loaded_flag = $reflection_class->getProperty('common_language_files_loaded'); + $loaded_flag->setAccessible(true); + $loaded_flag->setValue($lang, true); + + // Set up test language data + $lang_array = $reflection_class->getProperty('lang'); + $lang_array->setAccessible(true); + $lang_array->setValue($lang, $this->get_test_language_data_set()); + + // Set up event dispatcher + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + + // Set up path helper + $filesystem = new \phpbb\filesystem\filesystem(); + $path_helper = new \phpbb\path_helper( + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + $filesystem, + $this->getMock('\phpbb\request\request'), + $phpbb_root_path, + $phpEx + ); + + $user = new \phpbb\user($lang, '\phpbb\datetime'); + $user->data['user_id'] = ANONYMOUS; + + $this->group_helper = new \phpbb\group\helper($auth, $cache_service, $config, $lang, $phpbb_dispatcher, $path_helper, $user); + } + + public function setUp() + { + $this->setup_engine(); + } +} diff --git a/tests/notification/group_request_test.php b/tests/notification/group_request_test.php index f53fe32f2a..e849c66fa5 100644 --- a/tests/notification/group_request_test.php +++ b/tests/notification/group_request_test.php @@ -65,9 +65,7 @@ class phpbb_notification_group_request_test extends phpbb_tests_notification_bas $phpbb_root_path, $phpEx ), - $this->user, - $phpbb_root_path, - $phpEx + $this->user )); $phpbb_dispatcher = new phpbb_mock_event_dispatcher; $phpbb_log = new \phpbb\log\dummy(); From 50cec4d54cfe2985df3d7fb4fd381c1c810a356b Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 30 Dec 2018 14:15:03 +0100 Subject: [PATCH 07/14] [ticket/15886] Change phpbb_get_group_avatar variable names PHPBB3-15886 --- phpBB/includes/functions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 99f65a0e92..e3c3a19c96 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4081,9 +4081,9 @@ function phpbb_get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = * * @return string Avatar html */ -function phpbb_get_group_avatar($user_row, $alt = 'GROUP_AVATAR', $ignore_config = false, $lazy = false) +function phpbb_get_group_avatar($group_row, $alt = 'GROUP_AVATAR', $ignore_config = false, $lazy = false) { - $row = \phpbb\avatar\manager::clean_row($user_row, 'group'); + $row = \phpbb\avatar\manager::clean_row($group_row, 'group'); return phpbb_get_avatar($row, $alt, $ignore_config, $lazy); } From f023dd590f28b2721773f8429a2d1fbf995f544f Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Wed, 2 Jan 2019 12:33:25 +0100 Subject: [PATCH 08/14] [ticket/15886] Mock path helper in group helper tests PHPBB3-15886 --- tests/group/helper_test_case.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/group/helper_test_case.php b/tests/group/helper_test_case.php index cfbcd5890c..e298770331 100644 --- a/tests/group/helper_test_case.php +++ b/tests/group/helper_test_case.php @@ -99,16 +99,16 @@ class phpbb_group_helper_test_case extends phpbb_test_case $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); // Set up path helper - $filesystem = new \phpbb\filesystem\filesystem(); - $path_helper = new \phpbb\path_helper( - new \phpbb\symfony_request( - new phpbb_mock_request() - ), - $filesystem, - $this->getMock('\phpbb\request\request'), - $phpbb_root_path, - $phpEx - ); + $path_helper = $this->getMockBuilder('\phpbb\path_helper') + ->disableOriginalConstructor() + ->setMethods(array()) + ->getMock(); + $path_helper->method('get_phpbb_root_path') + ->willReturn($phpbb_root_path); + $path_helper->method('get_php_ext') + ->willReturn($phpEx); + $path_helper->method('update_web_root_path') + ->will($this->returnArgument(0)); $user = new \phpbb\user($lang, '\phpbb\datetime'); $user->data['user_id'] = ANONYMOUS; From f6c93a81d302be22d754fb65e50d60699ea7b5c0 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Wed, 2 Jan 2019 13:21:44 +0100 Subject: [PATCH 09/14] [ticket/15886] No profile url for BOTS group PHPBB3-15886 --- phpBB/phpbb/group/helper.php | 8 +++++--- tests/group/helper_get_name_string_test.php | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 9a53df68d3..21a09e4e63 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -109,6 +109,8 @@ class helper */ public function get_name_string($mode, $group_id, $group_name, $group_colour = '', $custom_profile_url = false) { + $s_is_bots = ($group_name === 'BOTS'); + // This switch makes sure we only run code required for the mode switch ($mode) { @@ -117,7 +119,7 @@ class helper case 'colour': // Build correct group colour - $group_colour = ($group_colour) ? '#' . $group_colour : ''; + $group_colour = $group_colour ? '#' . $group_colour : ''; // Return colour if ($mode === 'colour') @@ -146,7 +148,7 @@ class helper // Build correct profile url - only show if not anonymous and permission to view profile if registered user // For anonymous the link leads to a login page. - if ($group_id && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) + if ($group_id && !$s_is_bots && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) { $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $this->name_strings['base_url']); } @@ -167,7 +169,7 @@ class helper if (!isset($group_name_string)) { - if (($mode === 'full' && empty($profile_url)) || $mode === 'no_profile') + if (($mode === 'full' && empty($profile_url)) || $mode === 'no_profile' || $s_is_bots) { $group_name_string = str_replace(array('{GROUP_COLOUR}', '{GROUP_NAME}'), array($group_colour, $group_name), (!$group_colour) ? $this->name_strings['tpl_noprofile'] : $this->name_strings['tpl_noprofile_colour']); } diff --git a/tests/group/helper_get_name_string_test.php b/tests/group/helper_get_name_string_test.php index 7ea0f156e4..565602a346 100644 --- a/tests/group/helper_get_name_string_test.php +++ b/tests/group/helper_get_name_string_test.php @@ -39,7 +39,7 @@ class phpbb_group_helper_get_name_string_test extends phpbb_group_helper_test_ca { return array( // Should be fine - array(0, 'Bots', 'AA0000', false, 'Bots'), + array(0, 'BOTS', 'AA0000', false, 'Bots'), array(1, 'new_group', '', false, 'Some new group'), array(2, 'group_with_ümlauts', '', 'http://www.example.org/group.php?mode=show', 'Should work'), @@ -80,7 +80,8 @@ class phpbb_group_helper_get_name_string_test extends phpbb_group_helper_test_ca global $phpbb_root_path, $phpEx; return array( - array(0, 'Bots', '000000', false, 'Bots'), + array(0, 'BOTS', '000000', false, 'Bots'), + array(1, 'BOTS', '111111', false, 'Bots'), array(7, 'new_group', 'FFA500', false, 'Some new group'), array(14, 'Awesome group', '', 'http://www.example.org/group.php?mode=show', 'Awesome group'), ); @@ -97,7 +98,7 @@ class phpbb_group_helper_get_name_string_test extends phpbb_group_helper_test_ca public function get_name_string_no_profile_data() { return array( - array(0, 'Bots', '000000', false, 'Bots'), + array(0, 'BOTS', '000000', false, 'Bots'), array(1, 'new_group', '', false, 'Some new group'), arraY(2, 'not_uppercase', 'FF0000', false, 'not_uppercase'), array(5, 'Awesome group', '', 'http://www.example.org/group.php?mode=show', 'Awesome group'), From 813789c644dfd34ec9f63e16ad9f6b8a2aa97163 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Wed, 2 Jan 2019 13:30:58 +0100 Subject: [PATCH 10/14] [ticket/15886] Capital Y in arraY PHPBB3-15886 --- tests/group/helper_get_name_string_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/group/helper_get_name_string_test.php b/tests/group/helper_get_name_string_test.php index 565602a346..c626328dcc 100644 --- a/tests/group/helper_get_name_string_test.php +++ b/tests/group/helper_get_name_string_test.php @@ -100,7 +100,7 @@ class phpbb_group_helper_get_name_string_test extends phpbb_group_helper_test_ca return array( array(0, 'BOTS', '000000', false, 'Bots'), array(1, 'new_group', '', false, 'Some new group'), - arraY(2, 'not_uppercase', 'FF0000', false, 'not_uppercase'), + array(2, 'not_uppercase', 'FF0000', false, 'not_uppercase'), array(5, 'Awesome group', '', 'http://www.example.org/group.php?mode=show', 'Awesome group'), ); } From 214cf2c0116cffdb0eed27c81ebb12d8697c485b Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 7 Jan 2019 21:19:58 +0100 Subject: [PATCH 11/14] [ticket/15886] Move append_sid from constructor PHPBB3-15886 --- phpBB/phpbb/group/helper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 21a09e4e63..5e60270931 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -74,7 +74,7 @@ class helper $this->phpbb_root_path = $path_helper->get_phpbb_root_path(); $this->name_strings = array( - 'base_url' => append_sid("{$path_helper->get_phpbb_root_path()}memberlist.{$path_helper->get_php_ext()}", 'mode=group&g={GROUP_ID}'), + 'base_url' => "{$path_helper->get_phpbb_root_path()}memberlist.{$path_helper->get_php_ext()}?mode=group&g={GROUP_ID}", 'tpl_noprofile' => '{GROUP_NAME}', 'tpl_noprofile_colour' => '{GROUP_NAME}', 'tpl_profile' => '{GROUP_NAME}', @@ -150,7 +150,7 @@ class helper // For anonymous the link leads to a login page. if ($group_id && !$s_is_bots && ($this->user->data['user_id'] == ANONYMOUS || $this->auth->acl_get('u_viewprofile'))) { - $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, $this->name_strings['base_url']); + $profile_url = ($custom_profile_url !== false) ? $custom_profile_url . '&g=' . (int) $group_id : str_replace(array('={GROUP_ID}', '=%7BGROUP_ID%7D'), '=' . (int) $group_id, append_sid($this->name_strings['base_url'])); } else { From 479201a3a1b821d2b3212000955fad1343f4efd0 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sat, 9 Mar 2019 10:30:19 +0100 Subject: [PATCH 12/14] [ticket/15886] Add @html doc for group/user strings PHPBB3-15886 --- phpBB/includes/functions_content.php | 1 + phpBB/phpbb/group/helper.php | 1 + 2 files changed, 2 insertions(+) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 8284aab6a4..e83a9ec195 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1501,6 +1501,7 @@ function get_username_string($mode, $user_id, $username, $username_colour = '', { global $phpbb_root_path, $phpEx; + /** @html User name strings for usage in the template */ $_profile_cache['base_url'] = append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile&u={USER_ID}'); $_profile_cache['tpl_noprofile'] = '{USERNAME}'; $_profile_cache['tpl_noprofile_colour'] = '{USERNAME}'; diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 5e60270931..7f249d7aee 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -73,6 +73,7 @@ class helper $this->phpbb_root_path = $path_helper->get_phpbb_root_path(); + /** @html Group name strings for usage in the template */ $this->name_strings = array( 'base_url' => "{$path_helper->get_phpbb_root_path()}memberlist.{$path_helper->get_php_ext()}?mode=group&g={GROUP_ID}", 'tpl_noprofile' => '{GROUP_NAME}', From 45e1aff14a149a652924e77cd404e39b2e5aacb5 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Sun, 10 Mar 2019 17:44:03 +0100 Subject: [PATCH 13/14] [ticket/15886] Appropriate HTML docs PHPBB3-15886 --- phpBB/includes/functions_content.php | 4 +++- phpBB/phpbb/group/helper.php | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index e83a9ec195..1840416efc 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1482,6 +1482,8 @@ function truncate_string($string, $max_length = 60, $max_store_length = 255, $al * Get username details for placing into templates. * This function caches all modes on first call, except for no_profile and anonymous user - determined by $user_id. * +* @html Username spans and links +* * @param string $mode Can be profile (for getting an url to the profile), username (for obtaining the username), colour (for obtaining the user colour), full (for obtaining a html string representing a coloured link to the users profile) or no_profile (the same as full but forcing no profile link) * @param int $user_id The users id * @param string $username The users name @@ -1501,7 +1503,7 @@ function get_username_string($mode, $user_id, $username, $username_colour = '', { global $phpbb_root_path, $phpEx; - /** @html User name strings for usage in the template */ + /** @html Username spans and links for usage in the template */ $_profile_cache['base_url'] = append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile&u={USER_ID}'); $_profile_cache['tpl_noprofile'] = '{USERNAME}'; $_profile_cache['tpl_noprofile_colour'] = '{USERNAME}'; diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 7f249d7aee..5f6d1ba440 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -73,7 +73,7 @@ class helper $this->phpbb_root_path = $path_helper->get_phpbb_root_path(); - /** @html Group name strings for usage in the template */ + /** @html Group name spans and links for usage in the template */ $this->name_strings = array( 'base_url' => "{$path_helper->get_phpbb_root_path()}memberlist.{$path_helper->get_php_ext()}?mode=group&g={GROUP_ID}", 'tpl_noprofile' => '{GROUP_NAME}', @@ -96,6 +96,8 @@ class helper /** * Get group name details for placing into templates. * + * @html Group name spans and links + * * @param string $mode profile (for getting an url to the profile), * group_name (for obtaining the group name), * colour (for obtaining the group colour), @@ -212,6 +214,8 @@ class helper /** * Get group rank title and image * + * @html Group rank image element + * * @param array $group_data the current stored group data * * @return array An associative array containing the rank title (title), @@ -249,6 +253,7 @@ class helper $group_rank_data['img_src'] = (!empty($rank['rank_image'])) ? $this->path_helper->update_web_root_path($this->phpbb_root_path . $this->config['ranks_path'] . '/' . $rank['rank_image']) : ''; + /** @html Group rank image element for usage in the template */ $group_rank_data['img'] = (!empty($rank['rank_image'])) ? '' . $rank['rank_title'] . '' : ''; } } From cf9696767715eeef7d82f9dcd5533c50c4f119aa Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 28 Jul 2019 12:21:46 +0200 Subject: [PATCH 14/14] [ticket/15886] Fix minor code style issues PHPBB3-15886 --- phpBB/phpbb/group/helper.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/group/helper.php b/phpBB/phpbb/group/helper.php index 5f6d1ba440..aa3876b325 100644 --- a/phpBB/phpbb/group/helper.php +++ b/phpBB/phpbb/group/helper.php @@ -98,11 +98,11 @@ class helper * * @html Group name spans and links * - * @param string $mode profile (for getting an url to the profile), - * group_name (for obtaining the group name), - * colour (for obtaining the group colour), - * full (for obtaining a coloured group name link to the group's profile), - * no_profile (the same as full but forcing no profile link) + * @param string $mode Profile (for getting an url to the profile), + * group_name (for obtaining the group name), + * colour (for obtaining the group colour), + * full (for obtaining a coloured group name link to the group's profile), + * no_profile (the same as full but forcing no profile link) * @param int $group_id The group id * @param string $group_name The group name * @param string $group_colour The group colour @@ -195,7 +195,7 @@ class helper * @var string custom_profile_url Optional parameter to specify a profile url. * @var string group_name_string The string that has been generated * @var array name_strings Array of original return templates - * @since 3.2.6-RC1 + * @since 3.2.8-RC1 */ $vars = array( 'mode', @@ -216,7 +216,7 @@ class helper * * @html Group rank image element * - * @param array $group_data the current stored group data + * @param array $group_data The current stored group data * * @return array An associative array containing the rank title (title), * the rank image as full img tag (img) and the rank image source (img_src) @@ -234,7 +234,7 @@ class helper * * @event core.get_group_rank_before * @var array group_data Array with group's data - * @since 3.2.6-RC1 + * @since 3.2.8-RC1 */ $vars = array('group_data'); @@ -264,7 +264,7 @@ class helper * @event core.get_group_rank_after * @var array group_data Array with group's data * @var array group_rank_data Group rank data - * @since 3.2.6-RC1 + * @since 3.2.8-RC1 */ $vars = array(