Use bulk emoji API for loading recent emojis (#25043)

webapp/channels/src/actions/emoji_actions.js

@@ -3,8 +3,7 @@
 
 import * as EmojiActions from 'mattermost-redux/actions/emojis';
 import {savePreferences} from 'mattermost-redux/actions/preferences';
-import {getCustomEmojisByName} from 'mattermost-redux/selectors/entities/emojis';
-import {getConfig} from 'mattermost-redux/selectors/entities/general';
+import {getCustomEmojisByName as selectCustomEmojisByName, getCustomEmojisEnabled} from 'mattermost-redux/selectors/entities/emojis';
 import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';
 
 import {getEmojiMap, getRecentEmojisData, getRecentEmojisNames, isCustomEmojiEnabled} from 'selectors/emojis';
@@ -15,23 +14,16 @@ import Constants, {ActionTypes, Preferences} from 'utils/constants';
 import {EmojiIndicesByAlias} from 'utils/emoji';
 
 export function loadRecentlyUsedCustomEmojis() {
-    return async (dispatch, getState) => {
+    return (dispatch, getState) => {
         const state = getState();
-        const config = getConfig(state);
 
-        if (config.EnableCustomEmoji !== 'true') {
+        if (!getCustomEmojisEnabled(state)) {
             return {data: true};
         }
 
-        const recentEmojis = getRecentEmojisNames(state);
-        const emojiMap = getEmojiMap(state);
-        const missingEmojis = recentEmojis.filter((name) => !emojiMap.has(name));
+        const recentEmojiNames = getRecentEmojisNames(state);
 
-        missingEmojis.forEach((name) => {
-            dispatch(EmojiActions.getCustomEmojiByName(name));
-        });
-
-        return {data: true};
+        return dispatch(EmojiActions.getCustomEmojisByName(recentEmojiNames));
     };
 }
 
@@ -153,7 +145,7 @@ export function loadCustomEmojisIfNeeded(emojis) {
         }
 
         const systemEmojis = EmojiIndicesByAlias;
-        const customEmojisByName = getCustomEmojisByName(state);
+        const customEmojisByName = selectCustomEmojisByName(state);
         const nonExistentCustomEmoji = state.entities.emojis.nonExistentEmoji;
         const emojisToLoad = [];
 
@@ -180,7 +172,7 @@ export function loadCustomEmojisIfNeeded(emojis) {
             emojisToLoad.push(emoji);
         });
 
-        return dispatch(EmojiActions.getCustomEmojisByName(emojisToLoad));
+        return dispatch(EmojiActions.selectCustomEmojisByName(emojisToLoad));
     };
 }
 

webapp/channels/src/actions/emoji_actions.mock.test.js

@@ -1,75 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-import * as Actions from 'actions/emoji_actions';
-import {getEmojiMap, getRecentEmojisNames} from 'selectors/emojis';
-
-import mockStore from 'tests/test_store';
-
-const initialState = {
-    entities: {
-        general: {
-            config: {EnableCustomEmoji: 'true'},
-        },
-    },
-};
-
-jest.mock('selectors/emojis', () => ({
-    getEmojiMap: jest.fn(),
-    getRecentEmojisNames: jest.fn(),
-}));
-
-jest.mock('mattermost-redux/actions/emojis', () => ({
-    getCustomEmojiByName: (...args) => ({type: 'MOCK_GET_CUSTOM_EMOJI_BY_NAME', args}),
-}));
-
-describe('loadRecentlyUsedCustomEmojis', () => {
-    let testStore;
-    beforeEach(async () => {
-        testStore = await mockStore(initialState);
-    });
-
-    test('Get only emojis missing on the map', async () => {
-        getEmojiMap.mockImplementation(() => {
-            return new Map([['emoji1', {}], ['emoji3', {}], ['emoji4', {}]]);
-        });
-        getRecentEmojisNames.mockImplementation(() => {
-            return ['emoji1', 'emoji2', 'emoji3', 'emoji5'];
-        });
-
-        const expectedActions = [
-            {type: 'MOCK_GET_CUSTOM_EMOJI_BY_NAME', args: ['emoji2']},
-            {type: 'MOCK_GET_CUSTOM_EMOJI_BY_NAME', args: ['emoji5']},
-        ];
-
-        testStore.dispatch(Actions.loadRecentlyUsedCustomEmojis());
-        expect(testStore.getActions()).toEqual(expectedActions);
-    });
-
-    test('Does not get any emojis if none is missing on the map', async () => {
-        getEmojiMap.mockImplementation(() => {
-            return new Map([['emoji1', {}], ['emoji3', {}], ['emoji4', {}]]);
-        });
-        getRecentEmojisNames.mockImplementation(() => {
-            return ['emoji1', 'emoji3'];
-        });
-        const expectedActions = [];
-
-        testStore.dispatch(Actions.loadRecentlyUsedCustomEmojis());
-        expect(testStore.getActions()).toEqual(expectedActions);
-    });
-
-    test('Does not get any emojis if they are not enabled', async () => {
-        testStore = await mockStore({entities: {general: {config: {EnableCustomEmoji: 'false'}}}});
-        getEmojiMap.mockImplementation(() => {
-            return new Map([['emoji1', {}], ['emoji3', {}], ['emoji4', {}]]);
-        });
-        getRecentEmojisNames.mockImplementation(() => {
-            return ['emoji1', 'emoji2', 'emoji3', 'emoji5'];
-        });
-        const expectedActions = [];
-
-        testStore.dispatch(Actions.loadRecentlyUsedCustomEmojis());
-        expect(testStore.getActions()).toEqual(expectedActions);
-    });
-});

webapp/channels/src/actions/emoji_actions_load_recent.test.ts

@@ -0,0 +1,258 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import nock from 'nock';
+import {BATCH} from 'redux-batched-actions';
+
+import type {DeepPartial} from '@mattermost/types/utilities';
+
+import {EmojiTypes} from 'mattermost-redux/action_types';
+import {setSystemEmojis} from 'mattermost-redux/actions/emojis';
+import {Client4} from 'mattermost-redux/client';
+
+import * as Actions from 'actions/emoji_actions';
+
+import mergeObjects from 'packages/mattermost-redux/test/merge_objects';
+import mockStore from 'tests/test_store';
+import {Preferences} from 'utils/constants';
+import {EmojiIndicesByAlias} from 'utils/emoji';
+import {TestHelper} from 'utils/test_helper';
+
+import type {GlobalState} from 'types/store';
+
+Client4.setUrl('http://localhost:8065');
+
+describe('loadRecentlyUsedCustomEmojis', () => {
+    const currentUserId = 'currentUserId';
+
+    const emoji1 = TestHelper.getCustomEmojiMock({name: 'emoji1', id: 'emojiId1'});
+    const emoji2 = TestHelper.getCustomEmojiMock({name: 'emoji2', id: 'emojiId2'});
+    const loadedEmoji = TestHelper.getCustomEmojiMock({name: 'loaded', id: 'loadedId'});
+
+    const initialState: DeepPartial<GlobalState> = {
+        entities: {
+            emojis: {
+                customEmoji: {
+                    [loadedEmoji.id]: loadedEmoji,
+                },
+                nonExistentEmoji: new Set(),
+            },
+            general: {
+                config: {
+                    EnableCustomEmoji: 'true',
+                },
+            },
+            users: {
+                currentUserId,
+            },
+        },
+    };
+
+    setSystemEmojis(new Set(EmojiIndicesByAlias.keys()));
+
+    test('should get requested emojis', async () => {
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: emoji1.name, usageCount: 3},
+                                {name: emoji2.name, usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        const store = mockStore(state);
+
+        nock(Client4.getBaseRoute()).
+            post('/emoji/names', ['emoji1', 'emoji2']).
+            reply(200, [emoji1, emoji2]);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
+                data: [emoji1, emoji2],
+            },
+        ]);
+    });
+
+    test('should not request emojis which are already loaded', async () => {
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: emoji1.name, usageCount: 3},
+                                {name: loadedEmoji.name, usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        const store = mockStore(state);
+
+        nock(Client4.getBaseRoute()).
+            post('/emoji/names', ['emoji1']).
+            reply(200, [emoji1]);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
+                data: [emoji1],
+            },
+        ]);
+    });
+
+    test('should not make a request if all recentl emojis are loaded', async () => {
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: loadedEmoji.name, usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        const store = mockStore(state);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([]);
+    });
+
+    test('should properly store any names of nonexistent emojis', async () => {
+        const fakeEmojiName = 'fake-emoji-name';
+
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: emoji1.name, usageCount: 3},
+                                {name: fakeEmojiName, usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        const store = mockStore(state);
+
+        nock(Client4.getBaseRoute()).
+            post('/emoji/names', ['emoji1', fakeEmojiName]).
+            reply(200, [emoji1]);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: BATCH,
+                meta: {
+                    batch: true,
+                },
+                payload: [
+                    {
+                        type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
+                        data: [emoji1],
+                    },
+                    {
+                        type: EmojiTypes.CUSTOM_EMOJI_DOES_NOT_EXIST,
+                        data: fakeEmojiName,
+                    },
+                ],
+            },
+        ]);
+    });
+
+    test('should not request an emoji that we know does not exist', async () => {
+        const fakeEmojiName = 'fake-emoji-name';
+
+        // Add nonExistentEmoji separately because mergeObjects only works with plain objects
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: emoji1.name, usageCount: 3},
+                                {name: fakeEmojiName, usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        state.entities.emojis.nonExistentEmoji = new Set([fakeEmojiName]);
+        const store = mockStore(state);
+
+        nock(Client4.getBaseRoute()).
+            post('/emoji/names', ['emoji1']).
+            reply(200, [emoji1]);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
+                data: [emoji1],
+            },
+        ]);
+    });
+
+    test('should not request a system emoji', async () => {
+        const state = mergeObjects(initialState, {
+            entities: {
+                preferences: {
+                    myPreferences: TestHelper.getPreferencesMock([
+                        {
+                            category: Preferences.RECENT_EMOJIS,
+                            name: currentUserId,
+                            value: JSON.stringify([
+                                {name: emoji1.name, usageCount: 3},
+                                {name: 'taco', usageCount: 5},
+                            ]),
+                        },
+                    ]),
+                },
+            },
+        });
+        const store = mockStore(state);
+
+        nock(Client4.getBaseRoute()).
+            post('/emoji/names', ['emoji1']).
+            reply(200, [emoji1]);
+
+        await store.dispatch(Actions.loadRecentlyUsedCustomEmojis());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: EmojiTypes.RECEIVED_CUSTOM_EMOJIS,
+                data: [emoji1],
+            },
+        ]);
+    });
+});

webapp/channels/src/packages/mattermost-redux/src/actions/emojis.ts

@@ -5,12 +5,13 @@ import type {AnyAction} from 'redux';
 import {batchActions} from 'redux-batched-actions';
 
 import type {CustomEmoji} from '@mattermost/types/emojis';
+import type {GlobalState} from '@mattermost/types/store';
 
 import {EmojiTypes} from 'mattermost-redux/action_types';
 import {Client4} from 'mattermost-redux/client';
 import {getCustomEmojisByName as selectCustomEmojisByName} from 'mattermost-redux/selectors/entities/emojis';
 import type {GetStateFunc, DispatchFunc, ActionFunc} from 'mattermost-redux/types/actions';
-import {parseNeededCustomEmojisFromText} from 'mattermost-redux/utils/emoji_utils';
+import {parseEmojiNamesFromText} from 'mattermost-redux/utils/emoji_utils';
 
 import {logError} from './errors';
 import {bindClientFunc, forceLogoutIfNecessary} from './helpers';
@@ -73,7 +74,9 @@ export function getCustomEmojiByName(name: string): ActionFunc {
 
 export function getCustomEmojisByName(names: string[]): ActionFunc {
     return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
-        if (!names || names.length === 0) {
+        const neededNames = filterNeededCustomEmojis(getState(), names);
+
+        if (neededNames.length === 0) {
             return {data: true};
         }
 
@@ -82,7 +85,7 @@ export function getCustomEmojisByName(names: string[]): ActionFunc {
 
         const batches = [];
         for (let i = 0; i < names.length; i += batchSize) {
-            batches.push(names.slice(i, i + batchSize));
+            batches.push(neededNames.slice(i, i + batchSize));
         }
 
         let results;
@@ -102,10 +105,10 @@ export function getCustomEmojisByName(names: string[]): ActionFunc {
             data,
         }];
 
-        if (data.length !== names.length) {
+        if (data.length !== neededNames.length) {
             const foundNames = new Set(data.map((emoji) => emoji.name));
 
-            for (const name of names) {
+            for (const name of neededNames) {
                 if (foundNames.has(name)) {
                     continue;
                 }
@@ -121,19 +124,22 @@ export function getCustomEmojisByName(names: string[]): ActionFunc {
     };
 }
 
+function filterNeededCustomEmojis(state: GlobalState, names: string[]) {
+    const nonExistentEmoji = state.entities.emojis.nonExistentEmoji;
+    const customEmojisByName = selectCustomEmojisByName(state);
+
+    return names.filter((name) => {
+        return !systemEmojis.has(name) && !nonExistentEmoji.has(name) && !customEmojisByName.has(name);
+    });
+}
+
 export function getCustomEmojisInText(text: string): ActionFunc {
-    return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
+    return (dispatch: DispatchFunc) => {
         if (!text) {
             return {data: true};
         }
 
-        const state = getState();
-        const nonExistentEmoji = state.entities.emojis.nonExistentEmoji;
-        const customEmojisByName = selectCustomEmojisByName(state);
-
-        const emojisToLoad = parseNeededCustomEmojisFromText(text, systemEmojis, customEmojisByName, nonExistentEmoji);
-
-        return getCustomEmojisByName(Array.from(emojisToLoad))(dispatch, getState);
+        return dispatch(getCustomEmojisByName(parseEmojiNamesFromText(text)));
     };
 }
 

webapp/channels/src/packages/mattermost-redux/src/utils/emoji_utils.test.ts

@@ -6,75 +6,39 @@ import * as EmojiUtils from 'mattermost-redux/utils/emoji_utils';
 import TestHelper from '../../test/test_helper';
 
 describe('EmojiUtils', () => {
-    describe('parseNeededCustomEmojisFromText', () => {
-        it('no emojis', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
+    describe('parseEmojiNamesFromText', () => {
+        test('should return empty array for text without emojis', () => {
+            const actual = EmojiUtils.parseEmojiNamesFromText(
                 'This has no emojis',
-                new Set(),
-                new Map(),
-                new Set(),
             );
-            const expected = new Set([]);
+            const expected: string[] = [];
 
             expect(actual).toEqual(expected);
         });
 
-        it('some emojis', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
+        test('should return anything that looks like an emoji', () => {
+            const actual = EmojiUtils.parseEmojiNamesFromText(
                 ':this: :is_all: :emo-jis: :123:',
-                new Set(),
-                new Map(),
-                new Set(),
             );
-            const expected = new Set(['this', 'is_all', 'emo-jis', '123']);
+            const expected = ['this', 'is_all', 'emo-jis', '123'];
 
             expect(actual).toEqual(expected);
         });
 
-        it('text surrounding emojis', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
+        test('should correctly handle text surrounding emojis', () => {
+            const actual = EmojiUtils.parseEmojiNamesFromText(
                 ':this:/:is_all: (:emo-jis:) surrounding:123:text:456:asdf',
-                new Set(),
-                new Map(),
-                new Set(),
             );
-            const expected = new Set(['this', 'is_all', 'emo-jis', '123', '456']);
+            const expected = ['this', 'is_all', 'emo-jis', '123', '456'];
 
             expect(actual).toEqual(expected);
         });
 
-        it('system emojis', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
-                ':this: :is_all: :emo-jis: :123:',
-                new Set(['this', '123']),
-                new Map(),
-                new Set(),
+        test('should not return duplicates', () => {
+            const actual = EmojiUtils.parseEmojiNamesFromText(
+                ':emoji: :emoji: :emoji: :emoji:',
             );
-            const expected = new Set(['is_all', 'emo-jis']);
-
-            expect(actual).toEqual(expected);
-        });
-
-        it('custom emojis', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
-                ':this: :is_all: :emo-jis: :123:',
-                new Set(),
-                new Map([['is_all', TestHelper.getCustomEmojiMock({name: 'is_all'})], ['emo-jis', TestHelper.getCustomEmojiMock({name: 'emo-jis'})]]),
-                new Set(),
-            );
-            const expected = new Set(['this', '123']);
-
-            expect(actual).toEqual(expected);
-        });
-
-        it('emojis that we\'ve already tried to load', () => {
-            const actual = EmojiUtils.parseNeededCustomEmojisFromText(
-                ':this: :is_all: :emo-jis: :123:',
-                new Set(),
-                new Map(),
-                new Set(['this', 'emo-jis']),
-            );
-            const expected = new Set(['is_all', '123']);
+            const expected = ['emoji'];
 
             expect(actual).toEqual(expected);
         });

webapp/channels/src/packages/mattermost-redux/src/utils/emoji_utils.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import type {Emoji, SystemEmoji, CustomEmoji} from '@mattermost/types/emojis';
+import type {Emoji, SystemEmoji} from '@mattermost/types/emojis';
 
 import {Client4} from 'mattermost-redux/client';
 
@@ -29,9 +29,9 @@ export function getEmojiImageUrl(emoji: Emoji): string {
     return Client4.getEmojiRoute(emoji.id) + '/image';
 }
 
-export function parseNeededCustomEmojisFromText(text: string, systemEmojis: Set<string>, customEmojisByName: Map<string, CustomEmoji>, nonExistentEmoji: Set<string>): Set<string> {
+export function parseEmojiNamesFromText(text: string): string[] {
     if (!text.includes(':')) {
-        return new Set();
+        return [];
     }
 
     const pattern = /:([A-Za-z0-9_-]+):/gi;
@@ -42,23 +42,8 @@ export function parseNeededCustomEmojisFromText(text: string, systemEmojis: Set<
             continue;
         }
 
-        if (systemEmojis.has(match[1])) {
-            // It's a system emoji, go the next match
-            continue;
-        }
-
-        if (nonExistentEmoji.has(match[1])) {
-            // We've previously confirmed this is not a custom emoji
-            continue;
-        }
-
-        if (customEmojisByName.has(match[1])) {
-            // We have the emoji, go to the next match
-            continue;
-        }
-
         customEmojis.add(match[1]);
     }
 
-    return customEmojis;
+    return Array.from(customEmojis);
 }