And that's the long overdue fix for bans (again: I broke them, I tried to fix them)

Signed-off-by: emanuele <[email protected]>

Sources/ManageBans.php

@@ -365,175 +365,176 @@ function BanEdit()
 		foreach ($context['ban_errors'] as $error)
 			$context['error_messages'][$error] = $txt[$error];
 	}
-
-	// If we're editing an existing ban, get it from the database.
-	if (!empty($ban_group_id))
+	else
 	{
-		$context['ban_group_id'] = $ban_group_id;
-
-		// We're going to want this for making our list.
-		require_once($sourcedir . '/Subs-List.php');
-
-		$listOptions = array(
-			'id' => 'ban_items',
-			'base_href' => $scripturl . '?action=admin;area=ban;sa=edit;bg=' . $ban_group_id,
-			'no_items_label' => $txt['ban_no_triggers'],
-			'items_per_page' => $modSettings['defaultMaxMessages'],
-			'get_items' => array(
-				'function' => 'list_getBanItems',
-				'params' => array(
-					'ban_group_id' => $ban_group_id,
-				),
-			),
-			'get_count' => array(
-				'function' => 'list_getNumBanItems',
-				'params' => array(
-					'ban_group_id' => $ban_group_id,
-				),
-			),
-			'columns' => array(
-				'type' => array(
-					'header' => array(
-						'value' => $txt['ban_banned_entity'],
-						'style' => 'width: 60%;text-align: left;',
-					),
-					'data' => array(
-						'function' => create_function('$ban_item', '
-							global $txt;
-
-							if (in_array($ban_item[\'type\'], array(\'ip\', \'hostname\', \'email\')))
-								return \'<strong>\' . $txt[$ban_item[\'type\']] . \':</strong>&nbsp;\' . $ban_item[$ban_item[\'type\']];
-							elseif ($ban_item[\'type\'] == \'user\')
-								return \'<strong>\' . $txt[\'username\'] . \':</strong>&nbsp;\' . $ban_item[\'user\'][\'link\'];
-							else
-								return \'<strong>\' . $txt[\'unknown\'] . \':</strong>&nbsp;\' . $ban_item[\'no_bantype_selected\'];
-						'),
-						'style' => 'text-align: left;',
+		// If we're editing an existing ban, get it from the database.
+		if (!empty($ban_group_id))
+		{
+			$context['ban_group_id'] = $ban_group_id;
+
+			// We're going to want this for making our list.
+			require_once($sourcedir . '/Subs-List.php');
+
+			$listOptions = array(
+				'id' => 'ban_items',
+				'base_href' => $scripturl . '?action=admin;area=ban;sa=edit;bg=' . $ban_group_id,
+				'no_items_label' => $txt['ban_no_triggers'],
+				'items_per_page' => $modSettings['defaultMaxMessages'],
+				'get_items' => array(
+					'function' => 'list_getBanItems',
+					'params' => array(
+						'ban_group_id' => $ban_group_id,
 					),
 				),
-				'hits' => array(
-					'header' => array(
-						'value' => $txt['ban_hits'],
-						'style' => 'width: 15%; text-align: center;',
-					),
-					'data' => array(
-						'db' => 'hits',
-						'style' => 'text-align: center;',
+				'get_count' => array(
+					'function' => 'list_getNumBanItems',
+					'params' => array(
+						'ban_group_id' => $ban_group_id,
 					),
 				),
-				'id' => array(
-					'header' => array(
-						'value' => $txt['ban_actions'],
-						'style' => 'width: 15%; text-align: center;',
+				'columns' => array(
+					'type' => array(
+						'header' => array(
+							'value' => $txt['ban_banned_entity'],
+							'style' => 'width: 60%;text-align: left;',
+						),
+						'data' => array(
+							'function' => create_function('$ban_item', '
+								global $txt;
+
+								if (in_array($ban_item[\'type\'], array(\'ip\', \'hostname\', \'email\')))
+									return \'<strong>\' . $txt[$ban_item[\'type\']] . \':</strong>&nbsp;\' . $ban_item[$ban_item[\'type\']];
+								elseif ($ban_item[\'type\'] == \'user\')
+									return \'<strong>\' . $txt[\'username\'] . \':</strong>&nbsp;\' . $ban_item[\'user\'][\'link\'];
+								else
+									return \'<strong>\' . $txt[\'unknown\'] . \':</strong>&nbsp;\' . $ban_item[\'no_bantype_selected\'];
+							'),
+							'style' => 'text-align: left;',
+						),
 					),
-					'data' => array(
-						'function' => create_function('$ban_item', '
-							global $txt, $context, $scripturl;
-							
-							return \'<a href="\' . $scripturl . \'?action=admin;area=ban;sa=edittrigger;bg=\' . $context[\'ban\'][\'id\'] . \';bi=\' . $ban_item[\'id\'] . \'">\' . $txt[\'ban_edit_trigger\'] . \'</a>\';
-						'),
-						'style' => 'text-align: center;',
+					'hits' => array(
+						'header' => array(
+							'value' => $txt['ban_hits'],
+							'style' => 'width: 15%; text-align: center;',
+						),
+						'data' => array(
+							'db' => 'hits',
+							'style' => 'text-align: center;',
+						),
 					),
-				),
-				'checkboxes' => array(
-					'header' => array(
-						'value' => '<input type="checkbox" onclick="invertAll(this, this.form, \'ban_items\');" class="input_check" />',
-						'style' => 'width: 5%; text-align: center;',
+					'id' => array(
+						'header' => array(
+							'value' => $txt['ban_actions'],
+							'style' => 'width: 15%; text-align: center;',
+						),
+						'data' => array(
+							'function' => create_function('$ban_item', '
+								global $txt, $context, $scripturl;
+								
+								return \'<a href="\' . $scripturl . \'?action=admin;area=ban;sa=edittrigger;bg=\' . $context[\'ban\'][\'id\'] . \';bi=\' . $ban_item[\'id\'] . \'">\' . $txt[\'ban_edit_trigger\'] . \'</a>\';
+							'),
+							'style' => 'text-align: center;',
+						),
 					),
-					'data' => array(
-						'sprintf' => array(
-							'format' => '<input type="checkbox" name="ban_items[]" value="%1$d" class="input_check" />',
-							'params' => array(
-								'id' => false,
+					'checkboxes' => array(
+						'header' => array(
+							'value' => '<input type="checkbox" onclick="invertAll(this, this.form, \'ban_items\');" class="input_check" />',
+							'style' => 'width: 5%; text-align: center;',
+						),
+						'data' => array(
+							'sprintf' => array(
+								'format' => '<input type="checkbox" name="ban_items[]" value="%1$d" class="input_check" />',
+								'params' => array(
+									'id' => false,
+								),
 							),
+							'style' => 'text-align: center;',
 						),
-						'style' => 'text-align: center;',
 					),
 				),
-			),
-			'form' => array(
-				'href' => $scripturl . '?action=admin;area=featuresettings;sa=profile',
-				'name' => 'standardProfileFields',
-			),
-			'additional_rows' => array(
-				array(
-					'position' => 'below_table_data',
-					'value' => '
-					<input type="submit" name="remove_selection" value="' . $txt['ban_remove_selected_triggers'] . '" class="button_submit" /> <a class="button_link" href="' . $scripturl . '?action=admin;area=ban;sa=edittrigger;bg=' . $ban_group_id . '">' . $txt['ban_add_trigger'] . '</a>',
-					'style' => 'text-align: right;',
+				'form' => array(
+					'href' => $scripturl . '?action=admin;area=ban;sa=edit;bg=' . $ban_group_id,
 				),
-				array(
-					'position' => 'below_table_data',
-					'value' => '
-					<input type="hidden" name="bg" value="' . $ban_group_id . '" />
-					<input type="hidden" name="' . $context['session_var'] . '" value="' . $context['session_id'] . '" />
-					<input type="hidden" name="' . $context['admin-bet_token_var'] . '" value="' . $context['admin-bet_token'] . '" />',
+				'additional_rows' => array(
+					array(
+						'position' => 'below_table_data',
+						'value' => '
+						<input type="submit" name="remove_selection" value="' . $txt['ban_remove_selected_triggers'] . '" class="button_submit" /> <a class="button_link" href="' . $scripturl . '?action=admin;area=ban;sa=edittrigger;bg=' . $ban_group_id . '">' . $txt['ban_add_trigger'] . '</a>',
+						'style' => 'text-align: right;',
+					),
+					array(
+						'position' => 'below_table_data',
+						'value' => '
+						<input type="hidden" name="bg" value="' . $ban_group_id . '" />
+						<input type="hidden" name="' . $context['session_var'] . '" value="' . $context['session_id'] . '" />
+						<input type="hidden" name="' . $context['admin-bet_token_var'] . '" value="' . $context['admin-bet_token'] . '" />',
+					),
 				),
-			),
-		);
-		createList($listOptions);
-	}
-	// Not an existing one, then it's probably a new one.
-	else
-	{
-		$context['ban'] = array(
-			'id' => 0,
-			'name' => '',
-			'expiration' => array(
-				'status' => 'never',
-				'days' => 0
-			),
-			'reason' => '',
-			'notes' => '',
-			'ban_days' => 0,
-			'cannot' => array(
-				'access' => true,
-				'post' => false,
-				'register' => false,
-				'login' => false,
-			),
-			'is_new' => true,
-		);
-		$context['ban_suggestions'] = array(
-			'main_ip' => '',
-			'hostname' => '',
-			'email' => '',
-			'member' => array(
-				'id' => 0,
-			),
-		);
-
-		// Overwrite some of the default form values if a user ID was given.
-		if (!empty($_REQUEST['u']))
+			);
+			createList($listOptions);
+		}
+		// Not an existing one, then it's probably a new one.
+		else
 		{
-			$request = $smcFunc['db_query']('', '
-				SELECT id_member, real_name, member_ip, email_address
-				FROM {db_prefix}members
-				WHERE id_member = {int:current_user}
-				LIMIT 1',
-				array(
-					'current_user' => (int) $_REQUEST['u'],
-				)
+			$context['ban'] = array(
+				'id' => 0,
+				'name' => '',
+				'expiration' => array(
+					'status' => 'never',
+					'days' => 0
+				),
+				'reason' => '',
+				'notes' => '',
+				'ban_days' => 0,
+				'cannot' => array(
+					'access' => true,
+					'post' => false,
+					'register' => false,
+					'login' => false,
+				),
+				'is_new' => true,
+			);
+			$context['ban_suggestions'] = array(
+				'main_ip' => '',
+				'hostname' => '',
+				'email' => '',
+				'member' => array(
+					'id' => 0,
+				),
 			);
-			if ($smcFunc['db_num_rows']($request) > 0)
-				list ($context['ban_suggestions']['member']['id'], $context['ban_suggestions']['member']['name'], $context['ban_suggestions']['main_ip'], $context['ban_suggestions']['email']) = $smcFunc['db_fetch_row']($request);
-			$smcFunc['db_free_result']($request);
 
-			if (!empty($context['ban_suggestions']['member']['id']))
+			// Overwrite some of the default form values if a user ID was given.
+			if (!empty($_REQUEST['u']))
 			{
-				$context['ban_suggestions']['href'] = $scripturl . '?action=profile;u=' . $context['ban_suggestions']['member']['id'];
-				$context['ban_suggestions']['member']['link'] = '<a href="' . $context['ban_suggestions']['href'] . '">' . $context['ban_suggestions']['member']['name'] . '</a>';
+				$request = $smcFunc['db_query']('', '
+					SELECT id_member, real_name, member_ip, email_address
+					FROM {db_prefix}members
+					WHERE id_member = {int:current_user}
+					LIMIT 1',
+					array(
+						'current_user' => (int) $_REQUEST['u'],
+					)
+				);
+				if ($smcFunc['db_num_rows']($request) > 0)
+					list ($context['ban_suggestions']['member']['id'], $context['ban_suggestions']['member']['name'], $context['ban_suggestions']['main_ip'], $context['ban_suggestions']['email']) = $smcFunc['db_fetch_row']($request);
+				$smcFunc['db_free_result']($request);
+
+				if (!empty($context['ban_suggestions']['member']['id']))
+				{
+					$context['ban_suggestions']['href'] = $scripturl . '?action=profile;u=' . $context['ban_suggestions']['member']['id'];
+					$context['ban_suggestions']['member']['link'] = '<a href="' . $context['ban_suggestions']['href'] . '">' . $context['ban_suggestions']['member']['name'] . '</a>';
 
-				// Default the ban name to the name of the banned member.
-				$context['ban']['name'] = $context['ban_suggestions']['member']['name'];
-				// @todo: there should be a better solution...used to lock the "Ban on Username" input when banning from profile
-				$context['ban']['from_user'] = true;
+					// Default the ban name to the name of the banned member.
+					$context['ban']['name'] = $context['ban_suggestions']['member']['name'];
+					// @todo: there should be a better solution...used to lock the "Ban on Username" input when banning from profile
+					$context['ban']['from_user'] = true;
 
-				// Would be nice if we could also ban the hostname.
-				if ((preg_match('/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/', $context['ban_suggestions']['main_ip']) == 1 || isValidIPv6($context['ban_suggestions']['main_ip'])) && empty($modSettings['disableHostnameLookup']))
-					$context['ban_suggestions']['hostname'] = host_from_ip($context['ban_suggestions']['main_ip']);
+					// Would be nice if we could also ban the hostname.
+					if ((preg_match('/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/', $context['ban_suggestions']['main_ip']) == 1 || isValidIPv6($context['ban_suggestions']['main_ip'])) && empty($modSettings['disableHostnameLookup']))
+						$context['ban_suggestions']['hostname'] = host_from_ip($context['ban_suggestions']['main_ip']);
 
-				$context['ban_suggestions']['other_ips'] = banLoadAdditionalIPs($context['ban_suggestions']['member']['id']);
+					$context['ban_suggestions']['other_ips'] = banLoadAdditionalIPs($context['ban_suggestions']['member']['id']);
+				}
 			}
 		}
 	}
@@ -807,24 +808,29 @@ function banEdit2()
 
 	if (isset($_POST['ban_suggestions']))
 		// @TODO: is $_REQUEST['bi'] ever set?
-		$context['ban_suggestions']['saved_triggers'] = saveTriggers($_POST['ban_suggestions'], $ban_info['id'], isset($_REQUEST['u']) ? (int) $_REQUEST['u'] : 0, isset($_REQUEST['bi']) ? (int) $_REQUEST['bi'] : 0);
+		$saved_triggers = saveTriggers($_POST['ban_suggestions'], $ban_info['id'], isset($_REQUEST['u']) ? (int) $_REQUEST['u'] : 0, isset($_REQUEST['bi']) ? (int) $_REQUEST['bi'] : 0);
 
 	// Something went wrong somewhere... Oh well, let's go back.
 	if (!empty($context['ban_errors']))
 	{
+		$context['ban_suggestions'] = $saved_triggers;
+		$context['ban']['from_user'] = true;
+		$context['ban_suggestions'] = array_merge($context['ban_suggestions'], getMemberData((int) $_REQUEST['u']));
+
 		// Not strictly necessary, but it's nice
 		if (!empty($context['ban_suggestions']['member']['id']))
 			$context['ban_suggestions']['other_ips'] = banLoadAdditionalIPs($context['ban_suggestions']['member']['id']);
 		return BanEdit();
 	}
+	$context['ban_suggestions']['saved_triggers'] = $saved_triggers;
 
 	if (isset($_POST['ban_items']))
 	{
 		$items_ids = array();
-		$group_id = isset($_REQUEST['bg']) ? (int) $_REQUEST['bg'] : 0;
+		$ban_group_id = isset($_REQUEST['bg']) ? (int) $_REQUEST['bg'] : 0;
 		array_map('intval', $_POST['ban_items']);
 
-		removeBanTriggers($_POST['ban_items'], $group_id);
+		removeBanTriggers($_POST['ban_items'], $ban_group_id);
 	}
 
 	// Register the last modified date.
@@ -832,6 +838,7 @@ function banEdit2()
 
 	// Update the member table to represent the new ban situation.
 	updateBanMembers();
+	redirectexit('action=admin;area=ban;sa=edit;bg=' . $ban_group_id);
 }
 
 /**
@@ -1014,14 +1021,14 @@ function validateTriggers(&$triggers)
 
 	$ban_triggers = array();
 	$log_info = array();
-	$array_keys = array();
-	call_integration_hook('integrate_extend_ban_triggers', array(&$array_keys));
-	$array_keys += array('ips_in_messages', 'ips_in_errors');
 
 	foreach ($triggers as $key => $value)
 	{
 		if (!empty($value))
 		{
+			if ($key == 'member')
+				continue;
+
 			if ($key == 'main_ip')
 			{
 				$value = trim($value);
@@ -1114,7 +1121,7 @@ function validateTriggers(&$triggers)
 				else
 					$ban_triggers['user']['id_member'] = $value;
 			}
-			elseif (in_array($key, $array_keys))
+			elseif (in_array($key, array('ips_in_messages', 'ips_in_errors')))
 			{
 				// Special case, those two are arrays themselves
 				$values = array_unique($value);
@@ -1233,15 +1240,11 @@ function addTriggers($group_id = 0, $triggers = array(), $logs = array())
 		'ip_high8' => 'int',
 	);
 
-	$array_keys = array();
-	call_integration_hook('integrate_extend_ban_triggers', array(&$array_keys));
-	$array_keys += array('ips_in_messages', 'ips_in_errors');
-
 	$insertTriggers = array();
 	foreach ($triggers as $key => $trigger)
 	{
 		// Exceptions, exceptions, exceptions...always exceptions... :P
-		if (in_array($key, $array_keys))
+		if (in_array($key, array('ips_in_messages', 'ips_in_errors')))
 			foreach ($trigger as $real_trigger)
 				$insertTriggers[] = array_merge($values, $real_trigger);
 		else
@@ -2334,5 +2337,25 @@ function updateBanMembers()
 	updateStats('member');
 }
 
+function getMemberData($id)
+{
+	global $smcFunc;
+
+	$suggestions = array();
+	$request = $smcFunc['db_query']('', '
+		SELECT id_member, real_name, member_ip, email_address
+		FROM {db_prefix}members
+		WHERE id_member = {int:current_user}
+		LIMIT 1',
+		array(
+			'current_user' => $id,
+		)
+	);
+	if ($smcFunc['db_num_rows']($request) > 0)
+		list ($suggestions['member']['id'], $suggestions['member']['name'], $suggestions['main_ip'], $suggestions['email']) = $smcFunc['db_fetch_row']($request);
+	$smcFunc['db_free_result']($request);
+
+	return $suggestions;
+}
 
 ?>

Themes/default/ManageBans.template.php

@@ -27,7 +27,7 @@ function template_ban_edit()
 		echo '
 			<div class="information">', $txt['ban_add_notes'], '</div>';
 
-	// If there were errors for sending the PM, show them.
+	// If there were errors creating the ban, show them.
 	if (!empty($context['error_messages']))
 	{
 		echo '
@@ -172,7 +172,8 @@ function template_ban_edit()
 	echo '
 				<input type="submit" name="', $context['ban']['is_new'] ? 'add_ban' : 'modify_ban', '" value="', $context['ban']['is_new'] ? $txt['ban_add'] : $txt['ban_modify'], '" class="button_submit" />
 				<input type="hidden" name="old_expire" value="', $context['ban']['expiration']['days'], '" />
-				<input type="hidden" name="bg" value="', $context['ban']['id'], '" />
+				<input type="hidden" name="bg" value="', $context['ban']['id'], '" />', isset($context['ban']['from_user']) ? '
+				<input type="hidden" name="u" value="' . $context['ban_suggestions']['member']['id'] . '" />' : '', '
 				<input type="hidden" name="', $context['session_var'], '" value="', $context['session_id'], '" />
 				<input type="hidden" name="', $context['admin-bet_token_var'], '" value="', $context['admin-bet_token'], '" />
 			</div>