From 78ea30860800e5e58bf8d7b8cfbbcaf08193dece Mon Sep 17 00:00:00 2001 From: javiexin Date: Thu, 28 May 2015 22:25:06 +0200 Subject: [PATCH 01/13] [ticket/13867] Enable/disable mechanism for new profile field types Adds methods to enable, disable and purge profile field types to the profilefields\manager class. These methods are to be called from extensions that add new profile field types on the enable, disable and purge methods of the ext class. If not done, any profile field of a new type that is left after extension is disabled or removed will break the forum in several places. PHPBB3-13867 --- .../container/services_profilefield.yml | 1 + phpBB/phpbb/profilefields/manager.php | 176 +++++++++++++++++- 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/phpBB/config/default/container/services_profilefield.yml b/phpBB/config/default/container/services_profilefield.yml index 90b22836e5..8b519e5c7c 100644 --- a/phpBB/config/default/container/services_profilefield.yml +++ b/phpBB/config/default/container/services_profilefield.yml @@ -2,6 +2,7 @@ services: profilefields.manager: class: phpbb\profilefields\manager arguments: + - '@service_container' - '@auth' - '@dbal.conn' - '@dispatcher' diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index 73b8cdd38e..bb76d39f85 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -13,11 +13,19 @@ namespace phpbb\profilefields; +use Symfony\Component\DependencyInjection\ContainerInterface; + /** * Custom Profile Fields */ class manager { + /** + * Container interface + * @var ContainerInterface + */ + protected $container; + /** * Auth object * @var \phpbb\auth\auth @@ -68,9 +76,14 @@ class manager protected $profile_cache = array(); + protected $config_text; + + protected $db_tools; + /** * Construct * + * @param ContainerInterface $container A container * @param \phpbb\auth\auth $auth Auth object * @param \phpbb\db\driver\driver_interface $db Database object * @param \phpbb\event\dispatcher_interface $dispatcher Event dispatcher object @@ -82,8 +95,9 @@ class manager * @param string $fields_language_table * @param string $fields_data_table */ - public function __construct(\phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\di\service_collection $type_collection, \phpbb\user $user, $fields_table, $fields_language_table, $fields_data_table) + public function __construct(ContainerInterface $container, \phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\di\service_collection $type_collection, \phpbb\user $user, $fields_table, $fields_language_table, $fields_data_table) { + $this->container = $container; $this->auth = $auth; $this->db = $db; $this->dispatcher = $dispatcher; @@ -95,6 +109,9 @@ class manager $this->fields_table = $fields_table; $this->fields_language_table = $fields_language_table; $this->fields_data_table = $fields_data_table; + + $this->config_text = $this->container->get('config_text'); + $this->db_tools = $this->container->get('dbal.tools'); } /** @@ -499,4 +516,161 @@ class manager return $cp_data; } + + /** + * Disable all profile fields of a certain type + * + * This should be called when an extension which has profile field types + * is disabled so that all those profile fields are hidden and do not + * cause errors + * + * @param string $profilefield_type_name Type identifier of the profile fields + */ + public function disable_profilefields($profilefield_type_name) + { + // Get list of profile fields affected by this operation, if any + $pfs = array(); + $sql = 'SELECT field_id, field_ident + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_active = 1 + AND field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $pfs[(int) $row['field_id']] = $row['field_ident']; + } + $this->db->sql_freeresult($result); + + // If no profile fields affected, then nothing to do + if (!sizeof($pfs)) + { + return; + } + + // Update the affected profile fields to "inactive" + $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + SET field_active = 0 + WHERE field_active = 1 + AND field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; + $this->db->sql_query($sql); + + // Save modified information into a config_text field to recover on enable + $this->config_text->set($profilefield_type_name . '.saved', base64_encode(serialize($pfs))); + + // Log activity + foreach ($pfs as $field_ident) + { + add_log('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', $field_ident); + } + } + + /** + * Purge all profile fields of a certain type + * + * This should be called when an extension which has profile field types + * is purged so that all those profile fields are removed + * + * @param string $profilefield_type_name Type identifier of the profile fields + */ + public function purge_profilefields($profilefield_type_name) + { + // Remove the information saved on disable in a config_text field, not needed any longer + $this->config_text->delete($profilefield_type_name . '.saved'); + + // Get list of profile fields affected by this operation, if any + $pfs = array(); + $sql = 'SELECT field_id, field_ident + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $pfs[(int) $row['field_id']] = $row['field_ident']; + } + $this->db->sql_freeresult($result); + + // If no profile fields exist, then nothing to do + if (!sizeof($pfs)) + { + return; + } + + $this->db->sql_transaction('begin'); + + // Delete entries from all profile field definition tables + $where = $this->db->sql_in_set('field_id', array_keys($pfs)); + $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_TABLE . ' WHERE ' . $where); + $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_LANG_TABLE . ' WHERE ' . $where); + $this->db->sql_query('DELETE FROM ' . PROFILE_LANG_TABLE . ' WHERE ' . $where); + + // Drop columns from the Profile Fields data table + foreach ($pfs as $field_ident) + { + $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_' . $field_ident); + } + + // Reset the order of the remaining fields + $order = 0; + + $sql = 'SELECT * + FROM ' . PROFILE_FIELDS_TABLE . ' + ORDER BY field_order'; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $order++; + if ($row['field_order'] != $order) + { + $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + SET field_order = $order + WHERE field_id = {$row['field_id']}"; + $this->db->sql_query($sql); + } + } + $this->db->sql_freeresult($result); + + $this->db->sql_transaction('commit'); + + // Log activity + foreach ($pfs as $field_ident) + { + add_log('admin', 'LOG_PROFILE_FIELD_REMOVED', $field_ident); + } + } + + /** + * Enable the profile fields of a certain type + * + * This should be called when an extension which has profile field types + * that was disabled is re-enabled so that all those profile fields that + * were disabled are enabled again + * + * @param string $profilefield_type_name Type identifier of the profile fields + */ + public function enable_profilefields($profilefield_type_name) + { + // Read the modified information saved on disable from a config_text field to recover values, then remove it + $pfs = $this->config_text->get($profilefield_type_name . '.saved'); + $this->config_text->delete($profilefield_type_name . '.saved'); + + // If nothing saved, then nothing to do + if ($pfs == '') + { + return; + } + + $pfs = unserialize(base64_decode($pfs)); + + // Restore the affected profile fields to "active" + $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + SET field_active = 1 + WHERE field_active = 0 + AND " . $this->db->sql_in_set('field_id', array_keys($pfs)); + $this->db->sql_query($sql); + foreach ($pfs as $field_ident) + { + add_log('admin', 'LOG_PROFILE_FIELD_ACTIVATE', $field_ident); + } + } } From b89539a70e1fa315b944a0e711208db170ebf079 Mon Sep 17 00:00:00 2001 From: javiexin Date: Fri, 19 Jun 2015 12:46:25 +0200 Subject: [PATCH 02/13] [ticket/13867] Enable/disable mechanism for new profile field types Adds methods to enable, disable and purge profile field types to the profilefields\manager class. These methods are to be called from extensions that add new profile field types on the enable, disable and purge methods of the ext class. If not done, any profile field of a new type that is left after extension is disabled or removed will break the forum in several places. Remove dependency from container, add dependencies with specific classes. Change try/catch by !isset. PHPBB3-13867 --- .../container/services_profilefield.yml | 3 +- phpBB/phpbb/profilefields/manager.php | 35 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/phpBB/config/default/container/services_profilefield.yml b/phpBB/config/default/container/services_profilefield.yml index 8b519e5c7c..968a54ee79 100644 --- a/phpBB/config/default/container/services_profilefield.yml +++ b/phpBB/config/default/container/services_profilefield.yml @@ -2,14 +2,15 @@ services: profilefields.manager: class: phpbb\profilefields\manager arguments: - - '@service_container' - '@auth' - '@dbal.conn' + - '@dbal.tools' - '@dispatcher' - '@request' - '@template' - '@profilefields.type_collection' - '@user' + - '@config_text' - '%tables.profile_fields%' - '%tables.profile_fields_language%' - '%tables.profile_fields_data%' diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index bb76d39f85..a7ce59aeb1 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -13,19 +13,11 @@ namespace phpbb\profilefields; -use Symfony\Component\DependencyInjection\ContainerInterface; - /** * Custom Profile Fields */ class manager { - /** - * Container interface - * @var ContainerInterface - */ - protected $container; - /** * Auth object * @var \phpbb\auth\auth @@ -38,6 +30,12 @@ class manager */ protected $db; + /** + * Database tools object + * @var \phpbb\db\tools + */ + protected $db_tools; + /** * Event dispatcher object * @var \phpbb\event\dispatcher_interface @@ -68,6 +66,12 @@ class manager */ protected $user; + /** + * Config_text object + * @var \phpbb\config\db_text + */ + protected $config_text; + protected $fields_table; protected $fields_language_table; @@ -76,42 +80,37 @@ class manager protected $profile_cache = array(); - protected $config_text; - - protected $db_tools; - /** * Construct * - * @param ContainerInterface $container A container * @param \phpbb\auth\auth $auth Auth object * @param \phpbb\db\driver\driver_interface $db Database object + * @param \phpbb\db\tools $db_tools Database object * @param \phpbb\event\dispatcher_interface $dispatcher Event dispatcher object * @param \phpbb\request\request $request Request object * @param \phpbb\template\template $template Template object * @param \phpbb\di\service_collection $type_collection * @param \phpbb\user $user User object + * @param \phpbb\config\db_text $config_text Config_text object * @param string $fields_table * @param string $fields_language_table * @param string $fields_data_table */ - public function __construct(ContainerInterface $container, \phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\di\service_collection $type_collection, \phpbb\user $user, $fields_table, $fields_language_table, $fields_data_table) + public function __construct(\phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\db\tools $db_tools, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\di\service_collection $type_collection, \phpbb\user $user, \phpbb\config\db_text $config_text, $fields_table, $fields_language_table, $fields_data_table) { - $this->container = $container; $this->auth = $auth; $this->db = $db; + $this->db_tools = $db_tools; $this->dispatcher = $dispatcher; $this->request = $request; $this->template = $template; $this->type_collection = $type_collection; $this->user = $user; + $this->config_text = $config_text; $this->fields_table = $fields_table; $this->fields_language_table = $fields_language_table; $this->fields_data_table = $fields_data_table; - - $this->config_text = $this->container->get('config_text'); - $this->db_tools = $this->container->get('dbal.tools'); } /** From b60322f72931573ff8bfb48d857b9a84a175263d Mon Sep 17 00:00:00 2001 From: javiexin Date: Sun, 25 Dec 2016 14:14:11 +0100 Subject: [PATCH 03/13] [ticket/13867] Enable/disable mechanism for new profile field types Adds methods to enable, disable and purge profile field types to the profilefields\manager class. These methods are to be called from extensions that add new profile field types on the enable, disable and purge methods of the ext class. If not done, any profile field of a new type that is left after extension is disabled or removed will break the forum in several places. Remove dependency from container, add dependencies with specific classes. Change try/catch by !isset. Execute requested changes: more meaningful var names, json_encode, empty. PHPBB3-13867 --- phpBB/phpbb/profilefields/manager.php | 40 +++++++++++++++------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index a7ce59aeb1..b1391eb5bb 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -528,7 +528,7 @@ class manager public function disable_profilefields($profilefield_type_name) { // Get list of profile fields affected by this operation, if any - $pfs = array(); + $profile_fields = array(); $sql = 'SELECT field_id, field_ident FROM ' . PROFILE_FIELDS_TABLE . " WHERE field_active = 1 @@ -536,12 +536,12 @@ class manager $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { - $pfs[(int) $row['field_id']] = $row['field_ident']; + $profile_fields[(int) $row['field_id']] = $row['field_ident']; } $this->db->sql_freeresult($result); // If no profile fields affected, then nothing to do - if (!sizeof($pfs)) + if (!sizeof($profile_fields)) { return; } @@ -554,10 +554,10 @@ class manager $this->db->sql_query($sql); // Save modified information into a config_text field to recover on enable - $this->config_text->set($profilefield_type_name . '.saved', base64_encode(serialize($pfs))); + $this->config_text->set($profilefield_type_name . '.saved', json_encode($profile_fields)); // Log activity - foreach ($pfs as $field_ident) + foreach ($profile_fields as $field_ident) { add_log('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', $field_ident); } @@ -577,19 +577,19 @@ class manager $this->config_text->delete($profilefield_type_name . '.saved'); // Get list of profile fields affected by this operation, if any - $pfs = array(); + $profile_fields = array(); $sql = 'SELECT field_id, field_ident FROM ' . PROFILE_FIELDS_TABLE . " WHERE field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { - $pfs[(int) $row['field_id']] = $row['field_ident']; + $profile_fields[(int) $row['field_id']] = $row['field_ident']; } $this->db->sql_freeresult($result); // If no profile fields exist, then nothing to do - if (!sizeof($pfs)) + if (!sizeof($profile_fields)) { return; } @@ -597,13 +597,13 @@ class manager $this->db->sql_transaction('begin'); // Delete entries from all profile field definition tables - $where = $this->db->sql_in_set('field_id', array_keys($pfs)); + $where = $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_TABLE . ' WHERE ' . $where); $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_LANG_TABLE . ' WHERE ' . $where); $this->db->sql_query('DELETE FROM ' . PROFILE_LANG_TABLE . ' WHERE ' . $where); // Drop columns from the Profile Fields data table - foreach ($pfs as $field_ident) + foreach ($profile_fields as $field_ident) { $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_' . $field_ident); } @@ -632,7 +632,7 @@ class manager $this->db->sql_transaction('commit'); // Log activity - foreach ($pfs as $field_ident) + foreach ($profile_fields as $field_ident) { add_log('admin', 'LOG_PROFILE_FIELD_REMOVED', $field_ident); } @@ -649,25 +649,29 @@ class manager */ public function enable_profilefields($profilefield_type_name) { - // Read the modified information saved on disable from a config_text field to recover values, then remove it - $pfs = $this->config_text->get($profilefield_type_name . '.saved'); - $this->config_text->delete($profilefield_type_name . '.saved'); + // Read the modified information saved on disable from a config_text field to recover values + $profile_fields = $this->config_text->get($profilefield_type_name . '.saved'); // If nothing saved, then nothing to do - if ($pfs == '') + if (empty($profile_fields)) { return; } - $pfs = unserialize(base64_decode($pfs)); + $profile_fields = json_decode($profile_fields, true); // Restore the affected profile fields to "active" $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " SET field_active = 1 WHERE field_active = 0 - AND " . $this->db->sql_in_set('field_id', array_keys($pfs)); + AND " . $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query($sql); - foreach ($pfs as $field_ident) + + // Remove the information saved in the config_text field, not needed any longer + $this->config_text->delete($profilefield_type_name . '.saved'); + + // Log activity + foreach ($profile_fields as $field_ident) { add_log('admin', 'LOG_PROFILE_FIELD_ACTIVATE', $field_ident); } From b1e1ee7d55af22ea16184db7bf405dff4557a940 Mon Sep 17 00:00:00 2001 From: javiexin Date: Fri, 30 Dec 2016 17:09:39 +0100 Subject: [PATCH 04/13] [ticket/13867] Enable/disable mechanism for new profile field types Adds methods to enable, disable and purge profile field types to the profilefields\manager class. These methods are to be called from extensions that add new profile field types on the enable, disable and purge methods of the ext class. If not done, any profile field of a new type that is left after extension is disabled or removed will break the forum in several places. Remove dependency from container, add dependencies with specific classes. Change try/catch by !isset. Execute requested changes: more meaningful var names, json_encode, empty. Execute requested changes: separate a utilit list function, use sql_in_set, use single quotes ('). Remove code approved in a different PR. PHPBB3-13867 --- phpBB/includes/acp/acp_profile.php | 4 -- phpBB/phpbb/profilefields/manager.php | 59 +++++++++++++++------------ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 49da7d84a4..8164c07704 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -783,10 +783,6 @@ class acp_profile $s_one_need_edit = true; } - if (!isset($this->type_collection[$row['field_type']])) - { - continue; - } $profile_field = $this->type_collection[$row['field_type']]; $field_block = array( diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index b1391eb5bb..9c8876e6bc 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -527,18 +527,8 @@ class manager */ public function disable_profilefields($profilefield_type_name) { - // Get list of profile fields affected by this operation, if any - $profile_fields = array(); - $sql = 'SELECT field_id, field_ident - FROM ' . PROFILE_FIELDS_TABLE . " - WHERE field_active = 1 - AND field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $profile_fields[(int) $row['field_id']] = $row['field_ident']; - } - $this->db->sql_freeresult($result); + // Get the list of active profile fields of this type + $profile_fields = $this->list_profilefields($profilefield_type_name, true); // If no profile fields affected, then nothing to do if (!sizeof($profile_fields)) @@ -547,10 +537,10 @@ class manager } // Update the affected profile fields to "inactive" - $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . ' SET field_active = 0 WHERE field_active = 1 - AND field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; + AND ' . $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query($sql); // Save modified information into a config_text field to recover on enable @@ -576,17 +566,8 @@ class manager // Remove the information saved on disable in a config_text field, not needed any longer $this->config_text->delete($profilefield_type_name . '.saved'); - // Get list of profile fields affected by this operation, if any - $profile_fields = array(); - $sql = 'SELECT field_id, field_ident - FROM ' . PROFILE_FIELDS_TABLE . " - WHERE field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'"; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $profile_fields[(int) $row['field_id']] = $row['field_ident']; - } - $this->db->sql_freeresult($result); + // Get the list of all profile fields of this type + $profile_fields = $this->list_profilefields($profilefield_type_name); // If no profile fields exist, then nothing to do if (!sizeof($profile_fields)) @@ -661,10 +642,10 @@ class manager $profile_fields = json_decode($profile_fields, true); // Restore the affected profile fields to "active" - $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . ' SET field_active = 1 WHERE field_active = 0 - AND " . $this->db->sql_in_set('field_id', array_keys($profile_fields)); + AND ' . $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query($sql); // Remove the information saved in the config_text field, not needed any longer @@ -676,4 +657,28 @@ class manager add_log('admin', 'LOG_PROFILE_FIELD_ACTIVATE', $field_ident); } } + + /** + * Get list of profile fields of a certain type, if any + * + * @param string $profilefield_type_name Type identifier of the profile fields + * @param bool $active True to limit output to active profile fields, false for all + * @return array Array with profile field ids as keys and idents as values + */ + private function list_profilefields($profilefield_type_name, $active=false) + { + // Get list of profile fields affected by this operation, if any + $profile_fields = array(); + $sql = 'SELECT field_id, field_ident + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'" . + ($active) ? 'AND field_active = 1' : ''; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $profile_fields[(int) $row['field_id']] = $row['field_ident']; + } + $this->db->sql_freeresult($result); + return $profile_fields; + } } From b3f3886cd90f5d675e552455d2e6eb7b2143972c Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Fri, 30 Dec 2016 15:13:47 +0100 Subject: [PATCH 05/13] [ticket/13867] Add tests PHPBB3-13867 --- phpBB/includes/acp/acp_profile.php | 4 + phpBB/phpbb/profilefields/manager.php | 2 +- tests/profilefields/fixtures/manager.xml | 31 +++++ tests/profilefields/manager_test.php | 137 +++++++++++++++++++++++ tests/test_framework/phpbb_test_case.php | 1 + 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 tests/profilefields/fixtures/manager.xml create mode 100644 tests/profilefields/manager_test.php diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 8164c07704..49da7d84a4 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -783,6 +783,10 @@ class acp_profile $s_one_need_edit = true; } + if (!isset($this->type_collection[$row['field_type']])) + { + continue; + } $profile_field = $this->type_collection[$row['field_type']]; $field_block = array( diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index 9c8876e6bc..d462d6d4f7 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -665,7 +665,7 @@ class manager * @param bool $active True to limit output to active profile fields, false for all * @return array Array with profile field ids as keys and idents as values */ - private function list_profilefields($profilefield_type_name, $active=false) + private function list_profilefields($profilefield_type_name, $active = false) { // Get list of profile fields affected by this operation, if any $profile_fields = array(); diff --git a/tests/profilefields/fixtures/manager.xml b/tests/profilefields/fixtures/manager.xml new file mode 100644 index 0000000000..b0b118bb13 --- /dev/null +++ b/tests/profilefields/fixtures/manager.xml @@ -0,0 +1,31 @@ + + + + field_id + field_ident + field_active + field_type + field_order + + 1 + pf_1 + 1 + foo_bar_type + 1 + + + 2 + pf_2 + 1 + foo_bar_type + 2 + + + 3 + pf_3 + 1 + other_type + 3 + +
+ diff --git a/tests/profilefields/manager_test.php b/tests/profilefields/manager_test.php new file mode 100644 index 0000000000..76bc63124f --- /dev/null +++ b/tests/profilefields/manager_test.php @@ -0,0 +1,137 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class manager_test extends phpbb_database_test_case +{ + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \phpbb\config\db_text */ + protected $config_text; + + /** @var \phpbb\profilefields\manager */ + protected $manager; + + /** @var \phpbb\log\log_interface */ + protected $log; + + /** @var \phpbb\db\tools */ + protected $db_tools; + + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/manager.xml'); + } + + public function setUp() + { + parent::setUp(); + + global $phpbb_log; + + $this->log = $this->prophesize('\phpbb\log\log_interface'); + $phpbb_log = $this->log->reveal(); + + $this->db = $this->new_dbal(); + $this->config_text = $this->prophesize('\phpbb\config\db_text'); + $this->db_tools = $this->prophesize('\phpbb\db\tools'); + + $this->manager = new \phpbb\profilefields\manager( + $this->prophesize('phpbb\auth\auth')->reveal(), + $this->db, + $this->db_tools->reveal(), + $this->prophesize('\phpbb\event\dispatcher_interface')->reveal(), + $this->prophesize('\phpbb\request\request')->reveal(), + $this->prophesize('\phpbb\template\template')->reveal(), + $this->prophesize('\phpbb\di\service_collection')->reveal(), + $this->prophesize('\phpbb\user')->reveal(), + $this->config_text->reveal(), + PROFILE_FIELDS_TABLE, + PROFILE_FIELDS_LANG_TABLE, + PROFILE_FIELDS_DATA_TABLE + ); + } + + public function test_disable_profilefields() + { + $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_1')->shouldBeCalled(); + $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_2')->shouldBeCalled(); + + $this->config_text->set('foo_bar_type.saved', json_encode([1 => 'pf_1', 2 => 'pf_2']))->shouldBeCalled(); + + $this->manager->disable_profilefields('foo_bar_type'); + + $sql = 'SELECT field_id, field_ident + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_active = 1 + AND field_type = 'foo_bar_type'"; + + $this->assertSqlResultEquals(false, $sql, 'All profile fields should be disabled'); + } + + public function test_enable_profilefields() + { + $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_1')->shouldBeCalled(); + $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_2')->shouldBeCalled(); + + $this->config_text->get('foo_bar_type.saved')->willReturn(json_encode([1 => 'pf_1', 2 => 'pf_2'])); + $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); + + $this->manager->enable_profilefields('foo_bar_type'); + + $sql = 'SELECT field_id + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_active = 1 + AND field_type = 'foo_bar_type'"; + + $this->assertSqlResultEquals([ + ['field_id' => '1'], + ['field_id' => '2'], + ], $sql, 'All profile fields should be enabled'); + } + + public function test_purge_profilefields() + { + $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_1')->shouldBeCalled(); + $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_2')->shouldBeCalled(); + + $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); + + $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_1')->shouldBeCalled(); + $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_2')->shouldBeCalled(); + + $this->manager->enable_profilefields('foo_bar_type'); + + $sql = 'SELECT field_id + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_type = 'foo_bar_type'"; + + $this->assertSqlResultEquals(false, $sql, 'All profile fields should be removed'); + + $sql = 'SELECT field_id + FROM ' . PROFILE_FIELDS_LANG_TABLE . " + WHERE field_type = 'foo_bar_type'"; + + $this->assertSqlResultEquals(false, $sql, 'All profile fields lang should be removed'); + + $sql = 'SELECT field_id + FROM ' . PROFILE_LANG_TABLE . " + WHERE field_type = 'foo_bar_type'"; + + $this->assertSqlResultEquals(false, $sql, 'All profile fields lang should be removed'); + + $sql = 'SELECT field_id, field_order FROM ' . PROFILE_FIELDS_TABLE; + + $this->assertSqlResultEquals([['field_id' => '3', 'field_order' => '1'],], $sql, 'Profile fields order should be recalculated, starting by 1'); + } +} diff --git a/tests/test_framework/phpbb_test_case.php b/tests/test_framework/phpbb_test_case.php index 01d26fb67d..0bf464567f 100644 --- a/tests/test_framework/phpbb_test_case.php +++ b/tests/test_framework/phpbb_test_case.php @@ -27,6 +27,7 @@ class phpbb_test_case extends PHPUnit_Framework_TestCase 'PHP_Token_Stream_CachingFactory' => array('cache'), 'phpbb_database_test_case' => array('already_connected', 'last_post_timestamp'), + 'Prophecy\Doubler\NameGenerator' => array('counter'), ); } From 84cd7e496b1140f39f781c35cfbd87be45b9711d Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 23 Mar 2020 20:56:15 +0100 Subject: [PATCH 06/13] [ticket/13867] Amend profile fields manager PHPBB3-13867 --- .../container/services_profilefield.yml | 7 +- phpBB/phpbb/profilefields/manager.php | 648 +++++++++--------- tests/profilefields/manager_test.php | 2 +- 3 files changed, 344 insertions(+), 313 deletions(-) diff --git a/phpBB/config/default/container/services_profilefield.yml b/phpBB/config/default/container/services_profilefield.yml index 968a54ee79..ebbd3fbf8e 100644 --- a/phpBB/config/default/container/services_profilefield.yml +++ b/phpBB/config/default/container/services_profilefield.yml @@ -3,17 +3,20 @@ services: class: phpbb\profilefields\manager arguments: - '@auth' + - '@config_text' - '@dbal.conn' - '@dbal.tools' - '@dispatcher' + - '@language' + - '@log' - '@request' - '@template' - '@profilefields.type_collection' - '@user' - - '@config_text' - '%tables.profile_fields%' - - '%tables.profile_fields_language%' - '%tables.profile_fields_data%' + - '%tables.profile_fields_options_language%' + - '%tables.profile_fields_language%' profilefields.lang_helper: class: phpbb\profilefields\lang_helper diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index d462d6d4f7..2e26f62355 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -1,125 +1,139 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\profilefields; /** -* Custom Profile Fields -*/ + * Custom Profile Fields (CPF) manager. + */ class manager { - /** - * Auth object - * @var \phpbb\auth\auth - */ + /** @var \phpbb\auth\auth */ protected $auth; - /** - * Database object - * @var \phpbb\db\driver\driver_interface - */ - protected $db; - - /** - * Database tools object - * @var \phpbb\db\tools - */ - protected $db_tools; - - /** - * Event dispatcher object - * @var \phpbb\event\dispatcher_interface - */ - protected $dispatcher; - - /** - * Request object - * @var \phpbb\request\request - */ - protected $request; - - /** - * Template object - * @var \phpbb\template\template - */ - protected $template; - - /** - * Service Collection object - * @var \phpbb\di\service_collection - */ - protected $type_collection; - - /** - * User object - * @var \phpbb\user - */ - protected $user; - - /** - * Config_text object - * @var \phpbb\config\db_text - */ + /** @var \phpbb\config\db_text */ protected $config_text; + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \phpbb\db\tools\tools */ + protected $db_tools; + + /** @var \phpbb\event\dispatcher_interface */ + protected $dispatcher; + + /** @var \phpbb\language\language */ + protected $language; + + /** @var \phpbb\log\log */ + protected $log; + + /** @var \phpbb\request\request */ + protected $request; + + /** @var \phpbb\template\template */ + protected $template; + + /** @var \phpbb\di\service_collection */ + protected $type_collection; + + /** @var \phpbb\user */ + protected $user; + + /** @var string Profile fields table */ protected $fields_table; - protected $fields_language_table; - + /** @var string Profile fields data table */ protected $fields_data_table; - protected $profile_cache = array(); + /** @var string Profile fields data (options) table */ + protected $fields_data_lang_table; + + /** @var string Profile fields language table */ + protected $fields_lang_table; + + /** @var array Users custom profile fields cache */ + protected $profile_cache = []; /** - * Construct - * - * @param \phpbb\auth\auth $auth Auth object - * @param \phpbb\db\driver\driver_interface $db Database object - * @param \phpbb\db\tools $db_tools Database object - * @param \phpbb\event\dispatcher_interface $dispatcher Event dispatcher object - * @param \phpbb\request\request $request Request object - * @param \phpbb\template\template $template Template object - * @param \phpbb\di\service_collection $type_collection - * @param \phpbb\user $user User object - * @param \phpbb\config\db_text $config_text Config_text object - * @param string $fields_table - * @param string $fields_language_table - * @param string $fields_data_table - */ - public function __construct(\phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\db\tools $db_tools, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\di\service_collection $type_collection, \phpbb\user $user, \phpbb\config\db_text $config_text, $fields_table, $fields_language_table, $fields_data_table) + * Construct + * + * @param \phpbb\auth\auth $auth Auth object + * @param \phpbb\config\db_text $config_text Config_text object + * @param \phpbb\db\driver\driver_interface $db Database object + * @param \phpbb\db\tools\tools $db_tools Database object + * @param \phpbb\event\dispatcher_interface $dispatcher Event dispatcher object + * @param \phpbb\language\language $language Language object + * @param \phpbb\log\log $log Log object + * @param \phpbb\request\request $request Request object + * @param \phpbb\template\template $template Template object + * @param \phpbb\di\service_collection $type_collection CPF Type collection + * @param \phpbb\user $user User object + * @param string $fields_table CPF Table + * @param string $fields_data_table CPF Data table + * @param string $fields_data_lang_table CPF Data language table + * @param string $fields_lang_table CPF Language table + */ + public function __construct( + \phpbb\auth\auth $auth, + \phpbb\config\db_text $config_text, + \phpbb\db\driver\driver_interface $db, + \phpbb\db\tools\tools $db_tools, + \phpbb\event\dispatcher_interface $dispatcher, + \phpbb\language\language $language, + \phpbb\log\log $log, + \phpbb\request\request $request, + \phpbb\template\template $template, + \phpbb\di\service_collection $type_collection, + \phpbb\user $user, + $fields_table, + $fields_data_table, + $fields_lang_table, + $fields_data_lang_table + ) { - $this->auth = $auth; - $this->db = $db; - $this->db_tools = $db_tools; - $this->dispatcher = $dispatcher; - $this->request = $request; - $this->template = $template; - $this->type_collection = $type_collection; - $this->user = $user; - $this->config_text = $config_text; + $this->auth = $auth; + $this->config_text = $config_text; + $this->db = $db; + $this->db_tools = $db_tools; + $this->dispatcher = $dispatcher; + $this->language = $language; + $this->log = $log; + $this->request = $request; + $this->template = $template; + $this->type_collection = $type_collection; + $this->user = $user; - $this->fields_table = $fields_table; - $this->fields_language_table = $fields_language_table; - $this->fields_data_table = $fields_data_table; + $this->fields_table = $fields_table; + $this->fields_lang_table = $fields_lang_table; + $this->fields_data_table = $fields_data_table; + $this->fields_data_lang_table = $fields_data_lang_table; } /** - * Assign editable fields to template, mode can be profile (for profile change) or register (for registration) - * Called by ucp_profile and ucp_register - */ + * Assign editable fields to template. + * + * Called by ucp_profile and ucp_register. + * + * @param string $mode The mode (profile|register) + * @param int $lang_id The language identifier + * @return void + */ public function generate_profile_fields($mode, $lang_id) { $sql_where = ''; + switch ($mode) { case 'register': @@ -136,54 +150,57 @@ class manager break; default: - trigger_error('Wrong profile mode specified', E_USER_ERROR); + trigger_error('NO_MODE', E_USER_ERROR); break; } $sql = 'SELECT l.*, f.* - FROM ' . $this->fields_language_table . ' l, ' . $this->fields_table . " f - WHERE f.field_active = 1 - $sql_where - AND l.lang_id = " . (int) $lang_id . ' - AND l.field_id = f.field_id - ORDER BY f.field_order'; + FROM ' . $this->fields_lang_table . ' l, + ' . $this->fields_table . ' f + WHERE l.field_id = f.field_id + AND f.field_active = 1 + AND l.lang_id = ' . (int) $lang_id + . $sql_where . ' + ORDER BY f.field_order ASC'; $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) { - // Return templated field + /** @var \phpbb\profilefields\type\type_interface $profile_field */ $profile_field = $this->type_collection[$row['field_type']]; - $tpl_snippet = $profile_field->process_field_row('change', $row); - $this->template->assign_block_vars('profile_fields', array( - 'LANG_NAME' => $this->user->lang($row['lang_name']), - 'LANG_EXPLAIN' => $this->user->lang($row['lang_explain']), - 'FIELD' => $tpl_snippet, + $this->template->assign_block_vars('profile_fields', [ + 'FIELD' => $profile_field->process_field_row('change', $row), 'FIELD_ID' => $profile_field->get_field_ident($row), - 'S_REQUIRED' => ($row['field_required']) ? true : false, - )); + 'LANG_NAME' => $this->language->lang($row['lang_name']), + 'LANG_EXPLAIN' => $this->language->lang($row['lang_explain']), + 'S_REQUIRED' => (bool) $row['field_required'], + ]); } $this->db->sql_freeresult($result); } /** - * Build profile cache, used for display - */ + * Build profile cache, used for display. + * + * @return void + */ protected function build_cache() { - $this->profile_cache = array(); + $this->profile_cache = []; // Display hidden/no_view fields for admin/moderator - $sql = 'SELECT l.*, f.* - FROM ' . $this->fields_language_table . ' l, ' . $this->fields_table . ' f - WHERE l.lang_id = ' . $this->user->get_iso_lang_id() . ' - AND f.field_active = 1 ' . - ((!$this->auth->acl_gets('a_', 'm_') && !$this->auth->acl_getf_global('m_')) ? ' AND f.field_hide = 0 ' : '') . ' - AND f.field_no_view = 0 - AND l.field_id = f.field_id - ORDER BY f.field_order'; - $result = $this->db->sql_query($sql); + $sql_where = !$this->auth->acl_gets('a_', 'm_') && !$this->auth->acl_getf_global('m_') ? ' AND f.field_hide = 0' : ''; + $sql = 'SELECT l.*, f.* + FROM ' . $this->fields_lang_table . ' l, + ' . $this->fields_table . ' f + WHERE l.field_id = f.field_id + AND f.field_active = 1 + AND f.field_no_view = 0 + AND l.lang_id = ' . $this->user->get_iso_lang_id() + . $sql_where . ' + ORDER BY f.field_order ASC'; + $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { $this->profile_cache[$row['field_ident']] = $row; @@ -192,11 +209,17 @@ class manager } /** - * Submit profile field for validation - */ + * Submit profile field for validation. + * + * @param string $mode The mode (profile|register) + * @param int $lang_id The language identifier + * @param array $cp_data Custom profile field data + * @param array $cp_error Custom profile field errors + */ public function submit_cp_field($mode, $lang_id, &$cp_data, &$cp_error) { $sql_where = ''; + switch ($mode) { case 'register': @@ -213,21 +236,22 @@ class manager break; default: - trigger_error('Wrong profile mode specified', E_USER_ERROR); + trigger_error('NO_MODE', E_USER_ERROR); break; } $sql = 'SELECT l.*, f.* - FROM ' . $this->fields_language_table . ' l, ' . $this->fields_table . ' f - WHERE l.lang_id = ' . (int) $lang_id . " + FROM ' . $this->fields_lang_table . ' l, + ' . $this->fields_table . ' f + WHERE l.field_id = f.field_id AND f.field_active = 1 - $sql_where - AND l.field_id = f.field_id - ORDER BY f.field_order"; + AND l.lang_id = ' . (int) $lang_id + . $sql_where . ' + ORDER BY f.field_order'; $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) { + /** @var \phpbb\profilefields\type\type_interface $profile_field */ $profile_field = $this->type_collection[$row['field_type']]; $cp_data['pf_' . $row['field_ident']] = $profile_field->get_profile_field($row); $check_value = $cp_data['pf_' . $row['field_ident']]; @@ -242,11 +266,14 @@ class manager } /** - * Update profile field data directly - */ + * Update profile field data directly. + * + * @param int $user_id The user identifier + * @param array $cp_data Custom profile field data + */ public function update_profile_field_data($user_id, $cp_data) { - if (!count($cp_data)) + if (empty($cp_data)) { return; } @@ -261,25 +288,26 @@ class manager $cp_data = $this->build_insert_sql_array($cp_data); $cp_data['user_id'] = (int) $user_id; - $sql = 'INSERT INTO ' . $this->fields_data_table . ' ' . $this->db->sql_build_array('INSERT', $cp_data); + $sql = 'INSERT INTO ' . $this->fields_data_table . $this->db->sql_build_array('INSERT', $cp_data); $this->db->sql_query($sql); } } /** - * Generate the template arrays in order to display the column names - * - * @param string $restrict_option Restrict the published fields to a certain profile field option - * @return array Returns an array with the template variables type, name and explain for the fields to display - */ + * Generate the template arrays in order to display the column names. + * + * @param string $restrict_option Restrict the published fields to a certain profile field option + * @return array Returns an array with the template variables type, + * name and explain for the fields to display + */ public function generate_profile_fields_template_headlines($restrict_option = '') { - if (!count($this->profile_cache)) + if (empty($this->profile_cache)) { $this->build_cache(); } - $tpl_fields = array(); + $tpl_fields = []; // Go through the fields in correct order foreach ($this->profile_cache as $field_ident => $field_data) @@ -289,32 +317,29 @@ class manager continue; } + /** @var \phpbb\profilefields\type\type_interface $profile_field */ $profile_field = $this->type_collection[$field_data['field_type']]; - $tpl_fields[] = array( + $tpl_fields[] = [ 'PROFILE_FIELD_IDENT' => $field_ident, 'PROFILE_FIELD_TYPE' => $field_data['field_type'], 'PROFILE_FIELD_NAME' => $profile_field->get_field_name($field_data['lang_name']), - 'PROFILE_FIELD_EXPLAIN' => $this->user->lang($field_data['lang_explain']), - ); + 'PROFILE_FIELD_EXPLAIN' => $this->language->lang($field_data['lang_explain']), + ]; } $profile_cache = $this->profile_cache; /** - * Event to modify template headlines of the generated profile fields - * - * @event core.generate_profile_fields_template_headlines - * @var string restrict_option Restrict the published fields to a certain profile field option - * @var array tpl_fields Array with template data fields - * @var array profile_cache A copy of the profile cache to make additional checks - * @since 3.1.6-RC1 - */ - $vars = array( - 'restrict_option', - 'tpl_fields', - 'profile_cache', - ); + * Event to modify template headlines of the generated profile fields + * + * @event core.generate_profile_fields_template_headlines + * @var string restrict_option Restrict the published fields to a certain profile field option + * @var array tpl_fields Array with template data fields + * @var array profile_cache A copy of the profile cache to make additional checks + * @since 3.1.6-RC1 + */ + $vars = ['restrict_option', 'tpl_fields', 'profile_cache']; extract($this->dispatcher->trigger_event('core.generate_profile_fields_template_headlines', compact($vars))); unset($profile_cache); @@ -322,52 +347,44 @@ class manager } /** - * Grab the user specific profile fields data - * - * @param int|array $user_ids Single user id or an array of ids - * @return array Users profile fields data - */ + * Grab the user specific profile fields data. + * + * @param int|array $user_ids Single user id or an array of ids + * @return array Users profile fields data + */ public function grab_profile_fields_data($user_ids = 0) { - if (!is_array($user_ids)) - { - $user_ids = array($user_ids); - } - - if (!count($this->profile_cache)) + if (empty($this->profile_cache)) { $this->build_cache(); } - if (!count($user_ids)) + if (empty($user_ids)) { - return array(); + return []; } $sql = 'SELECT * FROM ' . $this->fields_data_table . ' - WHERE ' . $this->db->sql_in_set('user_id', array_map('intval', $user_ids)); + WHERE ' . $this->db->sql_in_set('user_id', array_map('intval', (array) $user_ids)); $result = $this->db->sql_query($sql); - - $field_data = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $field_data[$row['user_id']] = $row; - } + $rowset = $this->db->sql_fetchrowset($result); $this->db->sql_freeresult($result); + $field_data = array_column($rowset, null, 'user_id'); + /** - * Event to modify profile fields data retrieved from the database - * - * @event core.grab_profile_fields_data - * @var array user_ids Single user id or an array of ids - * @var array field_data Array with profile fields data - * @since 3.1.0-b3 - */ - $vars = array('user_ids', 'field_data'); + * Event to modify profile fields data retrieved from the database + * + * @event core.grab_profile_fields_data + * @var array user_ids Single user id or an array of ids + * @var array field_data Array with profile fields data + * @since 3.1.0-b3 + */ + $vars = ['user_ids', 'field_data']; extract($this->dispatcher->trigger_event('core.grab_profile_fields_data', compact($vars))); - $user_fields = array(); + $user_fields = []; // Go through the fields in correct order foreach (array_keys($this->profile_cache) as $used_ident) @@ -392,34 +409,39 @@ class manager } /** - * Assign the user's profile fields data to the template - * - * @param array $profile_row Array with users profile field data - * @param bool $use_contact_fields Should we display contact fields as such? - * This requires special treatments (links should not be parsed in the values, and more) - * @return array - */ + * Generate the user's profile fields data for the template. + * + * @param array $profile_row Array with users profile field data + * @param bool $use_contact_fields Should we display contact fields as such? + * This requires special treatments: + * (links should not be parsed in the values, and more) + * @return array The user's profile fields data + */ public function generate_profile_fields_template_data($profile_row, $use_contact_fields = true) { // $profile_row == $user_fields[$row['user_id']]; - $tpl_fields = array(); - $tpl_fields['row'] = $tpl_fields['blockrow'] = array(); + $tpl_fields = [ + 'row' => [], + 'blockrow' => [], + ]; /** - * Event to modify data of the generated profile fields, before the template assignment loop - * - * @event core.generate_profile_fields_template_data_before - * @var array profile_row Array with users profile field data - * @var array tpl_fields Array with template data fields - * @var bool use_contact_fields Should we display contact fields as such? - * @since 3.1.0-b3 - */ - $vars = array('profile_row', 'tpl_fields', 'use_contact_fields'); + * Event to modify data of the generated profile fields, before the template assignment loop + * + * @event core.generate_profile_fields_template_data_before + * @var array profile_row Array with users profile field data + * @var array tpl_fields Array with template data fields + * @var bool use_contact_fields Should we display contact fields as such? + * @since 3.1.0-b3 + */ + $vars = ['profile_row', 'tpl_fields', 'use_contact_fields']; extract($this->dispatcher->trigger_event('core.generate_profile_fields_template_data_before', compact($vars))); foreach ($profile_row as $ident => $ident_ary) { + /** @var \phpbb\profilefields\type\type_interface $profile_field */ $profile_field = $this->type_collection[$ident_ary['data']['field_type']]; + $value = $profile_field->get_profile_value($ident_ary['value'], $ident_ary['data']); $value_raw = $profile_field->get_profile_value_raw($ident_ary['value'], $ident_ary['data']); @@ -428,88 +450,99 @@ class manager continue; } - $field_desc = $contact_url = ''; + $field_desc = ''; + $contact_url = ''; + $ident_upper = strtoupper($ident); + if ($use_contact_fields && $ident_ary['data']['field_is_contact']) { $value = $profile_field->get_profile_contact_value($ident_ary['value'], $ident_ary['data']); - $field_desc = $this->user->lang($ident_ary['data']['field_contact_desc']); + $field_desc = $this->language->lang($ident_ary['data']['field_contact_desc']); + if (strpos($field_desc, '%s') !== false) { $field_desc = sprintf($field_desc, $value); } - $contact_url = ''; + if (strpos($ident_ary['data']['field_contact_url'], '%s') !== false) { $contact_url = sprintf($ident_ary['data']['field_contact_url'], $value); } } - $tpl_fields['row'] += array( - 'PROFILE_' . strtoupper($ident) . '_IDENT' => $ident, - 'PROFILE_' . strtoupper($ident) . '_VALUE' => $value, - 'PROFILE_' . strtoupper($ident) . '_VALUE_RAW' => $value_raw, - 'PROFILE_' . strtoupper($ident) . '_CONTACT' => $contact_url, - 'PROFILE_' . strtoupper($ident) . '_DESC' => $field_desc, - 'PROFILE_' . strtoupper($ident) . '_TYPE' => $ident_ary['data']['field_type'], - 'PROFILE_' . strtoupper($ident) . '_NAME' => $this->user->lang($ident_ary['data']['lang_name']), - 'PROFILE_' . strtoupper($ident) . '_EXPLAIN' => $this->user->lang($ident_ary['data']['lang_explain']), + $tpl_fields['row'] += [ + "PROFILE_{$ident_upper}_IDENT" => $ident, + "PROFILE_{$ident_upper}_VALUE" => $value, + "PROFILE_{$ident_upper}_VALUE_RAW" => $value_raw, + "PROFILE_{$ident_upper}_CONTACT" => $contact_url, + "PROFILE_{$ident_upper}_DESC" => $field_desc, + "PROFILE_{$ident_upper}_TYPE" => $ident_ary['data']['field_type'], + "PROFILE_{$ident_upper}_NAME" => $this->language->lang($ident_ary['data']['lang_name']), + "PROFILE_{$ident_upper}_EXPLAIN" => $this->language->lang($ident_ary['data']['lang_explain']), - 'S_PROFILE_' . strtoupper($ident) . '_CONTACT' => $ident_ary['data']['field_is_contact'], - 'S_PROFILE_' . strtoupper($ident) => true, - ); + "S_PROFILE_{$ident_upper}_CONTACT" => $ident_ary['data']['field_is_contact'], + "S_PROFILE_{$ident_upper}" => true, + ]; - $tpl_fields['blockrow'][] = array( + $tpl_fields['blockrow'][] = [ 'PROFILE_FIELD_IDENT' => $ident, 'PROFILE_FIELD_VALUE' => $value, 'PROFILE_FIELD_VALUE_RAW' => $value_raw, 'PROFILE_FIELD_CONTACT' => $contact_url, 'PROFILE_FIELD_DESC' => $field_desc, 'PROFILE_FIELD_TYPE' => $ident_ary['data']['field_type'], - 'PROFILE_FIELD_NAME' => $this->user->lang($ident_ary['data']['lang_name']), - 'PROFILE_FIELD_EXPLAIN' => $this->user->lang($ident_ary['data']['lang_explain']), + 'PROFILE_FIELD_NAME' => $this->language->lang($ident_ary['data']['lang_name']), + 'PROFILE_FIELD_EXPLAIN' => $this->language->lang($ident_ary['data']['lang_explain']), - 'S_PROFILE_CONTACT' => $ident_ary['data']['field_is_contact'], - 'S_PROFILE_' . strtoupper($ident) => true, - ); + 'S_PROFILE_CONTACT' => $ident_ary['data']['field_is_contact'], + "S_PROFILE_{$ident_upper}" => true, + ]; } /** - * Event to modify template data of the generated profile fields - * - * @event core.generate_profile_fields_template_data - * @var array profile_row Array with users profile field data - * @var array tpl_fields Array with template data fields - * @var bool use_contact_fields Should we display contact fields as such? - * @since 3.1.0-b3 - */ - $vars = array('profile_row', 'tpl_fields', 'use_contact_fields'); + * Event to modify template data of the generated profile fields + * + * @event core.generate_profile_fields_template_data + * @var array profile_row Array with users profile field data + * @var array tpl_fields Array with template data fields + * @var bool use_contact_fields Should we display contact fields as such? + * @since 3.1.0-b3 + */ + $vars = ['profile_row', 'tpl_fields', 'use_contact_fields']; extract($this->dispatcher->trigger_event('core.generate_profile_fields_template_data', compact($vars))); return $tpl_fields; } /** - * Build Array for user insertion into custom profile fields table - */ + * Build array for the custom profile fields table. + * + * @param array $cp_data Custom profile field data + * @return array Custom profile field data for SQL usage + */ public function build_insert_sql_array($cp_data) { - $sql_not_in = array(); + $prefix = 'pf_'; + $length = strlen($prefix); + $not_in = []; + foreach ($cp_data as $key => $null) { - $sql_not_in[] = (strncmp($key, 'pf_', 3) === 0) ? substr($key, 3) : $key; + $not_in[] = strncmp($key, $prefix, $length) === 0 ? substr($key, $length) : $key; } $sql = 'SELECT f.field_type, f.field_ident, f.field_default_value, l.lang_default_value - FROM ' . $this->fields_language_table . ' l, ' . $this->fields_table . ' f - WHERE l.lang_id = ' . $this->user->get_iso_lang_id() . ' - ' . ((count($sql_not_in)) ? ' AND ' . $this->db->sql_in_set('f.field_ident', $sql_not_in, true) : '') . ' - AND l.field_id = f.field_id'; + FROM ' . $this->fields_lang_table . ' l, + ' . $this->fields_table . ' f + WHERE l.field_id = f.field_id + AND l.lang_id = ' . $this->user->get_iso_lang_id() . + (!empty($not_in) ? ' AND ' . $this->db->sql_in_set('f.field_ident', $not_in, true) : ''); $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) { + /** @var \phpbb\profilefields\type\type_interface $profile_field */ $profile_field = $this->type_collection[$row['field_type']]; - $cp_data['pf_' . $row['field_ident']] = $profile_field->get_default_field_value($row); + $cp_data[$prefix . $row['field_ident']] = $profile_field->get_default_field_value($row); } $this->db->sql_freeresult($result); @@ -517,60 +550,59 @@ class manager } /** - * Disable all profile fields of a certain type - * - * This should be called when an extension which has profile field types - * is disabled so that all those profile fields are hidden and do not - * cause errors - * - * @param string $profilefield_type_name Type identifier of the profile fields - */ - public function disable_profilefields($profilefield_type_name) + * Disable all profile fields of a certain type. + * + * This should be called when an extension which has profile field types is disabled + * so that all those profile fields are hidden and do not cause errors. + * + * @param string $type_name Type identifier of the profile fields + */ + public function disable_profilefields($type_name) { // Get the list of active profile fields of this type - $profile_fields = $this->list_profilefields($profilefield_type_name, true); + $profile_fields = $this->list_profilefields($type_name, true); // If no profile fields affected, then nothing to do - if (!sizeof($profile_fields)) + if (empty($profile_fields)) { return; } // Update the affected profile fields to "inactive" - $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . ' + $sql = 'UPDATE ' . $this->fields_table . ' SET field_active = 0 WHERE field_active = 1 AND ' . $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query($sql); // Save modified information into a config_text field to recover on enable - $this->config_text->set($profilefield_type_name . '.saved', json_encode($profile_fields)); + $this->config_text->set($type_name . '.saved', json_encode($profile_fields)); // Log activity foreach ($profile_fields as $field_ident) { - add_log('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', $field_ident); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_PROFILE_FIELD_DEACTIVATE', time(), [$field_ident]); } } /** - * Purge all profile fields of a certain type - * - * This should be called when an extension which has profile field types - * is purged so that all those profile fields are removed - * - * @param string $profilefield_type_name Type identifier of the profile fields - */ - public function purge_profilefields($profilefield_type_name) + * Purge all profile fields of a certain type. + * + * This should be called when an extension which has profile field types is purged + * so that all those profile fields are removed. + * + * @param string $type_name Type identifier of the profile fields + */ + public function purge_profilefields($type_name) { // Remove the information saved on disable in a config_text field, not needed any longer - $this->config_text->delete($profilefield_type_name . '.saved'); + $this->config_text->delete($type_name . '.saved'); // Get the list of all profile fields of this type - $profile_fields = $this->list_profilefields($profilefield_type_name); + $profile_fields = $this->list_profilefields($type_name); // If no profile fields exist, then nothing to do - if (!sizeof($profile_fields)) + if (empty($profile_fields)) { return; } @@ -579,30 +611,30 @@ class manager // Delete entries from all profile field definition tables $where = $this->db->sql_in_set('field_id', array_keys($profile_fields)); - $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_TABLE . ' WHERE ' . $where); - $this->db->sql_query('DELETE FROM ' . PROFILE_FIELDS_LANG_TABLE . ' WHERE ' . $where); - $this->db->sql_query('DELETE FROM ' . PROFILE_LANG_TABLE . ' WHERE ' . $where); + $this->db->sql_query('DELETE FROM ' . $this->fields_table . ' WHERE ' . $where); + $this->db->sql_query('DELETE FROM ' . $this->fields_data_lang_table . ' WHERE ' . $where); + $this->db->sql_query('DELETE FROM ' . $this->fields_lang_table . ' WHERE ' . $where); // Drop columns from the Profile Fields data table foreach ($profile_fields as $field_ident) { - $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_' . $field_ident); + $this->db_tools->sql_column_remove($this->fields_data_table, 'pf_' . $field_ident); } // Reset the order of the remaining fields $order = 0; $sql = 'SELECT * - FROM ' . PROFILE_FIELDS_TABLE . ' + FROM ' . $this->fields_table . ' ORDER BY field_order'; $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) { $order++; + if ($row['field_order'] != $order) { - $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " + $sql = 'UPDATE ' . $this->fields_table . " SET field_order = $order WHERE field_id = {$row['field_id']}"; $this->db->sql_query($sql); @@ -615,23 +647,22 @@ class manager // Log activity foreach ($profile_fields as $field_ident) { - add_log('admin', 'LOG_PROFILE_FIELD_REMOVED', $field_ident); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_PROFILE_FIELD_REMOVED', time(), [$field_ident]); } } /** - * Enable the profile fields of a certain type - * - * This should be called when an extension which has profile field types - * that was disabled is re-enabled so that all those profile fields that - * were disabled are enabled again - * - * @param string $profilefield_type_name Type identifier of the profile fields - */ - public function enable_profilefields($profilefield_type_name) + * Enable the profile fields of a certain type. + * + * This should be called when an extension which has profile field types that was disabled is re-enabled + * so that all those profile fields that were disabled are enabled again. + * + * @param string $type_name Type identifier of the profile fields + */ + public function enable_profilefields($type_name) { // Read the modified information saved on disable from a config_text field to recover values - $profile_fields = $this->config_text->get($profilefield_type_name . '.saved'); + $profile_fields = $this->config_text->get($type_name . '.saved'); // If nothing saved, then nothing to do if (empty($profile_fields)) @@ -639,46 +670,43 @@ class manager return; } - $profile_fields = json_decode($profile_fields, true); + $profile_fields = (array) json_decode($profile_fields, true); // Restore the affected profile fields to "active" - $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . ' + $sql = 'UPDATE ' . $this->fields_table . ' SET field_active = 1 WHERE field_active = 0 AND ' . $this->db->sql_in_set('field_id', array_keys($profile_fields)); $this->db->sql_query($sql); // Remove the information saved in the config_text field, not needed any longer - $this->config_text->delete($profilefield_type_name . '.saved'); + $this->config_text->delete($type_name . '.saved'); // Log activity foreach ($profile_fields as $field_ident) { - add_log('admin', 'LOG_PROFILE_FIELD_ACTIVATE', $field_ident); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_PROFILE_FIELD_ACTIVATE', time(), [$field_ident]); } } /** - * Get list of profile fields of a certain type, if any - * - * @param string $profilefield_type_name Type identifier of the profile fields - * @param bool $active True to limit output to active profile fields, false for all - * @return array Array with profile field ids as keys and idents as values - */ - private function list_profilefields($profilefield_type_name, $active = false) + * Get list of profile fields of a certain type, if any + * + * @param string $type_name Type identifier of the profile fields + * @param bool $active True to limit output to active profile fields, false for all + * @return array Array with profile field ids as keys and idents as values + */ + private function list_profilefields($type_name, $active = false) { // Get list of profile fields affected by this operation, if any - $profile_fields = array(); $sql = 'SELECT field_id, field_ident - FROM ' . PROFILE_FIELDS_TABLE . " - WHERE field_type = '" . $this->db->sql_escape($profilefield_type_name) . "'" . - ($active) ? 'AND field_active = 1' : ''; + FROM ' . $this->fields_table . " + WHERE field_type = '" . $this->db->sql_escape($type_name) . "'" . + ($active ? ' AND field_active = 1' : ''); $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $profile_fields[(int) $row['field_id']] = $row['field_ident']; - } + $rowset = $this->db->sql_fetchrowset($result); $this->db->sql_freeresult($result); - return $profile_fields; + + return array_column($rowset, 'field_ident', 'field_id'); } } diff --git a/tests/profilefields/manager_test.php b/tests/profilefields/manager_test.php index 76bc63124f..87078ab846 100644 --- a/tests/profilefields/manager_test.php +++ b/tests/profilefields/manager_test.php @@ -22,7 +22,7 @@ class manager_test extends phpbb_database_test_case /** @var \phpbb\profilefields\manager */ protected $manager; - /** @var \phpbb\log\log_interface */ + /** @var \phpbb\log\log */ protected $log; /** @var \phpbb\db\tools */ From 903185a69c430ce7651d716c83cd580df8edb6bc Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 23 Mar 2020 21:56:59 +0100 Subject: [PATCH 07/13] [ticket/13867] Amend profile fields manager tests PHPBB3-13867 --- tests/profilefields/manager_test.php | 116 +++++++++++++++++---------- 1 file changed, 72 insertions(+), 44 deletions(-) diff --git a/tests/profilefields/manager_test.php b/tests/profilefields/manager_test.php index 87078ab846..be2d6796ef 100644 --- a/tests/profilefields/manager_test.php +++ b/tests/profilefields/manager_test.php @@ -13,20 +13,23 @@ class manager_test extends phpbb_database_test_case { + /** @var \phpbb\config\db_text */ + protected $config_text; + /** @var \phpbb\db\driver\driver_interface */ protected $db; - /** @var \phpbb\config\db_text */ - protected $config_text; + /** @var \phpbb\db\tools\tools */ + protected $db_tools; + + /** @var \phpbb\log\log_interface */ + protected $log; /** @var \phpbb\profilefields\manager */ protected $manager; - /** @var \phpbb\log\log */ - protected $log; - - /** @var \phpbb\db\tools */ - protected $db_tools; + /** @var string Table prefix */ + protected $table_prefix; public function getDataSet() { @@ -37,37 +40,53 @@ class manager_test extends phpbb_database_test_case { parent::setUp(); - global $phpbb_log; + global $phpbb_root_path, $phpEx, $table_prefix; - $this->log = $this->prophesize('\phpbb\log\log_interface'); - $phpbb_log = $this->log->reveal(); + $factory = new \phpbb\db\tools\factory(); - $this->db = $this->new_dbal(); - $this->config_text = $this->prophesize('\phpbb\config\db_text'); - $this->db_tools = $this->prophesize('\phpbb\db\tools'); + $this->db = $this->new_dbal(); + $this->db_tools = $factory->get($this->db, true); + $this->config_text = new \phpbb\config\db_text($this->db, $table_prefix . 'config_text'); + $this->table_prefix = $table_prefix; + + $container = new phpbb_mock_container_builder(); + $dispatcher = new phpbb_mock_event_dispatcher(); + + $request = $this->getMock('\phpbb\request\request'); + $template = $this->getMock('\phpbb\template\template'); + + $auth = new \phpbb\auth\auth(); + $language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); + $collection = new \phpbb\di\service_collection($container); + $user = new \phpbb\user($language, '\phpbb\datetime'); + + $this->log = new \phpbb\log\log($this->db, $user, $auth, $dispatcher, $phpbb_root_path, 'adm/', $phpEx, $table_prefix . 'log'); $this->manager = new \phpbb\profilefields\manager( - $this->prophesize('phpbb\auth\auth')->reveal(), + $auth, + $this->config_text, $this->db, - $this->db_tools->reveal(), - $this->prophesize('\phpbb\event\dispatcher_interface')->reveal(), - $this->prophesize('\phpbb\request\request')->reveal(), - $this->prophesize('\phpbb\template\template')->reveal(), - $this->prophesize('\phpbb\di\service_collection')->reveal(), - $this->prophesize('\phpbb\user')->reveal(), - $this->config_text->reveal(), - PROFILE_FIELDS_TABLE, - PROFILE_FIELDS_LANG_TABLE, - PROFILE_FIELDS_DATA_TABLE + $this->db_tools, + $dispatcher, + $language, + $this->log, + $request, + $template, + $collection, + $user, + $table_prefix . 'profile_fields', + $table_prefix . 'profile_fields_data', + $table_prefix . 'profile_fields_lang', + $table_prefix . 'profile_lang' ); } public function test_disable_profilefields() { - $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_1')->shouldBeCalled(); - $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_2')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_1')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_2')->shouldBeCalled(); - $this->config_text->set('foo_bar_type.saved', json_encode([1 => 'pf_1', 2 => 'pf_2']))->shouldBeCalled(); + # $this->config_text->set('foo_bar_type.saved', json_encode([1 => 'pf_1', 2 => 'pf_2'])); $this->manager->disable_profilefields('foo_bar_type'); @@ -76,16 +95,16 @@ class manager_test extends phpbb_database_test_case WHERE field_active = 1 AND field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals(false, $sql, 'All profile fields should be disabled'); + $this->assertSqlResultEquals([], $sql, 'All profile fields should be disabled'); } public function test_enable_profilefields() { - $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_1')->shouldBeCalled(); - $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_2')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_1')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_2')->shouldBeCalled(); - $this->config_text->get('foo_bar_type.saved')->willReturn(json_encode([1 => 'pf_1', 2 => 'pf_2'])); - $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); + # $this->config_text->get('foo_bar_type.saved')->willReturn(json_encode([1 => 'pf_1', 2 => 'pf_2'])); + # $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); $this->manager->enable_profilefields('foo_bar_type'); @@ -102,33 +121,42 @@ class manager_test extends phpbb_database_test_case public function test_purge_profilefields() { - $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_1')->shouldBeCalled(); - $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_2')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_1')->shouldBeCalled(); + # $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_2')->shouldBeCalled(); - $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); + # $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); - $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_1')->shouldBeCalled(); - $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_2')->shouldBeCalled(); + # $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_1')->shouldBeCalled(); + # $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_2')->shouldBeCalled(); - $this->manager->enable_profilefields('foo_bar_type'); + $sql = 'SELECT field_id + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_type = 'foo_bar_type'"; + $result = $this->db->sql_query($sql); + $rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + + $field_ids = array_map('intval', array_column($rowset, 'field_id')); + + $this->manager->purge_profilefields('foo_bar_type'); $sql = 'SELECT field_id FROM ' . PROFILE_FIELDS_TABLE . " WHERE field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals(false, $sql, 'All profile fields should be removed'); + $this->assertSqlResultEquals([], $sql, 'All profile fields should be removed'); $sql = 'SELECT field_id FROM ' . PROFILE_FIELDS_LANG_TABLE . " WHERE field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals(false, $sql, 'All profile fields lang should be removed'); + $this->assertSqlResultEquals([], $sql, 'All profile fields lang should be removed'); - $sql = 'SELECT field_id - FROM ' . PROFILE_LANG_TABLE . " - WHERE field_type = 'foo_bar_type'"; + $sql = 'SELECT lang_name + FROM ' . PROFILE_LANG_TABLE . ' + WHERE ' . $this->db->sql_in_set('field_id', $field_ids); - $this->assertSqlResultEquals(false, $sql, 'All profile fields lang should be removed'); + $this->assertSqlResultEquals([], $sql, 'All profile fields lang should be removed'); $sql = 'SELECT field_id, field_order FROM ' . PROFILE_FIELDS_TABLE; From 29d44973a7ffb481b0bc8d41eb9618e7c10af357 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 23 Mar 2020 22:27:16 +0100 Subject: [PATCH 08/13] [ticket/13867] Finish profile fields manager tests PHPBB3-13867 --- tests/profilefields/manager_test.php | 79 ++++++++++++++++------------ 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/tests/profilefields/manager_test.php b/tests/profilefields/manager_test.php index be2d6796ef..993a98b54e 100644 --- a/tests/profilefields/manager_test.php +++ b/tests/profilefields/manager_test.php @@ -31,6 +31,9 @@ class manager_test extends phpbb_database_test_case /** @var string Table prefix */ protected $table_prefix; + /** + * {@inheritdoc} + */ public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/manager.xml'); @@ -42,10 +45,8 @@ class manager_test extends phpbb_database_test_case global $phpbb_root_path, $phpEx, $table_prefix; - $factory = new \phpbb\db\tools\factory(); - $this->db = $this->new_dbal(); - $this->db_tools = $factory->get($this->db, true); + $this->db_tools = $this->getMock('\phpbb\db\tools\tools', [], [$this->db]); $this->config_text = new \phpbb\config\db_text($this->db, $table_prefix . 'config_text'); $this->table_prefix = $table_prefix; @@ -83,54 +84,56 @@ class manager_test extends phpbb_database_test_case public function test_disable_profilefields() { - # $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_1')->shouldBeCalled(); - # $this->log->add('admin', 'LOG_PROFILE_FIELD_DEACTIVATE', 'pf_2')->shouldBeCalled(); - - # $this->config_text->set('foo_bar_type.saved', json_encode([1 => 'pf_1', 2 => 'pf_2'])); - + // Disable the profile field type $this->manager->disable_profilefields('foo_bar_type'); $sql = 'SELECT field_id, field_ident - FROM ' . PROFILE_FIELDS_TABLE . " + FROM ' . $this->table_prefix . "profile_fields WHERE field_active = 1 AND field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals([], $sql, 'All profile fields should be disabled'); + + // Test that the config entry exists + $saved = $this->config_text->get('foo_bar_type.saved'); + $saved = (array) json_decode($saved, true); + $this->assertEquals([ + 1 => 'pf_1', + 2 => 'pf_2', + ], $saved, 'All disable profile fields should be saved'); } public function test_enable_profilefields() { - # $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_1')->shouldBeCalled(); - # $this->log->add('admin', 'LOG_PROFILE_FIELD_ACTIVATE', 'pf_2')->shouldBeCalled(); - - # $this->config_text->get('foo_bar_type.saved')->willReturn(json_encode([1 => 'pf_1', 2 => 'pf_2'])); - # $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); - + // Enable the profile field type $this->manager->enable_profilefields('foo_bar_type'); $sql = 'SELECT field_id - FROM ' . PROFILE_FIELDS_TABLE . " + FROM ' . $this->table_prefix . "profile_fields WHERE field_active = 1 AND field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals([ ['field_id' => '1'], ['field_id' => '2'], ], $sql, 'All profile fields should be enabled'); + + // Test that the config entry was removed + $saved = $this->config_text->get('foo_bar_type.saved'); + $this->assertEquals($saved, null, 'All disable profile fields should be removed'); } public function test_purge_profilefields() { - # $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_1')->shouldBeCalled(); - # $this->log->add('admin', 'LOG_PROFILE_FIELD_REMOVED', 'pf_2')->shouldBeCalled(); - - # $this->config_text->delete('foo_bar_type.saved')->shouldBeCalled(); - - # $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_1')->shouldBeCalled(); - # $this->db_tools->sql_column_remove(PROFILE_FIELDS_DATA_TABLE, 'pf_pf_2')->shouldBeCalled(); + $this->db_tools + ->expects($this->exactly(2)) + ->method('sql_column_remove') + ->with( + $this->table_prefix . 'profile_fields_data', + $this->stringStartsWith('pf_') + ); + // Get the field identifiers $sql = 'SELECT field_id - FROM ' . PROFILE_FIELDS_TABLE . " + FROM ' . $this->table_prefix . "profile_fields WHERE field_type = 'foo_bar_type'"; $result = $this->db->sql_query($sql); $rowset = $this->db->sql_fetchrowset($result); @@ -138,28 +141,36 @@ class manager_test extends phpbb_database_test_case $field_ids = array_map('intval', array_column($rowset, 'field_id')); + // Purge the profile field type $this->manager->purge_profilefields('foo_bar_type'); + // Test all the profile field tables $sql = 'SELECT field_id - FROM ' . PROFILE_FIELDS_TABLE . " + FROM ' . $this->table_prefix . "profile_fields WHERE field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals([], $sql, 'All profile fields should be removed'); $sql = 'SELECT field_id - FROM ' . PROFILE_FIELDS_LANG_TABLE . " + FROM ' . $this->table_prefix . "profile_fields_lang WHERE field_type = 'foo_bar_type'"; - $this->assertSqlResultEquals([], $sql, 'All profile fields lang should be removed'); $sql = 'SELECT lang_name - FROM ' . PROFILE_LANG_TABLE . ' + FROM ' . $this->table_prefix . 'profile_lang WHERE ' . $this->db->sql_in_set('field_id', $field_ids); - $this->assertSqlResultEquals([], $sql, 'All profile fields lang should be removed'); - $sql = 'SELECT field_id, field_order FROM ' . PROFILE_FIELDS_TABLE; + $sql = 'SELECT field_id, field_order + FROM ' . $this->table_prefix . 'profile_fields'; + $this->assertSqlResultEquals([ + [ + 'field_id' => '3', + 'field_order' => '1' + ] + ], $sql, 'Profile fields order should be recalculated, starting by 1'); - $this->assertSqlResultEquals([['field_id' => '3', 'field_order' => '1'],], $sql, 'Profile fields order should be recalculated, starting by 1'); + // Test that the config entry was removed + $saved = $this->config_text->get('foo_bar_type.saved'); + $this->assertEquals($saved, null, 'All disable profile fields should be removed'); } } From b602e0c0140a63e5967f8ccc53b7ee8f4689984d Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 23 Mar 2020 22:29:00 +0100 Subject: [PATCH 09/13] [ticket/13867] Remove profile field test prophecy PHPBB3-13867 --- tests/test_framework/phpbb_test_case.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_framework/phpbb_test_case.php b/tests/test_framework/phpbb_test_case.php index 0bf464567f..01d26fb67d 100644 --- a/tests/test_framework/phpbb_test_case.php +++ b/tests/test_framework/phpbb_test_case.php @@ -27,7 +27,6 @@ class phpbb_test_case extends PHPUnit_Framework_TestCase 'PHP_Token_Stream_CachingFactory' => array('cache'), 'phpbb_database_test_case' => array('already_connected', 'last_post_timestamp'), - 'Prophecy\Doubler\NameGenerator' => array('counter'), ); } From d26e0423f7f457fceae807d320b2f351e4f5b610 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 23 Mar 2020 22:47:18 +0100 Subject: [PATCH 10/13] [ticket/13867] Amend profile fields manager constructor PHPBB3-13867 --- phpBB/phpbb/profilefields/manager.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index 2e26f62355..e89956581e 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -99,8 +99,8 @@ class manager \phpbb\user $user, $fields_table, $fields_data_table, - $fields_lang_table, - $fields_data_lang_table + $fields_data_lang_table, + $fields_lang_table ) { $this->auth = $auth; @@ -116,9 +116,9 @@ class manager $this->user = $user; $this->fields_table = $fields_table; - $this->fields_lang_table = $fields_lang_table; $this->fields_data_table = $fields_data_table; $this->fields_data_lang_table = $fields_data_lang_table; + $this->fields_lang_table = $fields_lang_table; } /** From 9080a0e778616bdd7e1102eb5b81989f2f318341 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Tue, 24 Mar 2020 00:28:00 +0100 Subject: [PATCH 11/13] [ticket/13867] Force data type in profile fields manager PHPBB3-13867 --- phpBB/phpbb/profilefields/manager.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index e89956581e..cfa837cd91 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -364,9 +364,11 @@ class manager return []; } + $user_ids = (array) $user_ids; + $sql = 'SELECT * FROM ' . $this->fields_data_table . ' - WHERE ' . $this->db->sql_in_set('user_id', array_map('intval', (array) $user_ids)); + WHERE ' . $this->db->sql_in_set('user_id', array_map('intval', $user_ids)); $result = $this->db->sql_query($sql); $rowset = $this->db->sql_fetchrowset($result); $this->db->sql_freeresult($result); From 7149349e2582a069cbfe70650e5a60533df49274 Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Tue, 24 Mar 2020 01:06:02 +0100 Subject: [PATCH 12/13] [ticket/13867] Use order by in profile fields manager test PHPBB3-13867 --- tests/profilefields/manager_test.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/profilefields/manager_test.php b/tests/profilefields/manager_test.php index 993a98b54e..5431c97125 100644 --- a/tests/profilefields/manager_test.php +++ b/tests/profilefields/manager_test.php @@ -110,7 +110,8 @@ class manager_test extends phpbb_database_test_case $sql = 'SELECT field_id FROM ' . $this->table_prefix . "profile_fields WHERE field_active = 1 - AND field_type = 'foo_bar_type'"; + AND field_type = 'foo_bar_type' + ORDER BY field_id ASC"; $this->assertSqlResultEquals([ ['field_id' => '1'], ['field_id' => '2'], @@ -161,7 +162,8 @@ class manager_test extends phpbb_database_test_case $this->assertSqlResultEquals([], $sql, 'All profile fields lang should be removed'); $sql = 'SELECT field_id, field_order - FROM ' . $this->table_prefix . 'profile_fields'; + FROM ' . $this->table_prefix . 'profile_fields + ORDER BY field_id ASC'; $this->assertSqlResultEquals([ [ 'field_id' => '3', From c8e92608fdaecd673a865bcce1888cfef96eed4f Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 4 May 2020 22:05:07 +0200 Subject: [PATCH 13/13] [ticket/13867] Correct constructor comment for db tools PHPBB3-13867 --- phpBB/phpbb/profilefields/manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/profilefields/manager.php b/phpBB/phpbb/profilefields/manager.php index cfa837cd91..5daa61076c 100644 --- a/phpBB/phpbb/profilefields/manager.php +++ b/phpBB/phpbb/profilefields/manager.php @@ -72,7 +72,7 @@ class manager * @param \phpbb\auth\auth $auth Auth object * @param \phpbb\config\db_text $config_text Config_text object * @param \phpbb\db\driver\driver_interface $db Database object - * @param \phpbb\db\tools\tools $db_tools Database object + * @param \phpbb\db\tools\tools $db_tools Database tools object * @param \phpbb\event\dispatcher_interface $dispatcher Event dispatcher object * @param \phpbb\language\language $language Language object * @param \phpbb\log\log $log Log object