payment due 1st August LOW: [$500] New feature request: Frequently used emoji is not updated when reacting to messages

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.47-6
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
Expensify/Expensify Issue URL:
Issue reported by: @iwiznia
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1709671306336899

Action Performed:

1. Open any chat
2. React with emoji for a message

Expected Result:

Emoji should be displayed in the frequently used emojis

Actual Result:

Frequently used emoji list is not updated.

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.2822.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a421e19748004629
• Upwork Job ID: 1767189140562845696
• Last Price Increase: 2024-03-11

[melvin-bot]

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

[nkdengineer]

Proposal

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

• Frequently used emoji is not updated when reacting to messages

What is the root cause of that problem?

• When reacting to message in here, we do not call API UpdateFrequentlyUsedEmojis like we did when we type a emoji message

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

• We can update this to:

    User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(emoji));

• We need to apply change in other logics that use emoji as well, like the status page, ...

What alternative solutions did you explore? (Optional)

• Or, when calling the API AddEmojiReaction, BE should return the new frequently used emojis, then we just need to handle in FE side (optimistic data). This is very straightforward, because we can do the same as here

[neonbhai]

Proposal

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

Frequently used emoji is not updated when reacting to messages

What is the root cause of that problem?

We don't update the FrequentlyUsedEmojis when emoji is Selected for messages, but do it for new message and edit message composers separately:

• App/src/pages/home/report/ReportActionItemMessageEdit.tsx

  Line 238 in 042bdcc

  User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(insertedEmojis.current));

• App/src/pages/home/report/ReportActionItemMessageEdit.tsx

  Line 238 in 042bdcc

  User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(insertedEmojis.current));

But we don't have it for when emoji is selected from the popover.

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

We should have the Emoji updating logic in one place, preferably in the Emoji Picker component here:

App/src/components/EmojiPicker/EmojiPicker.js

Line 119 in 042bdcc

const selectEmoji = (emoji, emojiObject) => {

We will be using the emojiObject over the emoji string, since Onyx expects the object

User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(emojiObject));

[zanyrenney]

We discussed in Slack that this could be handled externally. Even though this is the expected behaviour and a new feature from the initial build, I am adding the external label to progress.

[melvin-bot]

Current assignee @zanyrenney is eligible for the NewFeature assigner, not assigning anyone new.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a421e19748004629

[melvin-bot]

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

[zanyrenney]

hey @jayeshmangwani - please can you review the proposals above?

[jayeshmangwani]

Starting reviewing proposals

[jayeshmangwani]

@neonbhai Your RCA seems wrong

But we don't have it for when emoji is selected from the popover.

We call updateFrequentlyUsedEmojis when we select the emoji from the suggestions.

I like the @nkdengineer 's proposal as they suggest to call the updateFrequentlyUsedEmojis when we call the toggleEmojiReaction function

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[iwiznia]

hmmmmm I think this issue needs to be solved in the backend probably, no? In AddEmojiReaction and RemoveEmojiReaction otherwise we would not be following our principle of only doing 1 API request per user interaction...

[melvin-bot]

@jasperhuangg, @jayeshmangwani, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jasperhuangg]

The App PR was deployed to production, and I just took the HOLD off of https://github.com/Expensify/Auth/pull/10919, we should be good for one last review before merging it!

[jasperhuangg]

Just the following PRs left on this, will push them forward now that the most important parts in the Auth PR are deployed.

#42208
https://github.com/Expensify/Web-Expensify/pull/42176

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[jasperhuangg]

Just merged the Web-E PR officially removing the UpdateFrequentlyUsedEmojis command!

[jasperhuangg]

I think we can close this out :)

[jayeshmangwani]

@jasperhuangg Can we please open the issue for handling the payment of reviewed PR?

[jasperhuangg]

Oops, yes sorry @jayeshmangwani. @zanyrenney we need to handle paying them out for the review of that PR. Thanks fo bringing that to my attention!

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: New feature
• [@jayeshmangwani] 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: New feature
• [@jayeshmangwani] 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
• [@jayeshmangwani] Determine if we should create a regression test for this bug. Yes
• [@jayeshmangwani] 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.

Regression Test Proposal

1. Create a new account.
2. Using this account, send a message with a few emojis in any report.
3. Open the emoji picker in the composer.
4. Verify the emojis you used in the message appear in the "Frequently Used" section.
5. Now react to the comment you just made.
6. Open the emoji picker in the composer.
7. Verify the emoji you used to react is sorted to the front of the "Frequently Used" list.

Do we agree 👍 or 👎

[jayeshmangwani]

@zanyrenney, can you please add a payment summary for the issue? The issue is now ready to be closed. The PR hit production on July 17.

[zanyrenney]

Sure thing!

[melvin-bot]

Current assignee @zanyrenney is eligible for the Bug assigner, not assigning anyone new.

[zanyrenney]

payment summary

$500 owed to @jayeshmangwani for this issue.

[jayeshmangwani]

Requested as per #37807 (comment)

[JmillsExpensify]

$500 approved for @jayeshmangwani