[ticket/9687] Create ban message in manager and add missing sort directions

PHPBB3-9687

phpBB/config/default/container/services_ban.yml

@@ -6,6 +6,7 @@ services:
             - '@ban.type_collection'
             - '@cache.driver'
             - '@dbal.conn'
+            - '@language'
             - '@user'
             - '%tables.bans%'
             - '%tables.users%'

phpBB/phpbb/ban/manager.php

@@ -19,6 +19,7 @@ use phpbb\ban\type\type_interface;
 use phpbb\cache\driver\driver_interface as cache_driver;
 use phpbb\db\driver\driver_interface;
 use phpbb\di\service_collection;
+use phpbb\language\language;
 use phpbb\user;
 
 class manager
@@ -39,6 +40,9 @@ class manager
 	/** @var service_collection */
 	protected $types;
 
+	/** @var language */
+	protected $language;
+
 	/** @var user */
 	protected $user;
 
@@ -56,12 +60,14 @@ class manager
 	 * @param string				$bans_table				The bans table
 	 * @param string				$users_table			The users table
 	 */
-	public function __construct(service_collection $types, cache_driver $cache, driver_interface $db, user $user, string $bans_table, string $users_table = '')
+	public function __construct(service_collection $types, cache_driver $cache, driver_interface $db,
+								language $language, user $user, string $bans_table, string $users_table = '')
 	{
 		$this->bans_table = $bans_table;
 		$this->cache = $cache;
 		$this->db = $db;
 		$this->types = $types;
+		$this->language = $language;
 		$this->user = $user;
 		$this->users_table = $users_table;
 	}
@@ -491,4 +497,25 @@ class manager
 
 		return $ban_info;
 	}
+
+	/**
+	 * Get ban info message
+	 *
+	 * @param array $ban_row Ban data row from database
+	 * @param string $ban_triggered_by Ban triggered by; allowed 'user', 'ip', 'email
+	 * @param string $contact_link Contact link URL
+	 *
+	 * @return string Ban message
+	 */
+	public function get_ban_message(array $ban_row, string $ban_triggered_by, string $contact_link): string
+	{
+		$till_date = ($ban_row && $ban_row['end'] > 0) ? $this->user->format_date($ban_row['end']) : '';
+
+		$ban_type = $ban_row['ban_end'] ? 'BOARD_BAN_TIME' : 'BOARD_BAN_PERM';
+		$message = $this->language->lang($ban_type, $till_date, '<a href="' . $contact_link . '">', '</a>');
+		$message .= $ban_row['reason'] ? '<br><br>' . $this->language->lang('BOARD_BAN_REASON', $ban_row['reason']) : '';
+		$message .= '<br><br><em>' . $this->language->lang('BAN_TRIGGERED_BY_' . strtoupper($ban_triggered_by)) . '</em>';
+
+		return $message;
+	}
 }

phpBB/phpbb/ban/type/base.php

@@ -115,7 +115,7 @@ abstract class base implements type_interface
 			WHERE (ban_end >= ' . time() . "
 					OR ban_end = 0)
 				AND ban_mode = '{$this->get_type()}'
-			ORDER BY ban_item, ban_id";
+			ORDER BY ban_item ASC, ban_id ASC";
 		$result = $this->db->sql_query($sql);
 		$rowset = $this->db->sql_fetchrowset($result);
 		$this->db->sql_freeresult($result);

phpBB/phpbb/ban/type/user.php

@@ -80,7 +80,7 @@ class user extends base
 					OR b.ban_end = 0)
 				AND b.ban_userid = u.user_id
 				AND b.ban_mode = '{$this->get_type()}'
-			ORDER BY username_clean";
+			ORDER BY username_clean ASC";
 		$result = $this->db->sql_query($sql);
 		while ($row = $this->db->sql_fetchrow($result))
 		{

phpBB/phpbb/session.php

@@ -1204,13 +1204,8 @@ class session
 			}
 
 			// Determine which message to output
-			$till_date = ($ban_row && $ban_row['end'] > 0) ? $this->format_date($ban_row['end']) : '';
-			$message = $this->get_ban_message($ban_row, $ban_triggered_by);
-
 			$contact_link = phpbb_get_board_contact_link($config, $phpbb_root_path, $phpEx);
-			$message = sprintf($this->lang[$message], $till_date, '<a href="' . $contact_link . '">', '</a>');
+			$message = $ban_manager->get_ban_message($ban_row, $ban_triggered_by, $contact_link);
-			$message .= ($ban_row['reason']) ? '<br><br>' . sprintf($this->lang['BOARD_BAN_REASON'], $ban_row['reason']) : '';
-			$message .= '<br><br><em>' . $this->lang['BAN_TRIGGERED_BY_' . strtoupper($ban_triggered_by)] . '</em>';
 
 			// A very special case... we are within the cron script which is not supposed to print out the ban message... show blank page
 			if (defined('IN_CRON'))
@@ -1254,19 +1249,6 @@ class session
 		}
 	}
 
-	/**
-	 * Get ban info message
-	 *
-	 * @param array $ban_row Ban data row from database
-	 * @param string $ban_triggered_by Ban triggered by; allowed 'user', 'ip', 'email'
-	 *
-	 * @return string
-	 */
-	protected function get_ban_message(array $ban_row, string $ban_triggered_by): string
-	{
-		return ($ban_row['ban_end']) ? 'BOARD_BAN_TIME' : 'BOARD_BAN_PERM';
-	}
-
 	/**
 	* Check if ip is blacklisted by Spamhaus SBL
 	*

tests/ban/ban_manager_test.php

@@ -48,7 +48,7 @@ class ban_manager_test extends \phpbb_session_test_case
 		$collection->add('ban.type.user');
 		$collection->add('ban.type.ip');
 
-		$this->ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $user, 'phpbb_bans', 'phpbb_users');
+		$this->ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $this->ban_manager);
 		$this->phpbb_container = $phpbb_container;
 	}
@@ -225,9 +225,9 @@ class ban_manager_test extends \phpbb_session_test_case
 				'email',
 				[
 					[
-						'ban_id' => '5',
+						'ban_id' => '9',
 						'ban_userid' => 0,
-						'ban_item' => 'bar@example.org',
+						'ban_item' => '*@foo.bar',
 						'ban_start' => '1111',
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
@@ -235,9 +235,9 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_mode' => 'email',
 					],
 					[
-						'ban_id' => '9',
+						'ban_id' => '5',
 						'ban_userid' => 0,
-						'ban_item' => '*@foo.bar',
+						'ban_item' => 'bar@example.org',
 						'ban_start' => '1111',
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
@@ -377,10 +377,7 @@ class ban_manager_test extends \phpbb_session_test_case
 		$language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx));
 		$user = new \phpbb\user($language, '\phpbb\datetime');
 
-		$config = new \phpbb\config\config([]);
+		$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users');
-		$phpbb_dispatcher = new \phpbb_mock_event_dispatcher();
-
-		$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $user, 'phpbb_bans', 'phpbb_users');
 
 		$this->assertEquals(
 			[
@@ -422,10 +419,7 @@ class ban_manager_test extends \phpbb_session_test_case
 		$language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx));
 		$user = new \phpbb\user($language, '\phpbb\datetime');
 
-		$config = new \phpbb\config\config([]);
+		$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users');
-		$phpbb_dispatcher = new \phpbb_mock_event_dispatcher();
-
-		$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $user, 'phpbb_bans', 'phpbb_users');
 
 		$start_time = new \DateTime();
 		$start_time->setTimestamp(1000);

tests/functions/validate_user_email_test.php

@@ -62,7 +62,7 @@ class phpbb_functions_validate_user_email_test extends phpbb_database_test_case
 		$collection->add('ban.type.user');
 		$collection->add('ban.type.ip');
 
-		$ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $this->user, 'phpbb_bans', 'phpbb_users');
+		$ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $this->user, 'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);
 	}
 

tests/session/check_ban_test.php

@@ -87,7 +87,7 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case
 		$collection->add('ban.type.user');
 		$collection->add('ban.type.ip');
 
-		$ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $user, 'phpbb_bans', 'phpbb_users');
+		$ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);
 	}
 

tests/session/testable_factory.php

@@ -121,7 +121,7 @@ class phpbb_session_testable_factory
 		$collection->add('ban.type.user');
 		$collection->add('ban.type.ip');
 
-		$ban_manager = new \phpbb\ban\manager($collection, $cache, $db,  $user,'phpbb_bans', 'phpbb_users');
+		$ban_manager = new \phpbb\ban\manager($collection, $cache, $db, $language,  $user,'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);
 
 		$session = new phpbb_mock_session_testable;