From 3c632fa8e4af4b9b666ddbab2de77d42716ce2b2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 24 Oct 2013 13:14:51 +0200 Subject: [PATCH 1/8] [ticket/11842] Use group_id 0 and correct avatar name after creating group It seems like the function group_correct_avatar() was forgotten while adding the new avatar system. We now pass the group_id 0 to the upload avatar and let the function group_correct_avatar() fix the avatar filename after creating the group like it was done previously. PHPBB3-11842 --- phpBB/includes/acp/acp_groups.php | 4 ++++ phpBB/includes/functions_user.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index ad29a5521b..e0f2e0ed8d 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -325,6 +325,10 @@ class acp_groups // This is normalised data, without the group_ prefix $avatar_data = \phpbb\avatar\manager::clean_row($group_row); + if (!isset($avatar_data['id'])) + { + $avatar_data['id'] = $group_id; + } } diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 30891c3fb5..475f59da1d 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -2314,7 +2314,7 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow { $group_id = $db->sql_nextid(); - if (isset($sql_ary['group_avatar_type']) && $sql_ary['group_avatar_type'] == AVATAR_UPLOAD) + if (isset($sql_ary['group_avatar_type']) && $sql_ary['group_avatar_type'] == 'avatar.driver.upload') { group_correct_avatar($group_id, $sql_ary['group_avatar']); } From 1f624e136785893d0c9e4bfeb1a12f9c08693a24 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 24 Oct 2013 21:41:41 +0200 Subject: [PATCH 2/8] [ticket/11842] Replace outdated occurences of user and group avatar_type user_avatar_type und group_avatar_type are now a string and should therefore be treated accordingly. PHPBB3-11842 --- phpBB/includes/functions_user.php | 10 +++++----- tests/functions_user/group_user_attributes_test.php | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 475f59da1d..9e84af5c28 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -213,7 +213,7 @@ function user_add($user_row, $cp_data = false) 'user_occ' => '', 'user_interests' => '', 'user_avatar' => '', - 'user_avatar_type' => 0, + 'user_avatar_type' => '', 'user_avatar_width' => 0, 'user_avatar_height' => 0, 'user_new_privmsg' => 0, @@ -463,7 +463,7 @@ function user_delete($mode, $user_ids, $retain_username = true) $added_guest_posts = 0; foreach ($user_rows as $user_id => $user_row) { - if ($user_row['user_avatar'] && $user_row['user_avatar_type'] == AVATAR_UPLOAD) + if ($user_row['user_avatar'] && ($user_row['user_avatar_type'] == AVATAR_UPLOAD || $user_row['user_avatar_type'] == 'avatar.driver.upload')) { avatar_delete('user', $user_row); } @@ -2415,7 +2415,7 @@ function avatar_remove_db($avatar_name) $sql = 'UPDATE ' . USERS_TABLE . " SET user_avatar = '', - user_avatar_type = 0 + user_avatar_type = '' WHERE user_avatar = '" . $db->sql_escape($avatar_name) . '\''; $db->sql_query($sql); } @@ -2825,7 +2825,7 @@ function remove_default_avatar($group_id, $user_ids) $sql = 'UPDATE ' . USERS_TABLE . " SET user_avatar = '', - user_avatar_type = 0, + user_avatar_type = '', user_avatar_width = 0, user_avatar_height = 0 WHERE group_id = " . (int) $group_id . " @@ -3083,7 +3083,7 @@ function group_set_user_default($group_id, $user_id_ary, $group_attributes = fal 'group_colour' => 'string', 'group_rank' => 'int', 'group_avatar' => 'string', - 'group_avatar_type' => 'int', + 'group_avatar_type' => 'string', 'group_avatar_width' => 'int', 'group_avatar_height' => 'int', ); diff --git a/tests/functions_user/group_user_attributes_test.php b/tests/functions_user/group_user_attributes_test.php index f8d52a9a6a..5ed60f6593 100644 --- a/tests/functions_user/group_user_attributes_test.php +++ b/tests/functions_user/group_user_attributes_test.php @@ -27,7 +27,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 2, array( 'group_avatar' => '', - 'group_avatar_type' => 0, + 'group_avatar_type' => '', 'group_avatar_height' => 0, 'group_avatar_width' => 0, 'group_rank' => 0, @@ -43,7 +43,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 2, array( 'group_avatar' => '', - 'group_avatar_type' => 0, + 'group_avatar_type' => '', 'group_avatar_height' => 0, 'group_avatar_width' => 0, 'group_rank' => 0, @@ -59,7 +59,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 2, array( 'group_avatar' => '', - 'group_avatar_type' => 0, + 'group_avatar_type' => '', 'group_avatar_height' => 0, 'group_avatar_width' => 0, 'group_rank' => 0, @@ -75,7 +75,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 3, array( 'group_avatar' => 'default2', - 'group_avatar_type' => 1, + 'group_avatar_type' => 'avatar.driver.upload', 'group_avatar_height' => 1, 'group_avatar_width' => 1, 'group_rank' => 3, @@ -91,7 +91,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 3, array( 'group_avatar' => 'default2', - 'group_avatar_type' => 1, + 'group_avatar_type' => 'avatar.driver.upload', 'group_avatar_height' => 1, 'group_avatar_width' => 1, 'group_rank' => 3, @@ -107,7 +107,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes 3, array( 'group_avatar' => 'default2', - 'group_avatar_type' => 1, + 'group_avatar_type' => 'avatar.driver.upload', 'group_avatar_height' => 1, 'group_avatar_width' => 1, 'group_rank' => 3, From 6db967bdd51c7df79ec7095e6d5dc506fbf3f132 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 20 Nov 2013 17:07:53 +0100 Subject: [PATCH 3/8] [ticket/11842] Add migration file for updating avatar type in database PHPBB3-11842 --- .../db/migration/data/v310/avatar_types.php | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 phpBB/phpbb/db/migration/data/v310/avatar_types.php diff --git a/phpBB/phpbb/db/migration/data/v310/avatar_types.php b/phpBB/phpbb/db/migration/data/v310/avatar_types.php new file mode 100644 index 0000000000..439e20889c --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v310/avatar_types.php @@ -0,0 +1,65 @@ +table_prefix . "users + SET user_avatar_type = 'avatar.driver.upload' + WHERE user_avatar_type = " . AVATAR_UPLOAD; + $this->db->sql_query($sql); + + $sql = 'UPDATE ' . $this->table_prefix . "users + SET user_avatar_type = 'avatar.driver.remote' + WHERE user_avatar_type = " . AVATAR_REMOTE; + $this->db->sql_query($sql); + + $sql = 'UPDATE ' . $this->table_prefix . "users + SET user_avatar_type = 'avatar.driver.local' + WHERE user_avatar_type = " . AVATAR_GALLERY; + $this->db->sql_query($sql); + } + + public function update_group_avatar_type() + { + $sql = 'UPDATE ' . $this->table_prefix . "groups + SET group_avatar_type = 'avatar.driver.upload' + WHERE group_avatar_type = " . AVATAR_UPLOAD; + $this->db->sql_query($sql); + + $sql = 'UPDATE ' . $this->table_prefix . "groups + SET group_avatar_type = 'avatar.driver.remote' + WHERE group_avatar_type = " . AVATAR_REMOTE; + $this->db->sql_query($sql); + + $sql = 'UPDATE ' . $this->table_prefix . "groups + SET group_avatar_type = 'avatar.driver.local' + WHERE group_avatar_type = " . AVATAR_GALLERY; + $this->db->sql_query($sql); + } +} From 3caeb09b18744a74ca2b46c670a9b943a3905c74 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 20 Nov 2013 17:15:32 +0100 Subject: [PATCH 4/8] [ticket/11842] Use only new avatar type in user_delete function PHPBB3-11842 --- phpBB/includes/functions_user.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 9e84af5c28..bdc7104e5f 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -463,7 +463,7 @@ function user_delete($mode, $user_ids, $retain_username = true) $added_guest_posts = 0; foreach ($user_rows as $user_id => $user_row) { - if ($user_row['user_avatar'] && ($user_row['user_avatar_type'] == AVATAR_UPLOAD || $user_row['user_avatar_type'] == 'avatar.driver.upload')) + if ($user_row['user_avatar'] && $user_row['user_avatar_type'] == 'avatar.driver.upload') { avatar_delete('user', $user_row); } From b5e6d107aea96aa3338584b5f8027eade9b68f99 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 23 Nov 2013 01:05:39 +0100 Subject: [PATCH 5/8] [ticket/11842] Add missing prefix for group id in avatar data PHPBB3-11842 --- phpBB/includes/acp/acp_groups.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index e0f2e0ed8d..449c905bfa 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -327,7 +327,7 @@ class acp_groups $avatar_data = \phpbb\avatar\manager::clean_row($group_row); if (!isset($avatar_data['id'])) { - $avatar_data['id'] = $group_id; + $avatar_data['id'] = 'g' . $group_id; } } From 0d4bf3ff45a76dcb763c76502944aa7bf78b690b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 25 Nov 2013 13:12:08 +0100 Subject: [PATCH 6/8] [ticket/11842] Use type map for updating avatar types in database PHPBB3-11842 --- .../db/migration/data/v310/avatar_types.php | 51 +++++++++---------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v310/avatar_types.php b/phpBB/phpbb/db/migration/data/v310/avatar_types.php index 439e20889c..5750a43ddd 100644 --- a/phpBB/phpbb/db/migration/data/v310/avatar_types.php +++ b/phpBB/phpbb/db/migration/data/v310/avatar_types.php @@ -11,6 +11,15 @@ namespace phpbb\db\migration\data\v310; class avatar_types extends \phpbb\db\migration\migration { + /** + * @var avatar type map + */ + protected $avatar_type_map = array( + AVATAR_UPLOAD => 'avatar.driver.upload', + AVATAR_REMOTE => 'avatar.driver.remote', + AVATAR_GALLERY => 'avatar.driver.local', + ); + static public function depends_on() { return array( @@ -29,37 +38,23 @@ class avatar_types extends \phpbb\db\migration\migration public function update_user_avatar_type() { - $sql = 'UPDATE ' . $this->table_prefix . "users - SET user_avatar_type = 'avatar.driver.upload' - WHERE user_avatar_type = " . AVATAR_UPLOAD; - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . $this->table_prefix . "users - SET user_avatar_type = 'avatar.driver.remote' - WHERE user_avatar_type = " . AVATAR_REMOTE; - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . $this->table_prefix . "users - SET user_avatar_type = 'avatar.driver.local' - WHERE user_avatar_type = " . AVATAR_GALLERY; - $this->db->sql_query($sql); + foreach ($this->avatar_type_map as $old => $new) + { + $sql = 'UPDATE ' . $this->table_prefix . "users + SET user_avatar_type = '$new' + WHERE user_avatar_type = $old"; + $this->db->sql_query($sql); + } } public function update_group_avatar_type() { - $sql = 'UPDATE ' . $this->table_prefix . "groups - SET group_avatar_type = 'avatar.driver.upload' - WHERE group_avatar_type = " . AVATAR_UPLOAD; - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . $this->table_prefix . "groups - SET group_avatar_type = 'avatar.driver.remote' - WHERE group_avatar_type = " . AVATAR_REMOTE; - $this->db->sql_query($sql); - - $sql = 'UPDATE ' . $this->table_prefix . "groups - SET group_avatar_type = 'avatar.driver.local' - WHERE group_avatar_type = " . AVATAR_GALLERY; - $this->db->sql_query($sql); + foreach ($this->avatar_type_map as $old => $new) + { + $sql = 'UPDATE ' . $this->table_prefix . "groups + SET group_avatar_type = '$new' + WHERE group_avatar_type = $old"; + $this->db->sql_query($sql); + } } } From abb2def48d7a946fd4d0a67f88682c9fa2556223 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 4 Dec 2013 15:42:17 +0100 Subject: [PATCH 7/8] [ticket/11842] Use avatar_data for obtaining driver that should be deleted PHPBB3-11842 --- phpBB/includes/acp/acp_groups.php | 2 +- phpBB/includes/acp/acp_users.php | 2 +- phpBB/includes/ucp/ucp_groups.php | 2 +- phpBB/includes/ucp/ucp_profile.php | 2 +- tests/functional/avatar_acp_groups_test.php | 9 +++++++++ 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index d36c9cda09..b36ea1a8d8 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -383,7 +383,7 @@ class acp_groups } else { - $driver = $phpbb_avatar_manager->get_driver($user->data['user_avatar_type']); + $driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type']); if ($driver) { $driver->delete($avatar_data); diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index fbc1cc1f14..1a7bc2d186 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -1775,7 +1775,7 @@ class acp_users } else { - $driver = $phpbb_avatar_manager->get_driver($user->data['user_avatar_type']); + $driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type']); if ($driver) { $driver->delete($avatar_data); diff --git a/phpBB/includes/ucp/ucp_groups.php b/phpBB/includes/ucp/ucp_groups.php index 716289eded..7c4bc8f617 100644 --- a/phpBB/includes/ucp/ucp_groups.php +++ b/phpBB/includes/ucp/ucp_groups.php @@ -509,7 +509,7 @@ class ucp_groups } else { - if ($driver = $phpbb_avatar_manager->get_driver($user->data['user_avatar_type'])) + if ($driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type'])) { $driver->delete($avatar_data); } diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index f7c6aca9e8..2252b2ea17 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -603,7 +603,7 @@ class ucp_profile } else { - if ($driver = $phpbb_avatar_manager->get_driver($user->data['user_avatar_type'])) + if ($driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type'])) { $driver->delete($avatar_data); } diff --git a/tests/functional/avatar_acp_groups_test.php b/tests/functional/avatar_acp_groups_test.php index 5e908bc6da..5f767b44f2 100644 --- a/tests/functional/avatar_acp_groups_test.php +++ b/tests/functional/avatar_acp_groups_test.php @@ -69,4 +69,13 @@ class phpbb_functional_avatar_acp_groups_test extends phpbb_functional_common_av { $this->assert_avatar_submit($expected, $avatar_type, $data); } + + // Test if avatar was really deleted + public function test_no_avatar_acp_groups() + { + $crawler = self::request('GET', $this->get_url() . '&sid=' . $this->sid); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $form_data = $form->getValues(); + $this->assertEmpty($form_data['avatar_type']); + } } From 80fa658e8fc94972349b5f0d01f93afba7744e0e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 4 Dec 2013 15:58:07 +0100 Subject: [PATCH 8/8] [ticket/11842] Add functional test for creating group PHPBB3-11842 --- tests/functional/group_create_test.php | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/functional/group_create_test.php diff --git a/tests/functional/group_create_test.php b/tests/functional/group_create_test.php new file mode 100644 index 0000000000..96780069f7 --- /dev/null +++ b/tests/functional/group_create_test.php @@ -0,0 +1,31 @@ +login(); + $this->admin_login(); + $this->add_lang('acp/groups'); + + $crawler = self::request('GET', 'adm/index.php?i=acp_groups&mode=manage&sid=' . $this->sid); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $crawler = self::submit($form, array('group_name' => 'testtest')); + + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); + $crawler = self::submit($form, array('group_name' => 'testtest')); + + $this->assertContainsLang('GROUP_CREATED', $crawler->filter('#main')->text()); + } +}