[HOLD for payment 2023-09-20] [$1000] Web - Emoji - Few emojis does not appear on search in english (works fine in spanish)

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open any report
3. Open emoji picker and search for 'jud'
4. It should display all 3 types of judges i.e. judge , man_judge, woman_judge
5. Similarily search for 'jug', it should display all 3 types of juggling emojis i.e. woman_juggling, juggling_person, man_juggling
6. Change language to spanish and search 'mala', it will display all 3 types of juggling emojis

Expected Result:

App should display all possible emoji types for any search

Actual Result:

App does not display all possible emoji types for few emoji searches like 'jug' for juggling emojis and 'jud' for judges emoji in english (works fine in spanish)

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: @dhanashree-sawant

Reproducible in staging?: Y

Reproducible in production?: Y

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

emoji.search.issue.mov

Recording.5915.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691513077592469

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012d680d6ea285b4fb
• Upwork Job ID: 1693977609738469376
• Last Price Increase: 2023-08-22
• Automatic offers:
• samh-nl | Contributor | 26507497
• Dhanashree-Sawant | Reporter | 26507499

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[alphaboss1104]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Few emojis does not appear on search in english

What is the root cause of that problem?

We didn't add the keywords to here.

For example when the language is english if we search for jug, the woman_juggling, juggling_person, man_juggling emojis should displayed but only juggling_person and amphora are displayed.
The reason is that juggling_person and amphora have the keyword that contains jug.

App/assets/emojis/en.js

Lines 1364 to 1366 in 08dd014

	'🤹': {
	keywords: ['balance', 'juggle', 'multitask', 'skill'],
	},

App/assets/emojis/en.js

Lines 2321 to 2323 in 08dd014

	'🏺': {
	keywords: ['aquarius', 'cooking', 'drink', 'jug', 'tool', 'weapon', 'zodiac'],
	},

But woman_juggling, man_juggling don't have got keyword that contains jug.

App/assets/emojis/en.js

Lines 1367 to 1372 in 08dd014

	'🤹‍♂️': {
	keywords: [],
	},
	'🤹‍♀️': {
	keywords: [],
	},

This is the root cause.

What changes do you think we should make in order to solve the problem?

We Should add the necessary keywords here.

What alternative solutions did you explore? (Optional)

None.

[ShogunFire]

it looks related, this is the reverse issue #23429

[dhanashree-sawant]

Hi @ShogunFire, we don't have any definitive root cause on that issue. Till then calling it dupe is not ideal. Also currently that issue is focused only on flags in spanish and not a universal solution, this issue can help to influence the solution of that issue.

[ShogunFire]

I changed my dupe comment after 2 minutes after seeing that this was a reverse issue, I still think it's valuable to link the other issue.

[samh-nl]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Few emojis does not appear on search in english (works fine in spanish)

What is the root cause of that problem?

For searching emojis, we build a lookup index based on the name of the emoji as well as keywords.

App/src/libs/EmojiTrie.js

Line 26 in 03cf0b1

trie.add(nm, {code: item.code, types: item.types, name: nm, suggestions: []});

If an emoji is named man_juggling, searching for "juggling" does not yield results unless "juggling" is explicitly specified as a keyword. In this case keywords are lacking.

However, the underlying problem is how this lookup index is created, it can only search for queries where the indexed string starts with the query.

What changes do you think we should make in order to solve the problem?

Instead of adding keywords for these specific emojis, I would propose a more general solution:
We should expand the way emojis are indexed.

Example: for man_juggling, we should also add "juggling" to the index for this emoji.

Pseudocode:

const nameParts = name.split('_').slice(1); // Remove the first part because we already index the full name
_.forEach(nameParts, (namePart) => {
  const namePartNode = trie.search(namePart);
  if (!namePartNode) {
    trie.add(namePart, {code: item.code, types: item.types, name, suggestions: []});
  } else {
    trie.update(namePart, {
      ...namePartNode.metaData,
      suggestions: [...namePartNode.metaData.suggestions, {code: item.code, types: item.types, name}],
    });
  }
});

Fixed result

chrome_0Wq2Ik9bp5.mp4

What alternative solutions did you explore? (Optional)

If we want full text search, a variation of my proposal would include indexing all substrings (of the entire name, or first splitting it into words), e.g.:

man_juggling
an_juggling
n_juggling
...

However, this may have an impact on initial load time and resource consumption.

[melvin-bot]

@bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012d680d6ea285b4fb

[melvin-bot]

Triggered auto assignment to @maddylewis (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

[bfitzexpensify]

Agreed that #23429, though it seems similar, is a separate issue

@allroundexperts we have a couple of proposals ready for review here

@maddylewis I'm ooo for a week, can pick this back up when I'm back if it's not resolved by then

[maddylewis]

waiting for @allroundexperts to review the existing proposals

[allroundexperts]

Thanks for your proposal @alphaboss1104. I would agree here that a more general solution would be better here instead of just adding keywords to each name.

I think @samh-nl's proposal is in the correct direction. Let's go with them.

Before proceeding to the PR, I would like to confirm from the assigned internal engineer if we want a full text search or a a search which indexes based on splitting the emoji name by _ character.

I think it should be fine doing a full text search.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Li357, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[allroundexperts]

Before proceeding to the PR, I would like to confirm from the assigned internal engineer if we want a full text search or a a search which indexes based on splitting the emoji name by _ character.

@Li357 Please check this out before assigning!

[samh-nl]

PR is ready for review: #26910

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @samh-nl got assigned: 2023-09-05 13:42:07 Z
• when the PR got merged: 2023-09-11 22:07:13 UTC
• days elapsed: 4

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.68-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• feat: index emoji name parts #26910

If no regressions arise, payment will be issued on 2023-09-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @allroundexperts does not require payment (Eligible for Manual Requests)
• @samh-nl requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allroundexperts] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Payment summary (this job was posted under the previous pricing model)

Reporting: @dhanashree-sawant - $250 to be paid via Upwork - offer sent
Contributor: @samh-nl - $1500 to be paid via Upwork (I'm considering this eligible for the reporting bonus since the PR was delayed by internal engineer review by 3 days, and there were no changes requested or required) - ✅
C+ @allroundexperts to be paid $1500 via manual request

[dhanashree-sawant]

Thanks @bfitzexpensify, I have accepted the offer

[bfitzexpensify]

@allroundexperts reminder to complete the BZ checklist when you get a chance - thanks!

[allroundexperts]

Requested payment. On to the checklist!

[bfitzexpensify]

Thanks!

[bfitzexpensify]

Bump on the checklist @allroundexperts - thank you!

[allroundexperts]

Reviewer Checklist

1. This was a case that we never thought about when we implemented the search trie. As such, there isn't a particular PR to point here.
2. N/A
3. N/A
4. A regression test would be helpful.

Regression Test

1. Open any chat and make sure that the app language is english.
2. Enter :jud in the composer.
3. Verify that man_judge, judge and women_judge show up in the options.

Do we 👍 or 👎 ?

[bfitzexpensify]

Looks good to me! OK, we're all good to close this out now.

[JmillsExpensify]

$1,500 payment approved for @allroundexperts based on BZ summary.