You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2023-09-18] [$1000] Both English & Spanish Emoji are present while making default language spanish #25735
Case 1:While making English as default language
1.login to app
2.go to settings
3. click on Preferences
4. choose language English
5. go to any chat composer
6. Type :fuego and clear
7. type :fire
8. In this case, you don't see any result while searching with spanish emoji name
Case 2:While making Spanish as default language
1.login to app
2.go to settings
3. click on Preferences
4. choose language Spanish
5. go to any chat composer
6. Type :fuego and clear
7. type :fire
8. In this case, you see result while searching with spanish emoji as well as english emoji
Expected Result:
While making spanish as a default language, english emoji should be hidden
Actual Result:
While making spanish as a default language, both spanish & english emoji should are shown
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: 1.3.56-14 Reproducible in staging?: Yes Reproducible in production?: Yes 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
Please re-state the problem that we are trying to solve in this issue.
When in Spanish language, we get suggestion for English emojis too.
What is the root cause of that problem?
The root cause of the problem is on EmojiTrie.js file.
On line 22, we have: const names = isDefaultLocale ? [name] : [...new Set([name, item.name])];
(@jjcoffee As you can see here, if the isDefaultLocale is false, we are passing a Set on the names, which includes both the preferredLanguage name (Spanish) and the item.name, which is the English one.)
, which adds both the spanish name and the english name on the list.
Basically, we add an English Trie node with the suggestion's name in English, which won't match the equivalent Spanish emoji, so it gets added as a separate match
What changes do you think we should make in order to solve the problem?
We still want users to be able to use the english names of emojis, as they are universally preferred.
In order to achieve that, instead of passing both languages names for each emoji as we do now, if the preferred language is not English, we will pass the English name of the emoji to the tire, suggesting the Spanish emoji.
This way, the english name of the emoji will reference the Spanish one.
An important notice is that when we are in Spanish and search for example :fire, we want :fuego: to appear first. For that reason, we will be adding the suggestion of the english name before all the other suggestions.
In order to achieve that, instead of keeping both names in a variable, we should keep the separately, and add to our trie only the preferred language names:
const englishName = item.name;
const preferredLanguageName = _.get(langEmojis, [item.code, 'name'], englishName);
// Add the actual name of the emoji for the current language
const node = trie.search(preferredLanguageName);
if (!node) {
trie.add(preferredLanguageName, {code: item.code, types: item.types, name: preferredLanguageName, suggestions: []});
} else {
trie.update(preferredLanguageName, {code: item.code, types: item.types, name: preferredLanguageName, suggestions: node.metaData.suggestions});
}
After doing that, we will include the keyword suggestions of both languages in our trie (already doing that).
Finally, after checking that the user is not using English, we will proceed to add the English names as reference to the Spanish ones:
As you can see here: suggestions: [{code: item.code, types: item.types, name: preferredLanguageName}, ...englishNode.metaData.suggestions] , we make sure the english name suggestion comes first!
Another important thing is that we want a valid emoji code to be immediately replaced when the user has set Spanish as the preferred language and types the emoji in English (e.x :fire: should be replaced with the fire emoji if the user is in Spanish).
For that reason, we should add the below code on EmojiUtils.js , at replaceEmojis function, line 322:
let checkEmoji = trie.search(name);
if ((!checkEmoji || !checkEmoji.metaData.code) && lang !== CONST.LOCALES.DEFAULT) {
const englishTrie = emojisTrie[CONST.LOCALES.DEFAULT];
if (englishTrie) {
const englishEmoji = englishTrie.search(name);
checkEmoji = englishEmoji;
}
}
changed the title [-]Both English & Spanish Emoji are present while making default language spanish[/-][+][$1000] Both English & Spanish Emoji are present while making default language spanish[/+]on Aug 23, 2023
@studentofcoding the expected result is that if English is set as the default, then the emojis would be listed in English (Spanish = Spanish), not having both available when one language is set as the preference over the other 👍
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.67-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-09-18. 🎊
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 — @lincolnb17 paid $250 for reporting bug
Contributor that fixed the issue — @Thanos30 paid $1,000 as Contributor
Contributor+ that helped on the issue and/or PR — @jjcoffee paid $1,000 as C+
For reference, here are some details about the assignees on this issue:
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:
[@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
[@jjcoffee] 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:
[@jjcoffee] 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:
[@jjcoffee] Determine if we should create a regression test for this bug.
[@jjcoffee] 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.
[@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
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: feat: support Spanish emojis #20828 (comment)
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: N/A
Determine if we should create a regression test for this bug. No - this would be difficult to test for as it didn't occur for all emoji searches
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. N/A
Activity
melvin-bot commentedon Aug 22, 2023
Triggered auto assignment to @CortneyOfstad (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Aug 22, 2023
Bug0 Triage Checklist (Main S/O)
Platformsin OP are ✅)Thanos30 commentedon Aug 23, 2023
Proposal Updated
Please re-state the problem that we are trying to solve in this issue.
When in Spanish language, we get suggestion for English emojis too.
What is the root cause of that problem?
The root cause of the problem is on
EmojiTrie.jsfile.On line 22, we have:
const names = isDefaultLocale ? [name] : [...new Set([name, item.name])];(@jjcoffee As you can see here, if the
isDefaultLocaleis false, we are passing a Set on the names, which includes both the preferredLanguage name (Spanish) and theitem.name, which is the English one.), which adds both the spanish name and the english name on the list.
Basically, we add an English Trie node with the suggestion's name in English, which won't match the equivalent Spanish emoji, so it gets added as a separate match
What changes do you think we should make in order to solve the problem?
We still want users to be able to use the english names of emojis, as they are universally preferred.
In order to achieve that, instead of passing both languages names for each emoji as we do now, if the preferred language is not English, we will pass the English name of the emoji to the tire, suggesting the Spanish emoji.
This way, the english name of the emoji will reference the Spanish one.
An important notice is that when we are in Spanish and search for example
:fire, we want:fuego:to appear first. For that reason, we will be adding the suggestion of the english name before all the other suggestions.In order to achieve that, instead of keeping both names in a variable, we should keep the separately, and add to our trie only the preferred language names:
After doing that, we will include the keyword suggestions of both languages in our trie (already doing that).
Finally, after checking that the user is not using English, we will proceed to add the English names as reference to the Spanish ones:
As you can see here:
suggestions: [{code: item.code, types: item.types, name: preferredLanguageName}, ...englishNode.metaData.suggestions], we make sure the english name suggestion comes first!Another important thing is that we want a valid emoji code to be immediately replaced when the user has set Spanish as the preferred language and types the emoji in English (e.x
:fire:should be replaced with the fire emoji if the user is in Spanish).For that reason, we should add the below code on
EmojiUtils.js, atreplaceEmojisfunction, line 322:Video of Solution:
Screen.Recording.2023-08-23.at.4.56.35.AM.mov
[-]Both English & Spanish Emoji are present while making default language spanish[/-][+][$1000] Both English & Spanish Emoji are present while making default language spanish[/+]melvin-bot commentedon Aug 23, 2023
Job added to Upwork: https://www.upwork.com/jobs/~0192922d7d2b8409c8
melvin-bot commentedon Aug 23, 2023
Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.
melvin-bot commentedon Aug 23, 2023
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (
External)studentofcoding commentedon Aug 23, 2023
What's the expected result on English language @m-natarajan @CortneyOfstad ? Will it show both English Emoji & Spanish or only English? Thanks
CortneyOfstad commentedon Aug 23, 2023
@studentofcoding the expected result is that if English is set as the default, then the emojis would be listed in English (Spanish = Spanish), not having both available when one language is set as the preference over the other 👍
67 remaining items
melvin-bot commentedon Sep 11, 2023
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Sep 11, 2023
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.67-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-09-18. 🎊
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.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
melvin-bot commentedon Sep 11, 2023
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:
jjcoffee commentedon Sep 14, 2023
CortneyOfstad commentedon Sep 18, 2023
@Thanos30 this is not eligible for the bonus at this time, but standard payment is being made now 👍
CortneyOfstad commentedon Sep 18, 2023
Payments are completed. Closing!
Thanos30 commentedon Sep 18, 2023
Okay @CortneyOfstad , Thank you 🙏