[HOLD for payment 2023-07-14] [$1000] Original message is displayed for few seconds after adding emoji reaction on edited message

[kbecciv]

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 and login with user A
2. Open the app in another device and login with user B
3. From user B, send message to user A
4. From user A, open user B report, hover on latest sent message and click on Add reaction
5. From user B, edit the message
6. Observe in user A, the user B message is displayed edited, from the current open emoji picker for reaction, click on any emoji to add reaction
7. Observe that after adding the reaction, app displays older (message before edit) for few seconds

Expected Result:

App should not display older message i.e. message before edit on adding reaction to edited message

Actual Result:

App displays older message i.e. message before edit on adding reaction to edited message

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.33-3
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

older.edit.message.is.displayed.on.reaction.2.mp4

Recording.946.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687870862782029

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015a60dfce3c07cce2
• Upwork Job ID: 1674267414000873472
• Last Price Increase: 2023-06-29

[melvin-bot]

Triggered auto assignment to @jliexpensify (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

[jeet-dhandha]

Proposal

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

• Problem is user is seeing old message for few seconds when someone edits the message, before the user add's emoji to the message.

What is the root cause of that problem?

• The reportAction given to Report.toggleEmojiReaction function is not updated with the latest message.

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

• We can access the latest message using reportID and reportActionID and pass it to a function ReportActionsUtils.getReportAction(reportID, reportActionID) (which i had to create) and now use this reportAction for further code.

Code for `toggleEmojiReaction`

function toggleEmojiReaction(reportID, reportAction, emoji, paramSkinTone = preferredSkinTone) {
    const latestReportAction = ReportActionsUtils.getReportAction(reportID, reportAction);
    const message = latestReportAction.message[0];
    const reactionObject = message.reactions && _.find(message.reactions, (reaction) => reaction.emoji === emoji.name);
    const skinTone = emoji.types === undefined ? null : paramSkinTone; // only use skin tone if emoji supports it
    if (reactionObject) {
        if (hasAccountIDReacted(currentUserAccountID, reactionObject.users, skinTone)) {
            return removeEmojiReaction(reportID, latestReportAction, emoji, skinTone);
        }
    }
    return addEmojiReaction(reportID, latestReportAction, emoji, skinTone);
}

What alternative solutions did you explore? (Optional)

• N/A

[dummy-1111]

Proposal

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

What is the root cause of that problem?

When user opens the emoji picker from the reaction bar, openEmojiPicker is called

App/src/components/Reactions/MiniQuickEmojiReactions.js

Lines 59 to 71 in d1a743c

	const openEmojiPicker = () => {
	props.onPressOpenPicker();
	EmojiPickerAction.showEmojiPicker(
	props.onEmojiPickerClosed,
	(emojiCode, emojiObject) => {
	props.onEmojiSelected(emojiObject);
	},
	ref.current,
	undefined,
	() => {},
	props.reportAction,
	);
	};

The value of props.onEmojiSelected is defined when openEmojiPicker is called. So reportAction value of the onEmojiSelected function when emoji is selected would have the value of when user opens the emoji picker.

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 60 to 63 in 53e4154

	const onEmojiSelected = (emoji) => {
	Report.toggleEmojiReaction(reportID, reportAction, emoji);
	closeContextMenu();
	};

This is the root cause

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

We can get latest reportAction in toggleEmojiReaction function

App/src/libs/actions/Report.js

Line 1641 in 53e4154

function toggleEmojiReaction(reportID, reportAction, emoji, paramSkinTone = preferredSkinTone) {

    const action = _.get(allReportActions, [reportID, reportAction.reportActionID], reportAction);

The final function would look like

function toggleEmojiReaction(reportID, reportAction, emoji, paramSkinTone = preferredSkinTone) {
    const action = _.get(allReportActions, [reportID, reportAction.reportActionID], reportAction);
    const message = action.message[0];
    const reactionObject = message.reactions && _.find(message.reactions, (reaction) => reaction.emoji === emoji.name);
    const skinTone = emoji.types === undefined ? null : paramSkinTone; // only use skin tone if emoji supports it
    if (reactionObject) {
        if (hasAccountIDReacted(currentUserAccountID, reactionObject.users, skinTone)) {
            return removeEmojiReaction(reportID, action, emoji, skinTone);
        }
    }
    return addEmojiReaction(reportID, action, emoji, skinTone);
}

This works as expected.

Result

21826_mac_chrome.mp4

What alternative solutions did you explore? (Optional)

[jeet-dhandha]

Comment : #21826 (comment)

Check below video for fix if necessary.

Fixed Video

EditText-PickerFixVideo.mp4

[jliexpensify]

Did some Android issues for Sophie, so doing an issue swap as I'm a bit swamped today and working 50%.

[sophiepintoraetz]

Able to reproduce!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015a60dfce3c07cce2

[melvin-bot]

Current assignee @sophiepintoraetz is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[rushatgabhane]

@jeet-dhandha i agree with the root cause and the solution.
C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[neil-marcellini]

@jeet-dhandha i agree with the root cause and the solution.

Sounds good to me too thanks!

[melvin-bot]

📣 @rushatgabhane You have been assigned to this job!
Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @jeet-dhandha You have been assigned to this job!
Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @dhanashree! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

The BZ member will need to manually hire dhanashree for this role Reporter. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[jeet-dhandha]

@neil-marcellini @rushatgabhane - Will create a PR and send here.

[sophiepintoraetz]

Okay so @jeet-dhandha - can you confirm whether your PR introduced a regression? If so, it is correct that is a 50% deduction for you, @rushatgabhane? (So 1500/2 = $750)

Lastly, what is needed to close out this issue?

[jeet-dhandha]

@sophiepintoraetz There’s a pr which needs to be merged before mine so waiting for that.

[sophiepintoraetz]

Okay but to be clear, your PR did not introduce a regression?

[jeet-dhandha]

It introduced on DEV env, basically someone was testing on main branch and had a issue in reacting with emoji in a thread chat’s first message. So i fixed it before it can go to Stagging. But my PR was not merged due to this #17996 PR.

[jeet-dhandha]

So you can somewhat count it as a regression on DEV end.

[jeet-dhandha]

Any idea on when can we merge that PR ?

[sophiepintoraetz]

Okay so we actually can't release payment until the 7 day window has passed. I think if a regression is introduced in production, that would have the penalty applied...so you're lucky! And Rushat will be best placed to say when we can merge.

[dhanashree-sawant]

Hi @sophiepintoraetz, I have accepted the offer, you have also sent invite for that job so not accepting that as offer already accepted.

[melvin-bot]

@neil-marcellini, @rushatgabhane, @sophiepintoraetz, @jeet-dhandha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jeet-dhandha]

Waiting for @stitesExpensify to complete the requested changes on this PR : #17996 and then i will have to make changes accordingly for my PR and then we will be able to complete this. @stitesExpensify if you need any help completing it quicker do tell me. Hoping to hear from you soon.

[dhanashree-sawant]

Hi @sophiepintoraetz, I just got another offer for this issue from @trjExpensify and I understood that it was already paid after accepting it. Not sure what to do about it, kindly suggest.

[dhanashree-sawant]

Most probably offer was for #20423 as @trjExpensify sent me offer with this issue title and had mentioned that offer was sent on that issue.

[trjExpensify]

Yeah, I just fudged the contract title on #20243. My bad! Paid that one out anyhow!

[neil-marcellini]

Not overdue

[jeet-dhandha]

@aimane-chnaif should i take a pull frommain branch on my current branch or should i create a new branch and make changes there?

[aimane-chnaif]

@aimane-chnaif should i take a pull frommain branch on my current branch or should i create a new branch and make changes there?

I think you can pull main and fix conflict as there's not much code changes

[jeet-dhandha]

Thanks will do that.

[sophiepintoraetz]

@jeet-dhandha @aimane-chnaif @rushatgabhane - can we confirm the PR for this issue has merged with no regressions in the last 7 days? I'm not sure why we're talking about creating/editing branches if the PR has merged?

[rushatgabhane]

@sophiepintoraetz i think they're talking about a different issue.

But the bottom line is this -
@jeet-dhandha's PR caused a regression. So no timeline bonus is applicable

@jeet-dhandha needs to be paid $500 (half of base price because no bonus in case of regression)
@rushatgabhane - $500

[sophiepintoraetz]

Got it - thanks. @anmurali or @JmillsExpensify will pay you $500 in NewDot once you've completed the checklist here!

[sophiepintoraetz]

Payment issue to Jeet and contract ended - closing.

[jeet-dhandha]

@sophiepintoraetz wondering does the payment gets 1/3rd when received ?

[sophiepintoraetz]

@jeet-dhandha - see the comment from the c+ here

[JmillsExpensify]

Reviewed details for @rushatgabhane. This is accurate and approved for payment in NewDot.