Prioritize @ autocomplete results based on recency and thread activity #7506

nixusUM · 2023-08-16T09:00:25Z

Summary

Prioritize @ autocomplete results based on recency and thread activity

Ticket Link

mattermost/mattermost#21235

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.

Device Information

This PR was tested on: TECNO P8 Android, Samsung Galaxy S7 Edge Android, iPhone 11 Pro

Screenshots

Release Note

Changed prioritize @ autocomplete results based on recency and thread activity

mattermost-build · 2023-08-16T09:00:30Z

Hello @nixusUM,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

enahum · 2023-08-17T12:54:00Z

app/components/autocomplete/at_mention/at_mention.tsx

 import {debounce} from 'lodash';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {Platform, SectionList, type SectionListData, type SectionListRenderItemInfo, type StyleProp, type ViewStyle} from 'react-native';

 import {searchGroupsByName, searchGroupsByNameInChannel, searchGroupsByNameInTeam} from '@actions/local/group';
 import {searchUsers} from '@actions/remote/user';
+import {General} from '@app/constants';


Suggested change

import {General} from '@app/constants';

import {General} from '@constants';

please update the import here

enahum · 2023-08-17T12:54:40Z

app/components/autocomplete/at_mention/at_mention.tsx

@@ -42,7 +51,7 @@ const getMatchTermForAtMention = (() => {
    return (value: string, isSearch: boolean) => {
        if (value !== lastValue || isSearch !== lastIsSearch) {
            const regex = isSearch ? AT_MENTION_SEARCH_REGEX : AT_MENTION_REGEX;
-            let term = value.toLowerCase();
+            let term = value;


why this change? this was changed recently and I on pupose

app/components/autocomplete/at_mention/at_mention.tsx

enahum · 2023-08-17T12:56:23Z

app/components/autocomplete/at_mention/at_mention.tsx

+    const database = DatabaseManager.serverDatabases[serverUrl]?.database;
+    if (!database) {
+        return [];
+    }
+
+    return database.get<UserModel>(USER).query(
+        Q.unsafeSqlQuery(`SELECT DISTINCT u.* FROM User u
+        INNER JOIN ${CHANNEL_MEMBERSHIP} cm ON cm.user_id=u.id
+        LEFT JOIN ${CHANNEL} c ON c.id=cm.id AND c.type='${General.DM_CHANNEL}'
+        LEFT JOIN ${MY_CHANNEL} my
+        WHERE cm.user_id IN (${`'${memberIds.join("','")}'`})
+        ORDER BY my.last_viewed_at DESC`)).fetch();
+};


wrap in a try / catch

enahum · 2023-08-17T13:53:09Z

app/components/autocomplete/at_mention/at_mention.tsx

+        LEFT JOIN ${CHANNEL} c ON c.id=cm.id AND c.type='${General.DM_CHANNEL}'
+        LEFT JOIN ${MY_CHANNEL} my


Why I left join? I expect that a DM channel has a Channel and MyChannel relation met. So this could be an inner join that should improve the query plan.

enahum · 2023-08-17T13:56:59Z

app/components/autocomplete/at_mention/at_mention.tsx

+            const sortedMembersId = sortedMembers.map((e) => e.id);
+            const membersNoDm = receivedUsers.users.filter((u) => !sortedMembersId.includes(u.id));


you could should make sortedMembersId a Set so that the filter is more performant

enahum · 2023-08-17T13:57:55Z

app/components/autocomplete/at_mention/at_mention.tsx


+            const slicedArray = sortedMembers.slice(0, 20);


why the slice here before filtering the result?

…utocomplete-resutls

enahum

make sure that after you change the imports that they are sorted correctly

enahum · 2023-08-18T13:03:39Z

app/components/autocomplete/at_mention/at_mention.tsx

 import {debounce} from 'lodash';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {Platform, SectionList, type SectionListData, type SectionListRenderItemInfo, type StyleProp, type ViewStyle} from 'react-native';

 import {searchGroupsByName, searchGroupsByNameInChannel, searchGroupsByNameInTeam} from '@actions/local/group';
 import {searchUsers} from '@actions/remote/user';
+import {General} from '@app/constants';


please update the import here

enahum · 2023-08-18T13:03:56Z

app/components/autocomplete/at_mention/at_mention.tsx

 import {debounce} from 'lodash';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {Platform, SectionList, type SectionListData, type SectionListRenderItemInfo, type StyleProp, type ViewStyle} from 'react-native';

 import {searchGroupsByName, searchGroupsByNameInChannel, searchGroupsByNameInTeam} from '@actions/local/group';
 import {searchUsers} from '@actions/remote/user';
+import {General} from '@app/constants';
+import {logError} from '@app/utils/log';


Suggested change

import {logError} from '@app/utils/log';

import {logError} from '@utils/log';

please update the import here

enahum · 2023-08-18T13:04:59Z

app/components/autocomplete/at_mention/at_mention.tsx

+const getUsersFromDMSorted = async (serverUrl: string, memberIds: string[]) => {
+    try {
+        const database = DatabaseManager.serverDatabases[serverUrl]?.database;
+        if (!database) {
+            return emptyUserlList;
+        }
+
+        return database.get<UserModel>(USER).query(
+            Q.unsafeSqlQuery(`SELECT DISTINCT u.* FROM User u
+            INNER JOIN ${CHANNEL_MEMBERSHIP} cm ON cm.user_id=u.id
+            INNER JOIN ${CHANNEL} c ON c.id=cm.id AND c.type='${General.DM_CHANNEL}'
+            INNER JOIN ${MY_CHANNEL} my
+            WHERE cm.user_id IN (${`'${memberIds.join("','")}'`})
+            ORDER BY my.last_viewed_at DESC`)).fetch();
+    } catch (error) {
+        logError('Failed sort members', error);
+        return emptyUserlList;
+    }
+};


this function should be moved to the queries folder

larkox

Apart of what enahum already said, a few minor comments.

larkox · 2023-08-21T08:44:02Z

app/components/autocomplete/at_mention/at_mention.tsx

    const teamMembers = useMemo(
-        () => [...usersInChannel, ...usersOutOfChannel],
-        [usersInChannel, usersOutOfChannel],
+        () => [...users],
+        [users],
    );


This is doing nothing, isn't? Shouldn't we be using directly users?

larkox · 2023-08-21T08:45:26Z

assets/base/i18n/en.json

-  "settings.about.app.version.title": "App Version:",
-  "settings.about.app.version.value": "{version} (Build {number})",
-  "settings.about.button.copyInfo": "Copy info",
+  "settings.about.build": "{version} (Build {number})",


These changes seem unrelated.

is fine, this is automatically generated.

assets/base/i18n/en.json

nixusUM · 2023-08-30T08:23:50Z

@enahum Hi, any update?

enahum · 2023-08-30T11:57:43Z

@nixusUM please fix the lint problems

nixusUM · 2023-09-04T08:22:19Z

@enahum Hi, I fixed lint problems

mattermost-build · 2023-09-15T01:02:24Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

enahum · 2024-04-30T23:16:03Z

@larkox what else is needed for us to merge this PR?

larkox · 2024-05-03T08:49:04Z

@enahum Nothing from my side I think. We are still missing @esethna review (I guess mostly to verify this is the expected behavior) and QA review. I will start the QA review process while we wait for the other review.

esethna

Exciting to see this get in, thanks @nixusUM! I'll defer to @yasserfaraazkhan for functionality check, please loop me in as needed if there's concerns.

mattermost#7506) * changed autocomplete results on recency * fixed issues * fixed imports, moved get fucn to queries folder * fixed en.json file * fixed lint promblems --------- Co-authored-by: Ilia Polozov <[email protected]>

changed autocomplete results on recency

5946720

mm-cloud-bot added the release-note label Aug 16, 2023

mattermost-build added the Contributor label Aug 16, 2023

nixusUM marked this pull request as ready for review August 16, 2023 16:38

larkox requested a review from enahum August 17, 2023 12:08

enahum requested changes Aug 17, 2023

View reviewed changes

androidonza added 2 commits August 18, 2023 13:25

Merge remote-tracking branch 'origin/main' into st-MM-46224-changed-a…

264f2aa

…utocomplete-resutls

fixed issues

284e1ae

enahum requested changes Aug 18, 2023

View reviewed changes

enahum requested a review from larkox August 18, 2023 13:07

enahum added 2: Dev Review Requires review by a core commiter 2: PM Review Requires review by a product manager 3: QA Review Requires review by a QA tester labels Aug 18, 2023

larkox requested changes Aug 21, 2023

View reviewed changes

fixed imports, moved get fucn to queries folder

77162ca

nixusUM requested review from larkox and enahum August 23, 2023 08:19

larkox reviewed Aug 23, 2023

View reviewed changes

assets/base/i18n/en.json Show resolved Hide resolved

fixed en.json file

9e1d887

nixusUM requested a review from larkox August 25, 2023 06:03

larkox approved these changes Aug 25, 2023

View reviewed changes

enahum approved these changes Aug 30, 2023

View reviewed changes

enahum removed the 2: Dev Review Requires review by a core commiter label Aug 30, 2023

fixed lint promblems

3f2e9f7

larkox requested a review from esethna August 31, 2023 08:22

enahum approved these changes Sep 4, 2023

View reviewed changes

mattermost-build added the Lifecycle/1:stale label Sep 15, 2023

larkox requested a review from yasserfaraazkhan May 3, 2024 08:49

esethna removed the 2: PM Review Requires review by a product manager label May 3, 2024

esethna approved these changes May 3, 2024

View reviewed changes

yasserfaraazkhan approved these changes May 5, 2024

View reviewed changes

larkox added 4: Reviews Complete All reviewers have approved the pull request and removed Lifecycle/1:stale 3: QA Review Requires review by a QA tester labels May 6, 2024

larkox merged commit 105073c into mattermost:main May 6, 2024
2 checks passed

amyblais added this to the v2.17.0 milestone May 6, 2024

This was referenced May 30, 2024

Fix user autocomplete not always showing on search screen #7977

Merged

Fix user autocomplete not showing users not in channel #7980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prioritize @ autocomplete results based on recency and thread activity #7506

Prioritize @ autocomplete results based on recency and thread activity #7506

nixusUM commented Aug 16, 2023 •

edited

Loading

mattermost-build commented Aug 16, 2023

enahum Aug 17, 2023

enahum Aug 18, 2023

enahum Aug 17, 2023

enahum Aug 17, 2023

enahum Aug 17, 2023

enahum Aug 17, 2023

enahum Aug 17, 2023

enahum left a comment

enahum Aug 18, 2023

enahum Aug 18, 2023

enahum Aug 18, 2023

larkox left a comment

larkox Aug 21, 2023

larkox Aug 21, 2023

enahum Aug 23, 2023

nixusUM commented Aug 30, 2023

enahum commented Aug 30, 2023

nixusUM commented Sep 4, 2023

mattermost-build commented Sep 15, 2023

enahum commented Apr 30, 2024

larkox commented May 3, 2024

esethna left a comment

	import {General} from '@app/constants';
	import {General} from '@constants';

		LEFT JOIN ${CHANNEL} c ON c.id=cm.id AND c.type='${General.DM_CHANNEL}'
		LEFT JOIN ${MY_CHANNEL} my

		const sortedMembersId = sortedMembers.map((e) => e.id);
		const membersNoDm = receivedUsers.users.filter((u) => !sortedMembersId.includes(u.id));

	import {logError} from '@app/utils/log';
	import {logError} from '@utils/log';

Prioritize @ autocomplete results based on recency and thread activity #7506

Prioritize @ autocomplete results based on recency and thread activity #7506

Conversation

nixusUM commented Aug 16, 2023 • edited Loading

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

mattermost-build commented Aug 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enahum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nixusUM commented Aug 30, 2023

enahum commented Aug 30, 2023

nixusUM commented Sep 4, 2023

mattermost-build commented Sep 15, 2023

enahum commented Apr 30, 2024

larkox commented May 3, 2024

esethna left a comment

Choose a reason for hiding this comment

nixusUM commented Aug 16, 2023 •

edited

Loading