Merge branch 'regression-improve-gfm-ac-emoji' into 'master'

Fix regression from !42669

See merge request gitlab-org/gitlab!45118

app/assets/javascripts/emoji/index.js

@@ -66,12 +66,8 @@ export function isEmojiNameValid(name) {
   return validEmojiNames.indexOf(name) >= 0;
 }
 
-export function getValidEmojiUnicodeValues() {
-  return Object.values(emojiMap).map(({ e }) => e);
-}
-
-export function getValidEmojiDescriptions() {
-  return Object.values(emojiMap).map(({ d }) => d);
+export function getAllEmoji() {
+  return emojiMap;
 }
 
 /**
@@ -106,16 +102,43 @@ export function getEmoji(query, fallback = false) {
 }
 
 const searchMatchers = {
-  fuzzy: (value, query) => fuzzaldrinPlus.score(value, query) > 0, // Fuzzy matching compares using a fuzzy matching library
-  contains: (value, query) => value.indexOf(query.toLowerCase()) >= 0, // Contains matching compares by indexOf
-  exact: (value, query) => value === query.toLowerCase(), // Exact matching compares by equality
+  // Fuzzy matching compares using a fuzzy matching library
+  fuzzy: (value, query) => {
+    const score = fuzzaldrinPlus.score(value, query) > 0;
+    return { score, success: score > 0 };
+  },
+  // Contains matching compares by indexOf
+  contains: (value, query) => {
+    const index = value.indexOf(query.toLowerCase());
+    return { index, success: index >= 0 };
+  },
+  // Exact matching compares by equality
+  exact: (value, query) => {
+    return { success: value === query.toLowerCase() };
+  },
 };
 
 const searchPredicates = {
-  name: (matcher, query) => emoji => matcher(emoji.name, query), // Search by name
-  alias: (matcher, query) => emoji => emoji.aliases.some(v => matcher(v, query)), // Search by alias
-  description: (matcher, query) => emoji => matcher(emoji.d, query), // Search by description
-  unicode: (matcher, query) => emoji => emoji.e === query, // Search by unicode value (always exact)
+  // Search by name
+  name: (matcher, query) => emoji => {
+    const m = matcher(emoji.name, query);
+    return [{ ...m, emoji, field: emoji.name }];
+  },
+  // Search by alias
+  alias: (matcher, query) => emoji =>
+    emoji.aliases.map(alias => {
+      const m = matcher(alias, query);
+      return { ...m, emoji, field: alias };
+    }),
+  // Search by description
+  description: (matcher, query) => emoji => {
+    const m = matcher(emoji.d, query);
+    return [{ ...m, emoji, field: emoji.d }];
+  },
+  // Search by unicode value (always exact)
+  unicode: (matcher, query) => emoji => {
+    return [{ emoji, field: emoji.e, success: emoji.e === query }];
+  },
 };
 
 /**
@@ -138,6 +161,8 @@ const searchPredicates = {
  * matching compares using a fuzzy matching library.
  * @param {Boolean} opts.fallback If true, a fallback emoji will be returned if
  * the result set is empty. Defaults to false.
+ * @param {Boolean} opts.raw Returns the raw match data instead of just the
+ * matching emoji.
  * @returns {Object[]} A list of emoji that match the query.
  */
 export function searchEmoji(query, opts) {
@@ -150,6 +175,7 @@ export function searchEmoji(query, opts) {
     fields = ['name', 'alias', 'description', 'unicode'],
     match = 'exact',
     fallback = false,
+    raw = false,
   } = opts || {};
 
   // optimization for an exact match in name and alias
@@ -161,16 +187,22 @@ export function searchEmoji(query, opts) {
   const matcher = searchMatchers[match] || searchMatchers.exact;
   const predicates = fields.map(f => searchPredicates[f](matcher, query));
 
-  const results = Object.values(emojiMap).filter(emoji =>
-    predicates.some(predicate => predicate(emoji)),
-  );
+  const results = Object.values(emojiMap)
+    .flatMap(emoji => predicates.flatMap(predicate => predicate(emoji)))
+    .filter(r => r.success);
 
   // Fallback to question mark for unknown emojis
   if (fallback && results.length === 0) {
+    if (raw) {
+      return [{ emoji: emojiMap.grey_question }];
+    }
     return [emojiMap.grey_question];
   }
 
-  return results;
+  if (raw) {
+    return results;
+  }
+  return results.map(r => r.emoji);
 }
 
 let emojiCategoryMap;

app/assets/javascripts/gfm_auto_complete.js

@@ -181,6 +181,9 @@ class GfmAutoComplete {
   }
 
   setupEmoji($input) {
+    const self = this;
+    const { filter, ...defaults } = this.getDefaultCallbacks();
+
     // Emoji
     $input.atwho({
       at: ':',
@@ -195,13 +198,43 @@ class GfmAutoComplete {
       skipSpecialCharacterTest: true,
       data: GfmAutoComplete.defaultLoadingData,
       callbacks: {
-        ...this.getDefaultCallbacks(),
+        ...defaults,
         matcher(flag, subtext) {
           const regexp = new RegExp(`(?:[^${glRegexp.unicodeLetters}0-9:]|\n|^):([^:]*)$`, 'gi');
           const match = regexp.exec(subtext);
 
           return match && match.length ? match[1] : null;
         },
+        filter(query, items, searchKey) {
+          const filtered = filter.call(this, query, items, searchKey);
+          if (query.length === 0 || GfmAutoComplete.isLoading(items)) {
+            return filtered;
+          }
+
+          // map from value to "<value> is <field> of <emoji>", arranged by emoji
+          const emojis = {};
+          filtered.forEach(({ name: value }) => {
+            self.emojiLookup[value].forEach(({ emoji: { name }, kind }) => {
+              let entry = emojis[name];
+              if (!entry) {
+                entry = {};
+                emojis[name] = entry;
+              }
+              if (!(kind in entry) || value.localeCompare(entry[kind]) < 0) {
+                entry[kind] = value;
+              }
+            });
+          });
+
+          // collate results to list, prefering name > unicode > alias > description
+          const results = [];
+          Object.values(emojis).forEach(({ name, unicode, alias, description }) => {
+            results.push(name || unicode || alias || description);
+          });
+
+          // return to the form atwho wants
+          return results.map(name => ({ name }));
+        },
       },
     });
   }
@@ -637,12 +670,33 @@ class GfmAutoComplete {
   async loadEmojiData($input, at) {
     await Emoji.initEmojiMap();
 
+    // All the emoji
+    const emojis = Emoji.getAllEmoji();
+
+    // Add all of the fields to atwho's database
     this.loadData($input, at, [
-      ...Emoji.getValidEmojiNames(),
-      ...Emoji.getValidEmojiDescriptions(),
-      ...Emoji.getValidEmojiUnicodeValues(),
+      ...Object.keys(emojis), // Names
+      ...Object.values(emojis).flatMap(({ aliases }) => aliases), // Aliases
+      ...Object.values(emojis).map(({ e }) => e), // Unicode values
+      ...Object.values(emojis).map(({ d }) => d), // Descriptions
     ]);
 
+    // Construct a lookup that can correlate a value to "<value> is the <field> of <emoji>"
+    const lookup = {};
+    const add = (key, kind, emoji) => {
+      if (!(key in lookup)) {
+        lookup[key] = [];
+      }
+      lookup[key].push({ kind, emoji });
+    };
+    Object.values(emojis).forEach(emoji => {
+      add(emoji.name, 'name', emoji);
+      add(emoji.d, 'description', emoji);
+      add(emoji.e, 'unicode', emoji);
+      emoji.aliases.forEach(a => add(a, 'alias', emoji));
+    });
+    this.emojiLookup = lookup;
+
     GfmAutoComplete.glEmojiTag = Emoji.glEmojiTag;
   }
 
@@ -711,19 +765,36 @@ GfmAutoComplete.atTypeMap = {
 
 GfmAutoComplete.typesWithBackendFiltering = ['vulnerabilities'];
 
+function findEmoji(name) {
+  return Emoji.searchEmoji(name, { match: 'contains', raw: true }).sort((a, b) => {
+    if (a.index !== b.index) {
+      return a.index - b.index;
+    }
+    return a.field.localeCompare(b.field);
+  });
+}
+
 // Emoji
 GfmAutoComplete.glEmojiTag = null;
 GfmAutoComplete.Emoji = {
   insertTemplateFunction(value) {
-    const { name = value.name } = Emoji.searchEmoji(value.name, { match: 'contains' })[0] || {};
-    return `:${name}:`;
+    const results = findEmoji(value.name);
+    if (results.length) {
+      return `:${results[0].emoji.name}:`;
+    }
+    return `:${value.name}:`;
   },
   templateFunction(name) {
     // glEmojiTag helper is loaded on-demand in fetchData()
     if (!GfmAutoComplete.glEmojiTag) return `<li>${name}</li>`;
 
-    const emoji = Emoji.searchEmoji(name, { match: 'contains' })[0];
-    return `<li>${name} ${GfmAutoComplete.glEmojiTag(emoji?.name || name)}</li>`;
+    const results = findEmoji(name);
+    if (!results.length) {
+      return `<li>${name} ${GfmAutoComplete.glEmojiTag(name)}</li>`;
+    }
+
+    const { field, emoji } = results[0];
+    return `<li>${field} ${GfmAutoComplete.glEmojiTag(emoji.name)}</li>`;
   },
 };
 // Team Members

spec/frontend/gfm_auto_complete_spec.js

@@ -705,12 +705,12 @@ describe('GfmAutoComplete', () => {
   });
 
   describe('emoji', () => {
-    const { atom } = emojiFixtureMap;
+    const { atom, heart, star } = emojiFixtureMap;
     const assertInserted = ({ input, subject, emoji }) =>
       expect(subject).toBe(`:${emoji?.name || input}:`);
-    const assertTemplated = ({ input, subject, emoji }) =>
+    const assertTemplated = ({ input, subject, emoji, field }) =>
       expect(subject.replace(/\s+/g, ' ')).toBe(
-        `<li>${input} <gl-emoji data-name="${emoji?.name || input}"></gl-emoji> </li>`,
+        `<li>${field || input} <gl-emoji data-name="${emoji?.name || input}"></gl-emoji> </li>`,
       );
 
     let mock;
@@ -731,33 +731,85 @@ describe('GfmAutoComplete', () => {
       ${'insertTemplateFunction'} | ${name => ({ name })} | ${assertInserted}
       ${'templateFunction'}       | ${name => name}       | ${assertTemplated}
     `('Emoji.$name', ({ name, inputFormat, assert }) => {
-      const execute = (input, emoji) =>
+      const execute = (accessor, input, emoji) =>
         assert({
           input,
           emoji,
+          field: accessor && accessor(emoji),
           subject: GfmAutoComplete.Emoji[name](inputFormat(input)),
         });
 
       describeEmojiFields('for $field', ({ accessor }) => {
         it('should work with lowercase', () => {
-          execute(accessor(atom), atom);
+          execute(accessor, accessor(atom), atom);
         });
 
         it('should work with uppercase', () => {
-          execute(accessor(atom).toUpperCase(), atom);
+          execute(accessor, accessor(atom).toUpperCase(), atom);
         });
 
         it('should work with partial value', () => {
-          execute(accessor(atom).slice(1), atom);
+          execute(accessor, accessor(atom).slice(1), atom);
         });
       });
 
       it('should work with unicode value', () => {
-        execute(atom.moji, atom);
+        execute(null, atom.moji, atom);
       });
 
       it('should pass through unknown value', () => {
-        execute('foo bar baz');
+        execute(null, 'foo bar baz');
+      });
+    });
+
+    const expectEmojiOrder = (first, second) => {
+      const keys = Object.keys(emojiFixtureMap);
+      const firstIndex = keys.indexOf(first);
+      const secondIndex = keys.indexOf(second);
+      expect(firstIndex).toBeGreaterThanOrEqual(0);
+      expect(secondIndex).toBeGreaterThanOrEqual(0);
+      expect(firstIndex).toBeLessThan(secondIndex);
+    };
+
+    describe('Emoji.insertTemplateFunction', () => {
+      it('should map ":heart" to :heart: [regression]', () => {
+        // the bug mapped heart to black_heart because the latter sorted first
+        expectEmojiOrder('black_heart', 'heart');
+
+        const item = GfmAutoComplete.Emoji.insertTemplateFunction({ name: 'heart' });
+        expect(item).toEqual(`:${heart.name}:`);
+      });
+
+      it('should map ":star" to :star: [regression]', () => {
+        // the bug mapped star to custard because the latter sorted first
+        expectEmojiOrder('custard', 'star');
+
+        const item = GfmAutoComplete.Emoji.insertTemplateFunction({ name: 'star' });
+        expect(item).toEqual(`:${star.name}:`);
+      });
+    });
+
+    describe('Emoji.templateFunction', () => {
+      it('should map ":heart" to ❤ [regression]', () => {
+        // the bug mapped heart to black_heart because the latter sorted first
+        expectEmojiOrder('black_heart', 'heart');
+
+        const item = GfmAutoComplete.Emoji.templateFunction('heart')
+          .replace(/(<gl-emoji)\s+(data-name)/, '$1 $2')
+          .replace(/>\s+|\s+</g, s => s.trim());
+        expect(item).toEqual(
+          `<li>${heart.name}<gl-emoji data-name="${heart.name}"></gl-emoji></li>`,
+        );
+      });
+
+      it('should map ":star" to ⭐ [regression]', () => {
+        // the bug mapped star to custard because the latter sorted first
+        expectEmojiOrder('custard', 'star');
+
+        const item = GfmAutoComplete.Emoji.templateFunction('star')
+          .replace(/(<gl-emoji)\s+(data-name)/, '$1 $2')
+          .replace(/>\s+|\s+</g, s => s.trim());
+        expect(item).toEqual(`<li>${star.name}<gl-emoji data-name="${star.name}"></gl-emoji></li>`);
       });
     });
   });

spec/frontend/helpers/emoji.js

@@ -4,42 +4,64 @@ import { initEmojiMap, EMOJI_VERSION } from '~/emoji';
 
 export const emojiFixtureMap = {
   atom: {
-    name: 'atom',
     moji: '⚛',
     description: 'atom symbol',
     unicodeVersion: '4.1',
     aliases: ['atom_symbol'],
   },
   bomb: {
-    name: 'bomb',
     moji: '💣',
     unicodeVersion: '6.0',
     description: 'bomb',
-    aliases: [],
   },
   construction_worker_tone5: {
-    name: 'construction_worker_tone5',
     moji: '👷🏿',
     unicodeVersion: '8.0',
     description: 'construction worker tone 5',
-    aliases: [],
   },
   five: {
-    name: 'five',
     moji: '5️⃣',
     unicodeVersion: '3.0',
     description: 'keycap digit five',
-    aliases: [],
   },
   grey_question: {
-    name: 'grey_question',
     moji: '❔',
     unicodeVersion: '6.0',
     description: 'white question mark ornament',
-    aliases: [],
+  },
+
+  // used for regression tests
+  // black_heart MUST come before heart
+  // custard MUST come before star
+  black_heart: {
+    moji: '🖤',
+    unicodeVersion: '1.1',
+    description: 'black heart',
+  },
+  heart: {
+    moji: '❤',
+    unicodeVersion: '1.1',
+    description: 'heavy black heart',
+  },
+  custard: {
+    moji: '🍮',
+    unicodeVersion: '6.0',
+    description: 'custard',
+  },
+  star: {
+    moji: '⭐',
+    unicodeVersion: '5.1',
+    description: 'white medium star',
   },
 };
 
+Object.keys(emojiFixtureMap).forEach(k => {
+  emojiFixtureMap[k].name = k;
+  if (!emojiFixtureMap[k].aliases) {
+    emojiFixtureMap[k].aliases = [];
+  }
+});
+
 export async function initEmojiMock() {
   const emojiData = Object.fromEntries(
     Object.values(emojiFixtureMap).map(m => {