aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarc Alexander <admin@m-a-styles.de>2014-08-07 17:19:08 +0200
committerMarc Alexander <admin@m-a-styles.de>2014-08-07 17:29:33 +0200
commite0c2013bb921f04f032744c3bc351c015c759d8d (patch)
treedc9f79569e572646a0ec49e3e81119802701119e
parent4f23bb711ca457efba370d7ac67394e0db59b578 (diff)
downloadforums-e0c2013bb921f04f032744c3bc351c015c759d8d.tar
forums-e0c2013bb921f04f032744c3bc351c015c759d8d.tar.gz
forums-e0c2013bb921f04f032744c3bc351c015c759d8d.tar.bz2
forums-e0c2013bb921f04f032744c3bc351c015c759d8d.tar.xz
forums-e0c2013bb921f04f032744c3bc351c015c759d8d.zip
[ticket/12822] Add method for deleting avatars and use confirm_box
The method will take care of the actual deletion process in the avatar manager. Additionally, we'll be using a confirm box while deleting the avatar to prevent users from accidentally deleting their avatars. PHPBB3-12822
-rw-r--r--phpBB/includes/acp/acp_groups.php22
-rw-r--r--phpBB/includes/acp/acp_users.php58
-rw-r--r--phpBB/includes/ucp/ucp_groups.php36
-rw-r--r--phpBB/includes/ucp/ucp_profile.php45
-rw-r--r--phpBB/language/en/common.php1
-rw-r--r--phpBB/phpbb/avatar/driver/remote.php5
-rw-r--r--phpBB/phpbb/avatar/manager.php36
-rw-r--r--tests/avatar/fixtures/users.xml33
-rw-r--r--tests/avatar/manager_test.php57
-rw-r--r--tests/functional/avatar_acp_groups_test.php2
-rw-r--r--tests/functional/avatar_acp_users_test.php2
-rw-r--r--tests/functional/avatar_ucp_groups_test.php2
-rw-r--r--tests/functional/avatar_ucp_users_test.php3
-rw-r--r--tests/functional/common_avatar_test.php15
14 files changed, 224 insertions, 93 deletions
diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php
index f10f0b1015..edfada1bf1 100644
--- a/phpBB/includes/acp/acp_groups.php
+++ b/phpBB/includes/acp/acp_groups.php
@@ -331,6 +331,28 @@ class acp_groups
}
}
+ if ($request->is_set_post('avatar_delete'))
+ {
+ if (confirm_box(true))
+ {
+ $avatar_data['id'] = substr($avatar_data['id'], 1);
+ $phpbb_avatar_manager->handle_avatar_delete($db, $user, $avatar_data, GROUPS_TABLE, 'group_');
+
+ $message = ($action == 'edit') ? 'GROUP_UPDATED' : 'GROUP_CREATED';
+ trigger_error($user->lang[$message] . adm_back_link($this->u_action));
+ }
+ else
+ {
+ confirm_box(false, $user->lang('CONFIRM_AVATAR_DELETE'), build_hidden_fields(array(
+ 'avatar_delete' => true,
+ 'i' => $id,
+ 'mode' => $mode,
+ 'g' => $group_id,
+ 'action' => $action))
+ );
+ }
+ }
+
// Did we submit?
if ($update)
{
diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php
index 71880c2267..58f9643dc9 100644
--- a/phpBB/includes/acp/acp_users.php
+++ b/phpBB/includes/acp/acp_users.php
@@ -465,25 +465,9 @@ class acp_users
trigger_error($user->lang['FORM_INVALID'] . adm_back_link($this->u_action . '&amp;u=' . $user_id), E_USER_WARNING);
}
- $sql_ary = array(
- 'user_avatar' => '',
- 'user_avatar_type' => '',
- 'user_avatar_width' => 0,
- 'user_avatar_height' => 0,
- );
-
- $sql = 'UPDATE ' . USERS_TABLE . '
- SET ' . $db->sql_build_array('UPDATE', $sql_ary) . "
- WHERE user_id = $user_id";
- $db->sql_query($sql);
-
// Delete old avatar if present
$phpbb_avatar_manager = $phpbb_container->get('avatar.manager');
- $driver = $phpbb_avatar_manager->get_driver($user_row['user_avatar_type']);
- if ($driver)
- {
- $driver->delete($user_row);
- }
+ $phpbb_avatar_manager->handle_avatar_delete($db, $user, $phpbb_avatar_manager->clean_row($user_row, 'user'), USERS_TABLE, 'user_');
add_log('admin', 'LOG_USER_DEL_AVATAR', $user_row['username']);
add_log('user', $user_id, 'LOG_USER_DEL_AVATAR_USER');
@@ -1779,29 +1763,6 @@ class acp_users
trigger_error($user->lang['USER_AVATAR_UPDATED'] . adm_back_link($this->u_action . '&amp;u=' . $user_id));
}
}
- else
- {
- $driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type']);
- if ($driver)
- {
- $driver->delete($avatar_data);
- }
-
- // Removing the avatar
- $result = array(
- 'user_avatar' => '',
- 'user_avatar_type' => '',
- 'user_avatar_width' => 0,
- 'user_avatar_height' => 0,
- );
-
- $sql = 'UPDATE ' . USERS_TABLE . '
- SET ' . $db->sql_build_array('UPDATE', $result) . '
- WHERE user_id = ' . (int) $user_id;
-
- $db->sql_query($sql);
- trigger_error($user->lang['USER_AVATAR_UPDATED'] . adm_back_link($this->u_action . '&amp;u=' . $user_id));
- }
}
else
{
@@ -1809,6 +1770,23 @@ class acp_users
}
}
+ // Handle deletion of avatars
+ if ($request->is_set_post('avatar_delete'))
+ {
+ if (!confirm_box(true))
+ {
+ confirm_box(false, $user->lang('CONFIRM_AVATAR_DELETE'), build_hidden_fields(array(
+ 'avatar_delete' => true))
+ );
+ }
+ else
+ {
+ $phpbb_avatar_manager->handle_avatar_delete($db, $user, $avatar_data, USERS_TABLE, 'user_');
+
+ trigger_error($user->lang['USER_AVATAR_UPDATED'] . adm_back_link($this->u_action . '&amp;u=' . $user_id));
+ }
+ }
+
$selected_driver = $phpbb_avatar_manager->clean_driver_name($request->variable('avatar_driver', $user_row['user_avatar_type']));
foreach ($avatar_drivers as $current_driver)
diff --git a/phpBB/includes/ucp/ucp_groups.php b/phpBB/includes/ucp/ucp_groups.php
index e4931fbe23..b9606945b4 100644
--- a/phpBB/includes/ucp/ucp_groups.php
+++ b/phpBB/includes/ucp/ucp_groups.php
@@ -471,6 +471,29 @@ class ucp_groups
$avatar_data = \phpbb\avatar\manager::clean_row($group_row, 'group');
}
+ // Handle deletion of avatars
+ if ($request->is_set_post('avatar_delete'))
+ {
+ if (confirm_box(true))
+ {
+ $phpbb_avatar_manager->handle_avatar_delete($db, $user, $avatar_data, GROUPS_TABLE, 'group_');
+ $cache->destroy('sql', GROUPS_TABLE);
+
+ $message = ($action == 'edit') ? 'GROUP_UPDATED' : 'GROUP_CREATED';
+ trigger_error($user->lang[$message] . $return_page);
+ }
+ else
+ {
+ confirm_box(false, $user->lang('CONFIRM_AVATAR_DELETE'), build_hidden_fields(array(
+ 'avatar_delete' => true,
+ 'i' => $id,
+ 'mode' => $mode,
+ 'g' => $group_id,
+ 'action' => $action))
+ );
+ }
+ }
+
// Did we submit?
if ($update)
{
@@ -510,19 +533,6 @@ class ucp_groups
$submit_ary = array_merge($submit_ary, $result);
}
}
- else
- {
- if ($driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type']))
- {
- $driver->delete($avatar_data);
- }
-
- // Removing the avatar
- $submit_ary['avatar_type'] = '';
- $submit_ary['avatar'] = '';
- $submit_ary['avatar_width'] = 0;
- $submit_ary['avatar_height'] = 0;
- }
// Merge any avatars errors into the primary error array
$error = array_merge($error, $phpbb_avatar_manager->localize_errors($user, $avatar_error));
diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php
index a315b167d7..361dc831aa 100644
--- a/phpBB/includes/ucp/ucp_profile.php
+++ b/phpBB/includes/ucp/ucp_profile.php
@@ -564,30 +564,6 @@ class ucp_profile
trigger_error($message);
}
}
- else
- {
- if ($driver = $phpbb_avatar_manager->get_driver($avatar_data['avatar_type']))
- {
- $driver->delete($avatar_data);
- }
-
- $result = array(
- 'user_avatar' => '',
- 'user_avatar_type' => '',
- 'user_avatar_width' => 0,
- 'user_avatar_height' => 0,
- );
-
- $sql = 'UPDATE ' . USERS_TABLE . '
- SET ' . $db->sql_build_array('UPDATE', $result) . '
- WHERE user_id = ' . (int) $user->data['user_id'];
-
- $db->sql_query($sql);
-
- meta_refresh(3, $this->u_action);
- $message = $user->lang['PROFILE_UPDATED'] . '<br /><br />' . sprintf($user->lang['RETURN_UCP'], '<a href="' . $this->u_action . '">', '</a>');
- trigger_error($message);
- }
}
else
{
@@ -595,6 +571,27 @@ class ucp_profile
}
}
+ // Handle deletion of avatars
+ if ($request->is_set_post('avatar_delete'))
+ {
+ if (!confirm_box(true))
+ {
+ confirm_box(false, $user->lang('CONFIRM_AVATAR_DELETE'), build_hidden_fields(array(
+ 'avatar_delete' => true,
+ 'i' => $id,
+ 'mode' => $mode))
+ );
+ }
+ else
+ {
+ $phpbb_avatar_manager->handle_avatar_delete($db, $user, $avatar_data, USERS_TABLE, 'user_');
+
+ meta_refresh(3, $this->u_action);
+ $message = $user->lang['PROFILE_UPDATED'] . '<br /><br />' . sprintf($user->lang['RETURN_UCP'], '<a href="' . $this->u_action . '">', '</a>');
+ trigger_error($message);
+ }
+ }
+
$selected_driver = $phpbb_avatar_manager->clean_driver_name($request->variable('avatar_driver', $user->data['user_avatar_type']));
foreach ($avatar_drivers as $current_driver)
diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php
index 13fc56e0a1..f84ace9f51 100644
--- a/phpBB/language/en/common.php
+++ b/phpBB/language/en/common.php
@@ -167,6 +167,7 @@ $lang = array_merge($lang, array(
'CONFIRM_CODE_EXPLAIN' => 'Enter the code exactly as it appears. All letters are case insensitive.',
'CONFIRM_CODE_WRONG' => 'The confirmation code you entered was incorrect.',
'CONFIRM_OPERATION' => 'Are you sure you wish to carry out this operation?',
+ 'CONFIRM_AVATAR_DELETE' => 'Are you sure you wish to delete this avatar?',
'CONGRATULATIONS' => 'Congratulations to',
'CONNECTION_FAILED' => 'Connection failed.',
'CONNECTION_SUCCESS' => 'Connection was successful!',
diff --git a/phpBB/phpbb/avatar/driver/remote.php b/phpBB/phpbb/avatar/driver/remote.php
index 455ddebf62..1bd7f6c927 100644
--- a/phpBB/phpbb/avatar/driver/remote.php
+++ b/phpBB/phpbb/avatar/driver/remote.php
@@ -53,6 +53,11 @@ class remote extends \phpbb\avatar\driver\driver
$width = $request->variable('avatar_remote_width', 0);
$height = $request->variable('avatar_remote_height', 0);
+ if (empty($url))
+ {
+ return false;
+ }
+
if (!preg_match('#^(http|https|ftp)://#i', $url))
{
$url = 'http://' . $url;
diff --git a/phpBB/phpbb/avatar/manager.php b/phpBB/phpbb/avatar/manager.php
index 7cf136be2c..42ae61a9a2 100644
--- a/phpBB/phpbb/avatar/manager.php
+++ b/phpBB/phpbb/avatar/manager.php
@@ -41,8 +41,8 @@ class manager
static protected $default_row = array(
'avatar' => '',
'avatar_type' => '',
- 'avatar_width' => '',
- 'avatar_height' => '',
+ 'avatar_width' => 0,
+ 'avatar_height' => 0,
);
/**
@@ -307,4 +307,36 @@ class manager
return $error;
}
+
+ /**
+ * Handle deleting avatars
+ *
+ * @param \phpbb\db\driver\driver_interface $db phpBB dbal
+ * @param \phpbb\user $user phpBB user object
+ * @param array $avatar_data Cleaned user data containing the user's
+ * avatar data
+ * @param string $table Database table from which the avatar should be deleted
+ * @param string $prefix Prefix of user data columns in database
+ * @return null
+ */
+ public function handle_avatar_delete(\phpbb\db\driver\driver_interface $db, \phpbb\user $user, $avatar_data, $table, $prefix)
+ {
+ if ($driver = $this->get_driver($avatar_data['avatar_type']))
+ {
+ $driver->delete($avatar_data);
+ }
+
+ $result = self::$default_row;
+
+ foreach ($result as $key => $value)
+ {
+ $result[$prefix . $key] = $value;
+ unset($result[$key]);
+ }
+
+ $sql = 'UPDATE ' . $table . '
+ SET ' . $db->sql_build_array('UPDATE', $result) . '
+ WHERE ' . $prefix . 'id = ' . (int) $avatar_data['id'];
+ $db->sql_query($sql);
+ }
}
diff --git a/tests/avatar/fixtures/users.xml b/tests/avatar/fixtures/users.xml
new file mode 100644
index 0000000000..3e6586e909
--- /dev/null
+++ b/tests/avatar/fixtures/users.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+ <table name="phpbb_users">
+ <column>user_id</column>
+ <column>username_clean</column>
+ <column>user_permissions</column>
+ <column>user_sig</column>
+ <column>user_avatar</column>
+ <column>user_avatar_type</column>
+ <column>user_avatar_width</column>
+ <column>user_avatar_height</column>
+ <row>
+ <value>1</value>
+ <value>barfoo</value>
+ <value></value>
+ <value></value>
+ <value>foobar@example.com</value>
+ <value>avatar.driver.gravatar</value>
+ <value>80</value>
+ <value>80</value>
+ </row>
+ <row>
+ <value>2</value>
+ <value>foobar</value>
+ <value></value>
+ <value></value>
+ <value></value>
+ <value></value>
+ <value></value>
+ <value></value>
+ </row>
+ </table>
+</dataset>
diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php
index d8099b40d4..a63a13ce50 100644
--- a/tests/avatar/manager_test.php
+++ b/tests/avatar/manager_test.php
@@ -13,13 +13,18 @@
require_once dirname(__FILE__) . '/driver/foobar.php';
-class phpbb_avatar_manager_test extends \phpbb_test_case
+class phpbb_avatar_manager_test extends \phpbb_database_test_case
{
/** @var \phpbb\avatar\manager */
protected $manager;
protected $avatar_foobar;
protected $avatar_barfoo;
+ public function getDataSet()
+ {
+ return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/users.xml');
+ }
+
public function setUp()
{
global $phpbb_root_path, $phpEx;
@@ -31,7 +36,7 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
->will($this->returnArgument(0));
// Prepare dependencies for avatar manager and driver
- $config = new \phpbb\config\config(array());
+ $this->config = new \phpbb\config\config(array());
$cache = $this->getMock('\phpbb\cache\driver\driver_interface');
$path_helper = new \phpbb\path_helper(
new \phpbb\symfony_request(
@@ -52,7 +57,7 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
$guesser = new \phpbb\mimetype\guesser($guessers);
// $this->avatar_foobar will be needed later on
- $this->avatar_foobar = $this->getMock('\phpbb\avatar\driver\foobar', array('get_name'), array($config, $phpbb_root_path, $phpEx, $path_helper, $cache));
+ $this->avatar_foobar = $this->getMock('\phpbb\avatar\driver\foobar', array('get_name'), array($this->config, $phpbb_root_path, $phpEx, $path_helper, $cache));
$this->avatar_foobar->expects($this->any())
->method('get_name')
->will($this->returnValue('avatar.driver.foobar'));
@@ -76,15 +81,17 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
$cur_avatar->expects($this->any())
->method('get_name')
->will($this->returnValue('avatar.driver.' . $driver));
- $config['allow_avatar_' . get_class($cur_avatar)] = false;
+ $this->config['allow_avatar_' . get_class($cur_avatar)] = $driver == 'gravatar';
$avatar_drivers[] = $cur_avatar;
}
- $config['allow_avatar_' . get_class($this->avatar_foobar)] = true;
- $config['allow_avatar_' . get_class($this->avatar_barfoo)] = false;
+ $this->config['allow_avatar_' . get_class($this->avatar_foobar)] = true;
+ $this->config['allow_avatar_' . get_class($this->avatar_barfoo)] = false;
// Set up avatar manager
- $this->manager = new \phpbb\avatar\manager($config, $avatar_drivers, $phpbb_container);
+ $this->manager = new \phpbb\avatar\manager($this->config, $avatar_drivers, $phpbb_container);
+ $this->db = $this->new_dbal();
+ $this->user = new \phpbb\user();
}
protected function avatar_drivers()
@@ -122,6 +129,7 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
{
return array(
array('avatar.driver.foobar', 'avatar.driver.foobar'),
+ array('avatar.driver.gravatar', 'avatar.driver.gravatar'),
array('avatar.driver.foo_wrong', null),
array('avatar.driver.local', null),
array(AVATAR_GALLERY, null),
@@ -196,8 +204,8 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
array(
'avatar' => '',
'avatar_type' => '',
- 'avatar_width' => '',
- 'avatar_height' => '',
+ 'avatar_width' => 0,
+ 'avatar_height' => 0,
),
),
array(
@@ -287,4 +295,35 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
array('FOOBAR_EXPLAIN', 'foo'),
)));
}
+
+ public function data_handle_avatar_delete()
+ {
+ return array(
+ array(array(
+ 'avatar' => '',
+ 'avatar_type' => '',
+ 'avatar_width' => 0,
+ 'avatar_height' => 0,
+ ), 1, array(
+ 'avatar' => 'foobar@example.com',
+ 'avatar_type' => 'avatar.driver.gravatar',
+ 'avatar_width' => '16',
+ 'avatar_height' => '16',
+ ), USERS_TABLE, 'user_'),
+ );
+ }
+
+ /**
+ * @dataProvider data_handle_avatar_delete
+ */
+ public function test_handle_avatar_delete($expected, $id, $avatar_data, $table, $prefix)
+ {
+ $this->config['allow_avatar_gravatar'] = true;
+ $this->assertNull($this->manager->handle_avatar_delete($this->db, $this->user, $avatar_data, $table, $prefix));
+
+ $sql = 'SELECT * FROM ' . $table . '
+ WHERE ' . $prefix . 'id = ' . $id;
+ $result = $this->db->sql_query($sql);
+ $this->assertSame($expected, $this->manager->clean_row($this->db->sql_fetchrow($result), substr($prefix, -1)));
+ }
}
diff --git a/tests/functional/avatar_acp_groups_test.php b/tests/functional/avatar_acp_groups_test.php
index 35f7956252..925335a2f7 100644
--- a/tests/functional/avatar_acp_groups_test.php
+++ b/tests/functional/avatar_acp_groups_test.php
@@ -48,7 +48,7 @@ class phpbb_functional_avatar_acp_groups_test extends phpbb_functional_common_av
),
// Delete avatar image to reset group settings
array(
- 'GROUP_UPDATED',
+ array('CONFIRM_AVATAR_DELETE', 'GROUP_UPDATED'),
'avatar_driver_gravatar',
array(
'avatar_delete' => array('tick', ''),
diff --git a/tests/functional/avatar_acp_users_test.php b/tests/functional/avatar_acp_users_test.php
index 38e001f516..5eca473157 100644
--- a/tests/functional/avatar_acp_users_test.php
+++ b/tests/functional/avatar_acp_users_test.php
@@ -46,7 +46,7 @@ class phpbb_functional_avatar_acp_users_test extends phpbb_functional_common_ava
),
// Reset avatar settings
array(
- 'USER_AVATAR_UPDATED',
+ array('CONFIRM_AVATAR_DELETE', 'USER_AVATAR_UPDATED'),
'avatar_driver_gravatar',
array(
'avatar_delete' => array('tick', ''),
diff --git a/tests/functional/avatar_ucp_groups_test.php b/tests/functional/avatar_ucp_groups_test.php
index fddf66933f..1e8ca911c6 100644
--- a/tests/functional/avatar_ucp_groups_test.php
+++ b/tests/functional/avatar_ucp_groups_test.php
@@ -55,7 +55,7 @@ class phpbb_functional_avatar_ucp_groups_test extends phpbb_functional_common_av
),
),
array(
- 'GROUP_UPDATED',
+ array('CONFIRM_AVATAR_DELETE', 'GROUP_UPDATED'),
'avatar_driver_gravatar',
array(
'avatar_delete' => array('tick', ''),
diff --git a/tests/functional/avatar_ucp_users_test.php b/tests/functional/avatar_ucp_users_test.php
index 09edde8dba..972bfa0fb2 100644
--- a/tests/functional/avatar_ucp_users_test.php
+++ b/tests/functional/avatar_ucp_users_test.php
@@ -36,8 +36,9 @@ class phpbb_functional_avatar_ucp_users_test extends phpbb_functional_common_ava
'avatar_gravatar_height' => 80,
),
),
+
array(
- 'PROFILE_UPDATED',
+ array('CONFIRM_AVATAR_DELETE', 'PROFILE_UPDATED'),
'avatar_driver_gravatar',
array(
'avatar_delete' => array('tick', ''),
diff --git a/tests/functional/common_avatar_test.php b/tests/functional/common_avatar_test.php
index 1bb9e5cd8e..82d7136c98 100644
--- a/tests/functional/common_avatar_test.php
+++ b/tests/functional/common_avatar_test.php
@@ -50,7 +50,7 @@ abstract class phpbb_functional_common_avatar_test extends phpbb_functional_test
$this->assertContainsLang('CONFIG_UPDATED', $crawler->text());
}
- public function assert_avatar_submit($expected, $type, $data, $button_text = 'SUBMIT')
+ public function assert_avatar_submit($expected, $type, $data, $delete = false, $button_text = 'SUBMIT')
{
$crawler = self::request('GET', $this->get_url() . '&sid=' . $this->sid);
@@ -72,6 +72,12 @@ abstract class phpbb_functional_common_avatar_test extends phpbb_functional_test
$crawler = self::submit($form);
+ if (is_array($expected))
+ {
+ $delete_expected = $expected[1];
+ $expected = $expected[0];
+ }
+
try
{
$this->assertContainsLang($expected, $crawler->text());
@@ -80,5 +86,12 @@ abstract class phpbb_functional_common_avatar_test extends phpbb_functional_test
{
$this->assertContains($expected, $crawler->text());
}
+
+ if ($delete)
+ {
+ $form = $crawler->selectButton('confirm')->form();
+ $crawler = self::submit($form);
+ $this->assertContainsLang($delete_expected, $crawler->text());
+ }
}
}