[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong #27464

kbecciv · 2023-09-14T18:16:36Z

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:

Open any report
Open the emoji picker
Choose an emoji that is not in the frequently used emoji list
Open the emoji picker again and see at what position this emoji is in the frequently used emoji list
Close the emoji picker
Type a lot of characters
Open the emoji picker

Expected Result:

The emoji you chose should be at the same position in step 4 and step 7 because you didn't used it more

Actual Result:

The emoji you used is in the top position of the frequently used emoji

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.70.2
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

2023-09-12.07-31-03.mp4

Recording.4477.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ShogunFire
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694526008852389

View all open jobs on GitHub

melvin-bot · 2023-09-14T18:16:41Z

Job added to Upwork: https://www.upwork.com/jobs/~013dbe1b2bb7790399

melvin-bot · 2023-09-14T18:16:41Z

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

melvin-bot · 2023-09-14T18:16:43Z

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

melvin-bot · 2023-09-14T18:16:46Z

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

melvin-bot · 2023-09-14T18:16:52Z

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

kbecciv · 2023-09-14T18:17:05Z

Proposal by: @ShogunFire
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694526008852389

Proposal

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

The frequently used emojis list is in the wrong order

What is the root cause of that problem?

We are updating the frequently used emojis with the emojis we inserted here:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Lines 134 to 137 in 2163c54

    
           const debouncedUpdateFrequentlyUsedEmojis = useCallback(() => { 
        
               User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(insertedEmojisRef.current)); 
        
               insertedEmojisRef.current = []; 
        
           }, []);

And we are retrieving those emojis in updateComment here:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 182 in 2163c54

insertedEmojisRef.current = [...insertedEmojisRef.current, ...emojis];

The emojis are extracted from the text in replaceAndExtractEmojis
The problem with that method is that we are returning the emojis from the text even when they were already there before so at each character the user type we will add the same emojis which will make us think that we are using the emoji way more than what is actually happening

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

We need to save the emojis that were in the text before in a variable called emojisPresentBefore and the insertedEmojis variable that we use to update the frequently used emoji will be filled with emojis that are in this text minus the emojis from emojisPresentBefore.
The same behaviour is present in ReportActionItemMessageEdit so we need to do the same thing.

What alternative solutions did you explore? (Optional)

DylanDylann · 2023-09-15T08:18:52Z

@ShogunFire It seems your solution will cause the regression, but I am not sure about that. To confirm could you help to detail your solution?

ShogunFire · 2023-09-15T14:41:52Z

Proposal

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

The frequently used emojis list is in the wrong order

What is the root cause of that problem?

We are updating the frequently used emojis with the emojis we inserted here:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Lines 134 to 137 in 2163c54

    
           const debouncedUpdateFrequentlyUsedEmojis = useCallback(() => { 
        
               User.updateFrequentlyUsedEmojis(EmojiUtils.getFrequentlyUsedEmojis(insertedEmojisRef.current)); 
        
               insertedEmojisRef.current = []; 
        
           }, []);

And we are retrieving those emojis in updateComment here:

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 182 in 2163c54

insertedEmojisRef.current = [...insertedEmojisRef.current, ...emojis];

The emojis are extracted from the text in replaceAndExtractEmojis
The problem with that method is that we are returning the emojis from the text even when they were already there before so at each character the user type we will add the same emojis which will make us think that we are using the emoji way more than what is actually happening

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

We need to save the emojis that were in the text before in a variable called emojisPresentBefore and the insertedEmojis variable that we use to update the frequently used emoji will be filled with emojis that are in this text minus the emojis from emojisPresentBefore.

emojisPresentBefore must be filled with the emojis present in the drafts to avoid sending the draft emojis twice:

var emojisPresentBefore = [];
const [value, setValue] = useState(() => {
    const draft = getDraftComment(reportID) || '';
    if (draft) {
        emojisPresentBefore = EmojiUtils.extractEmojis(draft);
    }
    return draft;
});

We can remove the callback onInsertedEmoji because the emojis inserted with suggestion list will be parsed and added in the frequent emojis list in updateComment like the other emojis
In update comment we fill insertedEmojis only with the new emojis like this:

const newEmojis = EmojiUtils.getAddedEmojis(emojis, emojisPresentBefore);

insertedEmojisRef.current = [...insertedEmojisRef.current, ...newEmojis];

And after that we also update emojisPresentBefore in every cases even if there is no emoji

The new method getAddedEmojis looks like this for now: Maybe there is a better way to get the new emojis:

function getAddedEmojis(currentEmojis, formerEmojis){
    
    let newEmojis = [...currentEmojis];
    // We are removing the emojis from the newEmojis array if they were already present before.
    formerEmojis.forEach((formerEmoji) => {
        const indexOfAlreadyPresentEmoji = newEmojis.findIndex(newEmoji => newEmoji.code == formerEmoji.code);
        if(indexOfAlreadyPresentEmoji >= 0) {
            newEmojis.splice(indexOfAlreadyPresentEmoji, 1);
        }
    });
    return newEmojis;
}

Right now extractEmojis method only returns 1 time the emojis even if there are 3 occurences of the same emoji, we need to return the 3 occurences

The same behaviour is present in ReportActionItemMessageEdit so we need to do the same thing.

This is just the primary version but here is my branch that is working fine for ComposerWithSuggestions:
https://github.com/ShogunFire/App/tree/fixFrequentlyUsedEmojiSentMultipleTimes

What alternative solutions did you explore? (Optional)

ShogunFire · 2023-09-15T14:43:27Z

@DylanDylann I couldn't edit the proposal but yes this was a little more complex than I thought, I hope the regression you were speaking about is not present with that version

mananjadhav · 2023-09-17T18:40:07Z

I think @ShogunFire's proposal will work. It looks a bit complex and we can try to simplify during the PR stage. For instance, @ShogunFire will this simplify your getAddedEmojis logic, if I understand it correct?

function getAddedEmojis(currentEmojis, formerEmojis) {
    return currentEmojis.filter((currentEmoji) => {
        return !formerEmojis.some((formerEmoji) => currentEmoji.code === formerEmoji.code);
    });
}

🎀 👀 🎀 C+ reviewed.

melvin-bot · 2023-09-17T18:40:13Z

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

ShogunFire · 2023-09-18T02:35:38Z

@mananjadhav Thanks, I don't think we can do that because if we have "🙂" before and "🙂🙂" after I think we need to update the count of the emoji so getAddedEmojis has to return 1 🙂. I will think of a better way to do that but it looks kind of hard to make it simple.

DylanDylann · 2023-09-18T07:27:43Z

@Pujan92

emojis: emojis.concat(extractEmojis(text)),

This is a feature, we can't remove it
ref: #21786

Pujan92 · 2023-09-18T08:07:03Z

emojis: emojis.concat(extractEmojis(text)),

This is a feature, we can't remove it ref: #21786

Thanks @DylanDylann , I tried but wasn't able to find this ref earlier.

melvin-bot · 2023-10-13T22:49:14Z

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

Only add the new emojis to the frequent emojis list by doing a difference between former and new emojis #27687

If no regressions arise, payment will be issued on 2023-10-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:

@mananjadhav does not require payment (Eligible for Manual Requests)
@ShogunFire requires payment
@ShogunFire requires payment

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 · 2023-10-13T22:49:15Z

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:

[@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
[@mananjadhav] 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:
[@mananjadhav] 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:
[@mananjadhav] Determine if we should create a regression test for this bug.
[@mananjadhav] 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.
[@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2023-10-16T21:11:55Z

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

Only add the new emojis to the frequent emojis list by doing a difference between former and new emojis #27687

If no regressions arise, payment will be issued on 2023-10-23. 🎊

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:

@mananjadhav does not require payment (Eligible for Manual Requests)
@ShogunFire requires payment
@ShogunFire requires payment

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 · 2023-10-16T21:11:56Z

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:

[@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
[@mananjadhav] 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:
[@mananjadhav] 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:
[@mananjadhav] Determine if we should create a regression test for this bug.
[@mananjadhav] 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.
[@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2023-10-16T21:37:11Z

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

Only add the new emojis to the frequent emojis list by doing a difference between former and new emojis #27687

If no regressions arise, payment will be issued on 2023-10-23. 🎊

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:

@mananjadhav does not require payment (Eligible for Manual Requests)
@ShogunFire requires payment
@ShogunFire requires payment

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 · 2023-10-16T21:37:13Z

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:

[@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
[@mananjadhav] 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:
[@mananjadhav] 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:
[@mananjadhav] Determine if we should create a regression test for this bug.
[@mananjadhav] 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.
[@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2023-10-23T12:26:30Z

@mananjadhav, @joelbettner, @ShogunFire, @stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

joelbettner · 2023-10-23T21:01:33Z

I'm so confused...

It took me forever to get to this since I was working on other high-priority, time sensitive issues. But...it looks like the soltion for this has already been merged/deployed?? (cc: @mananjadhav @stephanieelliott @ShogunFire)

stephanieelliott · 2023-10-24T02:11:25Z

Ya Melvin kirked out on this one, it's a single PR that triggered the automation multiple times. Everything is done here though, just needs to be paid out!

Summarizing payment:

Issue reporter: @ShogunFire $50, pay via Upwork
Contributor: @ShogunFire $500, pay via Upwork
Contributor+: @mananjadhav $500, request via NewDot

Upwork job is here, not eligible for bonus

stephanieelliott · 2023-10-24T02:11:53Z

@ShogunFire I extended the offer to you in Upwork, can you please accept when you get a chance? Thanks! https://www.upwork.com/jobs/~01bec5d8c4497434db

ShogunFire · 2023-10-24T17:35:27Z

@stephanieelliott thanks , I did

mananjadhav · 2023-10-24T18:13:03Z

I've raised the request on NewDot.

JmillsExpensify · 2023-10-24T18:36:11Z

$500 payment approved for @mananjadhav based on summary above.

stephanieelliott · 2023-10-25T21:12:01Z

All paid!

ShogunFire · 2024-05-26T11:07:14Z

Hello @stephanieelliott , sorry for the late request but could you please put the upwork mission as complete ? It still shows as in progress on my profile. Thanks

stephanieelliott · 2024-05-30T21:51:37Z

Hey @ShogunFire no problem. I saw your email come through and have I closed this contract as well as the other 2 completed contracts that were still active on your profile (#20998, #19389)

kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 14, 2023

melvin-bot bot changed the title ~~Web - Frequently used emojis is listed wrong~~ [$500] Web - Frequently used emojis is listed wrong Sep 14, 2023

melvin-bot bot assigned joekaufmanexpensify Sep 14, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 14, 2023

melvin-bot bot assigned stephanieelliott Sep 14, 2023

melvin-bot bot assigned mananjadhav Sep 14, 2023

melvin-bot bot assigned joelbettner Sep 17, 2023

mananjadhav mentioned this issue Sep 17, 2023

[$500] Web - Chat - 2 APIs updateFrequentlyUsed emoji is sent #27579

Closed

6 tasks

stephanieelliott assigned ShogunFire Sep 18, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 18, 2023

stephanieelliott unassigned joekaufmanexpensify Sep 18, 2023

DylanDylann mentioned this issue Sep 18, 2023

[HOLD for payment 2023-10-02] [$500] Web - Disable cursor is shown on completed task details #27243

Closed

6 tasks

This comment was marked as outdated.

Sign in to view

ShogunFire mentioned this issue Sep 18, 2023

Only add the new emojis to the frequent emojis list by doing a difference between former and new emojis #27687

Merged

58 tasks

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 13, 2023

melvin-bot bot changed the title ~~[HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong~~ [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong Oct 13, 2023

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 16, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Oct 20, 2023

melvin-bot bot removed the Overdue label Oct 23, 2023

stephanieelliott closed this as completed Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong #27464

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong #27464

kbecciv commented Sep 14, 2023 •

edited

Loading

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

kbecciv commented Sep 14, 2023 •

edited

Loading

DylanDylann commented Sep 15, 2023

ShogunFire commented Sep 15, 2023

ShogunFire commented Sep 15, 2023

mananjadhav commented Sep 17, 2023

melvin-bot bot commented Sep 17, 2023

ShogunFire commented Sep 18, 2023

This comment was marked as outdated.

DylanDylann commented Sep 18, 2023 •

edited

Loading

Pujan92 commented Sep 18, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 23, 2023

joelbettner commented Oct 23, 2023 •

edited

Loading

stephanieelliott commented Oct 24, 2023

stephanieelliott commented Oct 24, 2023

ShogunFire commented Oct 24, 2023

mananjadhav commented Oct 24, 2023

JmillsExpensify commented Oct 24, 2023

stephanieelliott commented Oct 25, 2023

ShogunFire commented May 26, 2024

stephanieelliott commented May 30, 2024

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong #27464

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Web - Frequently used emojis is listed wrong #27464

Comments

kbecciv commented Sep 14, 2023 • edited Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

kbecciv commented Sep 14, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

DylanDylann commented Sep 15, 2023

ShogunFire commented Sep 15, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

ShogunFire commented Sep 15, 2023

mananjadhav commented Sep 17, 2023

melvin-bot bot commented Sep 17, 2023

ShogunFire commented Sep 18, 2023

This comment was marked as outdated.

DylanDylann commented Sep 18, 2023 • edited Loading

Pujan92 commented Sep 18, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 23, 2023

joelbettner commented Oct 23, 2023 • edited Loading

stephanieelliott commented Oct 24, 2023

stephanieelliott commented Oct 24, 2023

ShogunFire commented Oct 24, 2023

mananjadhav commented Oct 24, 2023

JmillsExpensify commented Oct 24, 2023

stephanieelliott commented Oct 25, 2023

ShogunFire commented May 26, 2024

stephanieelliott commented May 30, 2024

kbecciv commented Sep 14, 2023 •

edited

Loading

kbecciv commented Sep 14, 2023 •

edited

Loading

DylanDylann commented Sep 18, 2023 •

edited

Loading

joelbettner commented Oct 23, 2023 •

edited

Loading