[ticket/13713] Refactor sorting functionality

PHPBB3-13713

phpBB/adm/style/acp_posting_buttons.html

@@ -13,7 +13,7 @@
 <!-- INCLUDEJS {T_ASSETS_PATH}/javascript/editor.js -->
 
 <!-- EVENT acp_posting_buttons_before -->
-<div id="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-topic-id="{S_TOPIC_ID}"<!-- ENDIF -->>
+<div id="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-mention-batch-size="{S_MENTION_BATCH_SIZE}" data-topic-id="{S_TOPIC_ID}"<!-- ENDIF -->>
 	<input type="button" class="button2" accesskey="b" name="addbbcode0" value=" B " style="font-weight:bold; width: 30px" onclick="bbstyle(0)" title="{L_BBCODE_B_HELP}" />
 	<input type="button" class="button2" accesskey="i" name="addbbcode2" value=" i " style="font-style:italic; width: 30px" onclick="bbstyle(2)" title="{L_BBCODE_I_HELP}" />
 	<input type="button" class="button2" accesskey="u" name="addbbcode4" value=" u " style="text-decoration: underline; width: 30px" onclick="bbstyle(4)" title="{L_BBCODE_U_HELP}" />

phpBB/assets/javascript/editor.js

@@ -386,21 +386,40 @@ function getCaretPosition(txtarea) {
 
 (function($) {
 	function Mentions() {
-		let $mentionDataContainer = $('[data-mention-url]:first'), cachedNames = null, filteredNamesCount = 0;
+		let $mentionDataContainer = $('[data-mention-url]:first');
+		let cachedNames = null;
+		let cachedFor = null;
+		let cachedSearchKey = 'name';
 
 		function defaultAvatar(type) {
 			return (type === 'group') ? '<svg class="mention-media-avatar" xmlns="http://www.w3.org/2000/svg" viewbox="0 0 24 24"><path fill-rule="evenodd" d="M16 11c1.66 0 2.99-1.34 2.99-3S17.66 5 16 5c-1.66 0-3 1.34-3 3s1.34 3 3 3zm-8 0c1.66 0 2.99-1.34 2.99-3S9.66 5 8 5C6.34 5 5 6.34 5 8s1.34 3 3 3zm0 2c-2.33 0-7 1.17-7 3.5V19h14v-2.5c0-2.33-4.67-3.5-7-3.5zm8 0c-.29 0-.62.02-.97.05 1.16.84 1.97 1.97 1.97 3.45V19h6v-2.5c0-2.33-4.67-3.5-7-3.5z"/></svg>' : '<svg class="mention-media-avatar" xmlns="http://www.w3.org/2000/svg" viewbox="0 0 24 24"><path fill-rule="evenodd" d="M12,19.2C9.5,19.2 7.29,17.92 6,16C6.03,14 10,12.9 12,12.9C14,12.9 17.97,14 18,16C16.71,17.92 14.5,19.2 12,19.2M12,5A3,3 0 0,1 15,8A3,3 0 0,1 12,11A3,3 0 0,1 9,8A3,3 0 0,1 12,5M12,2A10,10 0 0,0 2,12A10,10 0 0,0 12,22A10,10 0 0,0 22,12C22,6.47 17.5,2 12,2Z"/></svg>';
 		}
 
+		function getMatchedNames(query, items, searchKey) {
+			let i, len;
+			let _results = [];
+			for (i = 0, len = items.length; i < len; i++) {
+				let item = items[i];
+				if (String(item[searchKey]).toLowerCase().indexOf(query.toLowerCase()) === 0) {
+					_results.push(item);
+				}
+			}
+			return _results;
+		}
+
+		function getNumberOfMatchedCachedNames(query) {
+			return getMatchedNames(query, cachedNames, cachedSearchKey).length;
+		}
+
 		this.isEnabled = function() {
 			return $mentionDataContainer.length;
 		};
 
 		this.handle = function(txtarea) {
-			let mentionURL = $mentionDataContainer.data('mentionUrl'),
+			let mentionURL = $mentionDataContainer.data('mentionUrl');
-				mentionNamesLimit = $mentionDataContainer.data('mentionNamesLimit'),
+			let mentionBatchSize = $mentionDataContainer.data('mentionBatchSize');
-				mentionTopicId = $mentionDataContainer.data('topicId'),
+			let mentionNamesLimit = $mentionDataContainer.data('mentionNamesLimit');
-				defaultFilter = $.fn.atwho['default'].callbacks.filter;
+			let mentionTopicId = $mentionDataContainer.data('topicId');
 			$(txtarea).atwho({
 				at: "@",
 				acceptSpaceBar: true,
@@ -413,51 +432,78 @@ function getCaretPosition(txtarea) {
 				insertTpl: "[mention=${type}:${id}]${name}[/mention]",
 				limit: mentionNamesLimit,
 				callbacks: {
-					filter: function(query, data, searchKey) {
-						data = defaultFilter(query, data, searchKey);
-
-						// Update our cached statistics used by remoteFilter
-						filteredNamesCount = data.length;
-
-						return data;
-					},
 					remoteFilter: function(query, callback) {
-						if (cachedNames && filteredNamesCount >= mentionNamesLimit) {
+						/*
+						* Use cached values when we can:
+						* 1) There are some names in the cache
+						* 2) The cache contains relevant data for the query
+						*    (it was made for the query with the same first characters)
+						* 3) We have enough names to display OR
+						*    all relevant names have been fetched from the server
+						*/
+						if (cachedNames &&
+							query.indexOf(cachedFor) === 0 &&
+							(getNumberOfMatchedCachedNames(query) >= mentionNamesLimit ||
+								cachedNames.length < mentionBatchSize)) {
 							callback(cachedNames);
 							return;
 						}
+
 						let params = {keyword: query, topic_id: mentionTopicId, _referer: location.href};
 						$.getJSON(mentionURL, params, function (data) {
 							cachedNames = data;
+							cachedFor = query;
 							callback(data);
 						});
 					},
 					sorter: function(query, items, searchKey) {
-						let _unsorted, _results, _exactMatch, i, item, len, highestPriority = 0;
+						let i, len;
-						_unsorted = {u: {}, g: {}};
+						let highestPriority = 0;
-						_exactMatch = [];
+						let _unsorted = {u: {}, g: {}};
+						let _exactMatch = [];
+						let _results = [];
+
+						// Reduce the items array to the relevant ones
+						items = getMatchedNames(query, items, searchKey);
+
+						// Group names by their types and calculate priorities
 						for (i = 0, len = items.length; i < len; i++) {
-							item = items[i];
+							let item = items[i];
+
+							// Exact matches should not be prioritised - they always come first
 							if (item.name === query) {
 								_exactMatch.push(items[i]);
 								continue;
 							}
+
+							// Check for unsupported type - in general, this should never happen
 							if (!_unsorted[item.type]) {
 								continue;
 							}
+
+							// If the item hasn't been added yet - add it
+							// Group names do not have priorities and are also handled here
 							if (!_unsorted[item.type][item.id] || item.type === 'g') {
 								_unsorted[item.type][item.id] = item;
 								continue;
 							}
+
+							// Priority is calculated as the sum of priorities from different sources
 							_unsorted[item.type][item.id].priority += parseFloat(item.priority);
+
+							// Calculate the highest priority - we'll give it to group names
 							highestPriority = Math.max(highestPriority, _unsorted[item.type][item.id].priority);
 						}
-						_results = [];
+
+						// Push user names to the result array
 						if (_unsorted['u']) {
 							$.each(_unsorted['u'], function(name, value) {
 								_results.push(value);
 							});
 						}
+
+						// Push group names to the result array and give them the highest priority
+						// They will be sorted together with the usernames on top of the list
 						if (_unsorted['g']) {
 							$.each(_unsorted['g'], function(name, value) {
 								// Groups should come at the same level of importance
@@ -466,12 +512,17 @@ function getCaretPosition(txtarea) {
 								_results.push(value);
 							});
 						}
+
+						// Sort names by priorities - higher values come first
 						_results = _results.sort(function(a, b) {
-							return a.priority - b.priority;
+							return b.priority - a.priority;
 						});
+
+						// Exact match is the most important - should come above anything else
 						$.each(_exactMatch, function(name, value) {
 							_results.unshift(value);
 						});
+
 						return _results;
 					}
 				}

phpBB/includes/functions.php

@@ -3947,6 +3947,7 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		'U_FEED'				=> $controller_helper->route('phpbb_feed_index'),
 
 		'S_ALLOW_MENTIONS'		=> ($config['allow_mentions'] && $auth->acl_get('u_mention') && (empty($forum_id) || $auth->acl_get('f_mention', $forum_id))) ? true : false,
+		'S_MENTION_BATCH_SIZE'	=> 100, // TODO: do not hardcode the value
 		'S_MENTION_NAMES_LIMIT'	=> $config['mention_names_limit'],
 		'U_MENTION_URL'			=> $controller_helper->route('phpbb_mention_controller'),
 

phpBB/includes/functions_acp.php

@@ -90,6 +90,7 @@ function adm_page_header($page_title)
 		'U_INDEX'				=> append_sid("{$phpbb_root_path}index.$phpEx"),
 
 		'S_ALLOW_MENTIONS'		=> ($config['allow_mentions'] && $auth->acl_get('u_mention')) ? true : false,
+		'S_MENTION_BATCH_SIZE'	=> 100, // TODO: do not hardcode the value
 		'S_MENTION_NAMES_LIMIT'	=> $config['mention_names_limit'],
 		'U_MENTION_URL'			=> $controller_helper->route('phpbb_mention_controller'),
 

phpBB/phpbb/mention/controller/mention.php

@@ -55,7 +55,7 @@ class mention
 
 		foreach ($this->mention_sources as $source)
 		{
-			$names += $source->get($keyword, $topic_id);
+			$source->get($names, $keyword, $topic_id);
 		}
 
 		return new JsonResponse(array_values($names));

phpBB/phpbb/mention/source/base_group.php

@@ -109,7 +109,7 @@ abstract class base_group implements source_interface
 	/**
 	 * {@inheritdoc}
 	 */
-	public function get($keyword, $topic_id)
+	public function get(array &$names, $keyword, $topic_id)
 	{
 		// Grab all group IDs
 		$result = $this->db->sql_query($this->query($keyword, $topic_id));
@@ -128,11 +128,10 @@ abstract class base_group implements source_interface
 		$matches = preg_grep('/^' . preg_quote($keyword) . '.*/i', $groups['names']);
 		$group_ids = array_intersect($group_ids, array_flip($matches));
 
-		$names = [];
 		foreach ($group_ids as $group_id)
 		{
 			$group_rank = phpbb_get_user_rank($groups[$group_id], false);
-			$names[] = [
+			array_push($names, [
 				'name'		=> $groups[$group_id]['group_name'],
 				'type'		=> 'g',
 				'id'		=> $group_id,
@@ -141,7 +140,7 @@ abstract class base_group implements source_interface
 					'img'	=> phpbb_get_group_avatar($groups[$group_id]),
 				],
 				'rank'		=> $group_rank['title'],
-			];
+			]);
 		}
 
 		return $names;

phpBB/phpbb/mention/source/base_user.php

@@ -73,16 +73,15 @@ abstract class base_user implements source_interface
 	/**
 	 * {@inheritdoc}
 	 */
-	public function get($keyword, $topic_id)
+	public function get(array &$names, $keyword, $topic_id)
 	{
 		$keyword = utf8_clean_string($keyword);
 		$result = $this->db->sql_query_limit($this->query($keyword, $topic_id), self::NAMES_BATCH_SIZE);
 
-		$names = [];
 		while ($row = $this->db->sql_fetchrow($result))
 		{
 			$user_rank = $this->user_loader->get_rank($row['user_id'], true);
-			$names[] = [
+			array_push($names, [
 				'name'		=> $row['username'],
 				'type'		=> 'u',
 				'id'		=> $row['user_id'],
@@ -92,7 +91,7 @@ abstract class base_user implements source_interface
 				],
 				'rank'		=> (isset($user_rank['rank_title'])) ? $user_rank['rank_title'] : '',
 				'priority'	=> $this->get_priority($row),
-			];
+			]);
 		}
 
 		$this->db->sql_freeresult($result);

phpBB/phpbb/mention/source/friend.php

@@ -45,7 +45,6 @@ class friend extends base_user
 				]
 			],
 			'WHERE'     => 'z.friend = 1 AND z.user_id = ' . (int) $this->user->data['user_id'] . '
-				AND u.user_id <> ' . ANONYMOUS . '
 				AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . '
 				AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()),
 			'ORDER_BY'  => 'u.user_lastvisit DESC'

phpBB/phpbb/mention/source/source_interface.php

@@ -19,9 +19,10 @@ interface source_interface
 	 * Searches database for names to mention
 	 * and returns and array of found items
 	 *
+	 * @param array  $names    Array of already fetched data with names
 	 * @param string $keyword  Search string
 	 * @param int    $topic_id Current topic ID
 	 * @return array Array of names
 	 */
-	public function get($keyword, $topic_id);
+	public function get(array &$names, $keyword, $topic_id);
 }

phpBB/phpbb/mention/source/team.php

@@ -28,7 +28,6 @@ class team extends base_user
 				TEAMPAGE_TABLE => 't',
 			],
 			'WHERE'     => 'ug.group_id = t.group_id AND ug.user_id = u.user_id AND ug.user_pending = 0
-				AND u.user_id <> ' . ANONYMOUS . '
 				AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . '
 				AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()),
 			'ORDER_BY'  => 'u.user_lastvisit DESC'

phpBB/phpbb/mention/source/topic.php

@@ -31,7 +31,7 @@ class topic extends base_user
 					'ON'   => 'u.user_id = p.poster_id'
 				]
 			],
-			'WHERE'     => 'p.topic_id = ' . $topic_id . ' AND u.user_id <> ' . ANONYMOUS . '
+			'WHERE'     => 'p.topic_id = ' . $topic_id . '
 				AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . '
 				AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()),
 			'ORDER_BY'  => 'p.post_time DESC'

phpBB/phpbb/mention/source/user.php

@@ -25,8 +25,7 @@ class user extends base_user
 			'FROM'      => [
 				USERS_TABLE => 'u',
 			],
-			'WHERE'     => 'u.user_id <> ' . ANONYMOUS . '
+			'WHERE'     => $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . '
-				AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . '
 				AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()),
 			'ORDER_BY'  => 'u.user_lastvisit DESC'
 		]);

phpBB/styles/prosilver/template/posting_buttons.html

@@ -39,7 +39,7 @@
 </div>
 
 <!-- EVENT posting_editor_buttons_before -->
-<div id="format-buttons" class="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-topic-id="{S_TOPIC_ID}"<!-- ENDIF -->>
+<div id="format-buttons" class="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-mention-batch-size="{S_MENTION_BATCH_SIZE}" data-topic-id="{S_TOPIC_ID}"<!-- ENDIF -->>
 	<button type="button" class="button button-icon-only bbcode-b" accesskey="b" name="addbbcode0" value=" B " onclick="bbstyle(0)" title="{L_BBCODE_B_HELP}">
 		{{ Icon('iconify', 'mdi:format-bold', '', true, 'c-button-icon') }}
 	</button>