Merge pull request #6504 from marc1706/ticket/17153

[ticket/17153] Use avatar classes and twig to render avatars

phpBB/config/default/container/services_avatar.yml

@@ -56,11 +56,11 @@ services:
         class: phpbb\avatar\driver\upload
         arguments:
             - '@config'
-            - '@controller.helper'
             - '%core.root_path%'
             - '%core.php_ext%'
             - '@storage.avatar'
             - '@path_helper'
+            - '@routing.helper'
             - '@event_dispatcher'
             - '@files.factory'
             - '@php_ini'

phpBB/config/default/container/services_twig.yml

@@ -45,6 +45,8 @@ services:
 
     template.twig.extensions.avatar:
         class: phpbb\template\twig\extension\avatar
+        arguments:
+            - '@avatar.helper'
         tags:
             - { name: twig.extension }
 

phpBB/config/installer/container/services.yml

@@ -1,5 +1,6 @@
 imports:
     - { resource: services_installer.yml }
+    - { resource: services_avatar.yml }
     - { resource: ../../default/container/services_event.yml }
     - { resource: ../../default/container/services_filesystem.yml }
     - { resource: ../../default/container/services_http.yml }

phpBB/config/installer/container/services_avatar.yml

@@ -0,0 +1,25 @@
+services:
+    avatar.manager:
+        class: phpbb\avatar\manager
+        arguments:
+            - '@config'
+            - '@dispatcher'
+            - '@avatar.driver_collection'
+
+    avatar.helper:
+        class: phpbb\avatar\helper
+        arguments:
+            - '@config'
+            - '@dispatcher'
+            - '@language'
+            - '@avatar.manager'
+            - '@path_helper'
+            - '@user'
+
+    # ----- Avatar drivers -----
+    avatar.driver_collection:
+        class: phpbb\di\service_collection
+        arguments:
+            - '@service_container'
+        tags:
+            - { name: service_collection, tag: avatar.driver }

phpBB/includes/functions.php

@@ -3544,127 +3544,6 @@ function phpbb_quoteattr($data, $entities = null)
 	return $data;
 }
 
-/**
-* Get user avatar
-*
-* @deprecated 4.0.0 Use \phpbb\avatar\helper::get_user_avatar() instead
-*
-* @param array $user_row Row from the users table
-* @param string $alt Optional language string for alt tag within image, can be a language key or text
-* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
-* @param bool $lazy If true, will be lazy loaded (requires JS)
-*
-* @return string Avatar html
-*/
-function phpbb_get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false, $lazy = false)
-{
-	$row = \phpbb\avatar\manager::clean_row($user_row, 'user');
-	return phpbb_get_avatar($row, $alt, $ignore_config, $lazy);
-}
-
-/**
-* Get group avatar
-*
-* @deprecated 4.0.0 Use \phpbb\avatar\helper::get_group_avatar() instead
-*
-* @param array $group_row Row from the groups table
-* @param string $alt Optional language string for alt tag within image, can be a language key or text
-* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
-* @param bool $lazy If true, will be lazy loaded (requires JS)
-*
-* @return string Avatar html
-*/
-function phpbb_get_group_avatar($group_row, $alt = 'GROUP_AVATAR', $ignore_config = false, $lazy = false)
-{
-	$row = \phpbb\avatar\manager::clean_row($group_row, 'group');
-	return phpbb_get_avatar($row, $alt, $ignore_config, $lazy);
-}
-
-/**
-* Get avatar
-*
-* @deprecated 4.0.0 Use \phpbb\avatar\helper::get_avatar() instead
-*
-* @param array $row Row cleaned by \phpbb\avatar\manager::clean_row
-* @param string $alt Optional language string for alt tag within image, can be a language key or text
-* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
-* @param bool $lazy If true, will be lazy loaded (requires JS)
-*
-* @return string Avatar html
-*/
-function phpbb_get_avatar($row, $alt, $ignore_config = false, $lazy = false)
-{
-	global $user, $config;
-	global $phpbb_container, $phpbb_dispatcher;
-
-	if (!$config['allow_avatar'] && !$ignore_config)
-	{
-		return '';
-	}
-
-	$avatar_data = array(
-		'src' => $row['avatar'],
-		'width' => $row['avatar_width'],
-		'height' => $row['avatar_height'],
-	);
-
-	/* @var $phpbb_avatar_manager \phpbb\avatar\manager */
-	$phpbb_avatar_manager = $phpbb_container->get('avatar.manager');
-	$driver = $phpbb_avatar_manager->get_driver($row['avatar_type'], !$ignore_config);
-	$html = '';
-
-	if ($driver)
-	{
-		$html = $driver->get_custom_html($user, $row, $alt);
-		$avatar_data = $driver->get_data($row);
-	}
-	else
-	{
-		$avatar_data['src'] = '';
-	}
-
-	if (empty($html) && !empty($avatar_data['src']))
-	{
-		if ($lazy)
-		{
-			// This path is sent with the base template paths in the assign_vars()
-			// call below. We need to correct it in case we are accessing from a
-			// controller because the web paths will be incorrect otherwise.
-			$phpbb_path_helper = $phpbb_container->get('path_helper');
-			$web_path = $phpbb_path_helper->get_web_root_path();
-
-			$theme = "{$web_path}styles/" . rawurlencode($user->style['style_path']) . '/theme';
-
-			$src = 'src="' . $theme . '/images/no_avatar.gif" data-src="' . $avatar_data['src'] . '"';
-		}
-		else
-		{
-			$src = 'src="' . $avatar_data['src'] . '"';
-		}
-
-		$html = '<img class="avatar" ' . $src . ' ' .
-			($avatar_data['width'] ? ('width="' . $avatar_data['width'] . '" ') : '') .
-			($avatar_data['height'] ? ('height="' . $avatar_data['height'] . '" ') : '') .
-			'alt="' . ((!empty($user->lang[$alt])) ? $user->lang[$alt] : $alt) . '" />';
-	}
-
-	/**
-	* Event to modify HTML <img> tag of avatar
-	*
-	* @event core.get_avatar_after
-	* @var	array	row				Row cleaned by \phpbb\avatar\manager::clean_row
-	* @var	string	alt				Optional language string for alt tag within image, can be a language key or text
-	* @var	bool	ignore_config	Ignores the config-setting, to be still able to view the avatar in the UCP
-	* @var	array	avatar_data		The HTML attributes for avatar <img> tag
-	* @var	string	html			The HTML <img> tag of generated avatar
-	* @since 3.1.6-RC1
-	*/
-	$vars = array('row', 'alt', 'ignore_config', 'avatar_data', 'html');
-	extract($phpbb_dispatcher->trigger_event('core.get_avatar_after', compact($vars)));
-
-	return $html;
-}
-
 /**
 * Generate page header
 */

phpBB/includes/functions_compatibility.php

@@ -19,34 +19,6 @@ if (!defined('IN_PHPBB'))
 	exit;
 }
 
-/**
-* Get user avatar
-*
-* @deprecated 3.1.0-a1 (To be removed: 4.0.0)
-*
-* @param string $avatar Users assigned avatar name
-* @param int $avatar_type Type of avatar
-* @param string $avatar_width Width of users avatar
-* @param string $avatar_height Height of users avatar
-* @param string $alt Optional language string for alt tag within image, can be a language key or text
-* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
-* @param bool $lazy If true, will be lazy loaded (requires JS)
-*
-* @return string Avatar image
-*/
-function get_user_avatar($avatar, $avatar_type, $avatar_width, $avatar_height, $alt = 'USER_AVATAR', $ignore_config = false, $lazy = false)
-{
-	// map arguments to new function phpbb_get_avatar()
-	$row = array(
-		'avatar'		=> $avatar,
-		'avatar_type'	=> $avatar_type,
-		'avatar_width'	=> $avatar_width,
-		'avatar_height'	=> $avatar_height,
-	);
-
-	return phpbb_get_avatar($row, $alt, $ignore_config, $lazy);
-}
-
 /**
 * Hash the password
 *

phpBB/phpbb/avatar/driver/upload.php

@@ -15,10 +15,10 @@ namespace phpbb\avatar\driver;
 
 use bantu\IniGetWrapper\IniGetWrapper;
 use phpbb\config\config;
-use phpbb\controller\helper;
 use phpbb\event\dispatcher_interface;
 use phpbb\files\factory;
 use phpbb\path_helper;
+use phpbb\routing\helper;
 use phpbb\storage\exception\exception as storage_exception;
 use phpbb\storage\storage;
 
@@ -30,7 +30,7 @@ class upload extends \phpbb\avatar\driver\driver
 	/**
 	 * @var helper
 	 */
-	private $controller_helper;
+	private $routing_helper;
 
 	/**
 	 * @var storage
@@ -56,23 +56,23 @@ class upload extends \phpbb\avatar\driver\driver
 	 * Construct a driver object
 	 *
 	 * @param config $config phpBB configuration
-	 * @param helper $controller_helper
 	 * @param string $phpbb_root_path Path to the phpBB root
 	 * @param string $php_ext PHP file extension
 	 * @param storage $storage phpBB avatar storage
 	 * @param path_helper $path_helper phpBB path helper
+	 * @param helper $routing_helper phpBB routing helper
 	 * @param dispatcher_interface $dispatcher phpBB Event dispatcher object
 	 * @param factory $files_factory File classes factory
 	 * @param IniGetWrapper $php_ini ini_get() wrapper
 	 */
-	public function __construct(config $config, helper $controller_helper, string $phpbb_root_path, string $php_ext, storage $storage, path_helper $path_helper, dispatcher_interface $dispatcher, factory $files_factory, IniGetWrapper $php_ini)
+	public function __construct(config $config, string $phpbb_root_path, string $php_ext, storage $storage, path_helper $path_helper, helper $routing_helper, dispatcher_interface $dispatcher, factory $files_factory, IniGetWrapper $php_ini)
 	{
 		$this->config = $config;
-		$this->controller_helper = $controller_helper;
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->php_ext = $php_ext;
 		$this->storage = $storage;
 		$this->path_helper = $path_helper;
+		$this->routing_helper = $routing_helper;
 		$this->dispatcher = $dispatcher;
 		$this->files_factory = $files_factory;
 		$this->php_ini = $php_ini;
@@ -84,7 +84,7 @@ class upload extends \phpbb\avatar\driver\driver
 	public function get_data($row)
 	{
 		return array(
-			'src' => $this->controller_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]),
+			'src' => $this->routing_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]),
 			'width' => $row['avatar_width'],
 			'height' => $row['avatar_height'],
 		);

phpBB/phpbb/template/twig/extension/avatar.php

@@ -13,16 +13,36 @@
 
 namespace phpbb\template\twig\extension;
 
+use phpbb\avatar\helper;
+use phpbb\avatar\manager;
+use phpbb\template\twig\environment;
+use Twig\Error\Error;
 use Twig\Extension\AbstractExtension;
+use Twig\TwigFunction;
 
 class avatar extends AbstractExtension
 {
+	/**
+	 * @var helper
+	 */
+	private $avatar_helper;
+
+	/**
+	 * Constructor for avatar extension
+	 *
+	 * @param helper $avatar_helper
+	 */
+	public function __construct(helper $avatar_helper)
+	{
+		$this->avatar_helper = $avatar_helper;
+	}
+
 	/**
 	 * Get the name of this extension
 	 *
 	 * @return string
 	 */
-	public function getName()
+	public function getName(): string
 	{
 		return 'avatar';
 	}
@@ -30,13 +50,13 @@ class avatar extends AbstractExtension
 	/**
 	 * Returns a list of global functions to add to the existing list.
 	 *
-	 * @return \Twig\TwigFunction[] An array of global functions
+	 * @return TwigFunction[] An array of global functions
 	 */
 	public function getFunctions(): array
 	{
-		return array(
-			new \Twig\TwigFunction('avatar', array($this, 'get_avatar')),
-		);
+		return [
+			new TwigFunction('avatar', [$this, 'get_avatar'], ['needs_environment' => true]),
+		];
 	}
 
 	/**
@@ -48,35 +68,30 @@ class avatar extends AbstractExtension
 	 * The mode and row (group_row or user_row) are required.
 	 * The other fields (alt|ignore_config|lazy) are optional.
 	 *
-	 * @uses \phpbb_get_group_avatar()
-	 * @uses \phpbb_get_user_avatar()
-	 *
 	 * @return string	The avatar HTML for the specified mode
 	 */
-	public function get_avatar()
+	public function get_avatar(environment $environment, string $mode, array $row, ?string $alt, ?bool $ignore_config, ?bool $lazy): string
 	{
-		$args = func_get_args();
+		$alt = $alt ?? false;
+		$ignore_config = $ignore_config ?? false;
+		$lazy = $lazy ?? false;
+		$row = manager::clean_row($row, $mode);
+		$avatar = $this->avatar_helper->get_avatar($row, $alt, $ignore_config, $lazy);
 
-		$mode = (string) $args[0];
-		$row = (array) $args[1];
-		$alt = isset($args[2]) ? (string) $args[2] : false;
-		$ignore_config = isset($args[3]) ? (bool) $args[3] : false;
-		$lazy = isset($args[4]) ? (bool) $args[4] : false;
-
-		// To prevent having to redefine alt attribute ('USER_AVATAR'|'GROUP_AVATAR'), we check if an alternative has been provided
-		switch ($mode)
+		try
 		{
-			case 'group':
-				return $alt ? phpbb_get_group_avatar($row, $alt, $ignore_config, $lazy) : phpbb_get_group_avatar($row);
-			break;
-
-			case 'user':
-				return $alt ? phpbb_get_user_avatar($row, $alt, $ignore_config, $lazy) : phpbb_get_user_avatar($row);
-			break;
-
-			default:
-				return '';
-			break;
+			return $environment->render('macros/avatar.twig', [
+				'SRC'		=> $avatar['lazy'] ? $this->avatar_helper->get_no_avatar_source() : $avatar['src'],
+				'DATA_SRC'	=> $avatar['lazy'] ? $avatar['src'] : '',
+				'WIDTH'		=> $avatar['width'],
+				'HEIGHT'	=> $avatar['height'],
+				'TITLE'		=> $avatar['title'],
+				'LAZY'		=> $avatar['lazy'],
+			]);
+		}
+		catch (Error $e)
+		{
+			return '';
 		}
 	}
 }

phpBB/styles/all/template/macros/avatar.twig

@@ -0,0 +1 @@
+{% if SRC %}<img class="avatar" src="{{ SRC }}"{% if DATA_SRC %} data-src="{{ DATA_SRC }}"{% endif %}{% if WIDTH %} width="{{ WIDTH }}"{% endif %}{% if HEIGHT %} height="{{ HEIGHT }}"{% endif %} alt="{{ lang(TITLE) }}">{% endif %}

tests/avatar/manager_test.php

@@ -58,6 +58,7 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case
 		$phpbb_dispatcher = $dispatcher;
 
 		$controller_helper = $this->createMock('\phpbb\controller\helper');
+		$routing_helper = $this->createMock('\phpbb\routing\helper');
 
 		// $this->avatar_foobar will be needed later on
 		$this->avatar_foobar = $this->getMockBuilder('\phpbb\avatar\driver\foobar')
@@ -96,7 +97,7 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case
 			{
 				$cur_avatar = $this->getMockBuilder('\phpbb\avatar\driver\\' . $driver)
 				->setMethods(array('get_name'))
-				->setConstructorArgs(array($this->config, $controller_helper, $phpbb_root_path, $phpEx, $storage, $path_helper, $dispatcher, $files_factory, $php_ini))
+				->setConstructorArgs(array($this->config, $phpbb_root_path, $phpEx, $storage, $path_helper, $routing_helper, $dispatcher, $files_factory, $php_ini))
 				->getMock();
 			}
 			$cur_avatar->expects($this->any())

tests/template/extension_test.php

@@ -11,8 +11,6 @@
  *
  */
 
-use phpbb\controller\helper;
-
 require_once __DIR__ . '/template_test_case.php';
 
 class phpbb_template_extension_test extends phpbb_template_template_test_case
@@ -34,7 +32,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 		$this->user->style['style_parent_id'] = 0;
 
 		global $auth, $request, $symfony_request, $user;
-		$user = new phpbb_mock_user();
+		$user = $this->createMock(\phpbb\user::class);
 		$user->optionset('user_id', 2);
 		$user->style['style_path'] = '';
 		$user->data['user_id'] = 2;
@@ -68,9 +66,8 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 			->disableOriginalConstructor()
 			->getMock();
 
-		$controller_helper = $this->createMock(helper::class);
-		$controller_helper
-			->method('route')
+		$routing_helper = $this->createMock(\phpbb\routing\helper::class);
+		$routing_helper->method('route')
 			->willReturnCallback(function($route, $params) {
 				return 'download/avatar/' . $params['file'];
 			});
@@ -78,7 +75,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
 		$phpbb_container = new phpbb_mock_container_builder();
 		$files = new phpbb\files\factory($phpbb_container);
-		$upload_avatar_driver = new phpbb\avatar\driver\upload($config, $controller_helper, $phpbb_root_path, $phpEx, $storage, $phpbb_path_helper, $phpbb_dispatcher, $files, new \bantu\IniGetWrapper\IniGetWrapper());
+		$upload_avatar_driver = new phpbb\avatar\driver\upload($config, $phpbb_root_path, $phpEx, $storage, $phpbb_path_helper, $routing_helper, $phpbb_dispatcher, $files, new \bantu\IniGetWrapper\IniGetWrapper());
 		$upload_avatar_driver->set_name('avatar.driver.upload');
 		$phpbb_container->set('avatar.manager', new \phpbb\avatar\manager($config, $phpbb_dispatcher, [
 			$upload_avatar_driver,
@@ -89,6 +86,14 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 		$enabled_drivers = $class->getProperty('enabled_drivers');
 		$enabled_drivers->setAccessible(true);
 		$enabled_drivers->setValue($class, false);
+		$avatar_helper = new phpbb\avatar\helper(
+			$config,
+			$phpbb_dispatcher,
+			$lang,
+			$phpbb_container->get('avatar.manager'),
+			$phpbb_path_helper,
+			$user
+		);
 
 		$this->template_path = $this->test_path . '/templates';
 
@@ -122,7 +127,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 			$this->user,
 			[
 				new \phpbb\template\twig\extension($context, $twig, $this->lang),
-				new \phpbb\template\twig\extension\avatar(),
+				new \phpbb\template\twig\extension\avatar($avatar_helper),
 				new \phpbb\template\twig\extension\config($config),
 				new \phpbb\template\twig\extension\icon($this->user),
 				new \phpbb\template\twig\extension\username(),
@@ -153,7 +158,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 				],
 				[],
 				[],
-				'<img class="avatar" src="download/avatar/great_avatar.png" width="90" height="90" alt="foo" />',
+				'<img class="avatar" src="download/avatar/great_avatar.png" width="90" height="90" alt="foo">',
 				[]
 			],
 			[
@@ -171,7 +176,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 				],
 				[],
 				[],
-				'<img class="avatar" src="phpBB/styles//theme/images/no_avatar.gif" data-src="download/avatar/great_avatar.png" width="90" height="90" alt="foo" />',
+				'<img class="avatar" src="phpBB/styles//theme/images/no_avatar.gif" data-src="download/avatar/great_avatar.png" width="90" height="90" alt="foo">',
 				[]
 			],
 			[
@@ -190,6 +195,56 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case
 				'',
 				[]
 			],
+			[
+				'avatar_group.html',
+				[
+					'row' => [
+						'group_avatar' => 'great_avatar.png',
+						'group_avatar_type' => 'avatar.driver.upload',
+						'group_avatar_width' => 90,
+						'group_avatar_height' => 90,
+					],
+					'alt' => 'foo'
+				],
+				[],
+				[],
+				'<img class="avatar" src="download/avatar/great_avatar.png" width="90" height="90" alt="foo">',
+				[]
+			],
+			[
+				'avatar_group.html',
+				[
+					'row' => [
+						'group_avatar' => 'great_avatar.png',
+						'group_avatar_type' => 'avatar.driver.upload',
+						'group_avatar_width' => 90,
+						'group_avatar_height' => 90,
+					],
+					'alt' => 'foo',
+					'ignore_config' => true,
+					'lazy' => true,
+				],
+				[],
+				[],
+				'<img class="avatar" src="phpBB/styles//theme/images/no_avatar.gif" data-src="download/avatar/great_avatar.png" width="90" height="90" alt="foo">',
+				[]
+			],
+			[
+				'avatar_group.html',
+				[
+					'row' => [
+						'group_avatar' => 'foo@bar.com',
+						'group_avatar_type' => 'avatar.driver.gravatar',
+						'group_avatar_width' => 90,
+						'group_avatar_height' => 90,
+					],
+					'alt' => 'foo'
+				],
+				[],
+				[],
+				'',
+				[]
+			],
 			[
 				'extension_username_test.html',
 				[

tests/template/templates/avatar_group.html

@@ -0,0 +1 @@
+{{ avatar('group', row, alt, ignore_config, lazy) }}