[HOLD for payment 2023-04-24] [$1000] Users unable to text if an editing message is deleted

[kavimuru]

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. Log in with the same account on two different devices.
2. On deviceOne go to any report, send a message, click "Edit Comment" on the message.
3. On deviceTwo find the message from step # 2, delete the message.
4. Notice that you can no longer see the Composer on deviceOne. You need to logout and login again to see the Composer.

Expected Result:

If an editing message is deleted, toggleReportActionComposeView should be set to true.

Actual Result:

User on deviceOne can't see the composer if the message they were editing on deviceOne is deleted in the middle of editing from another device. The user on deviceOne needs to logout and login again to see the Composer.

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.2.90-7
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

RPReplay_Final1679984089.MP4

az_recorder_20230328_113250.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c737c02d343368a4
• Upwork Job ID: 1643291430408364032
• Last Price Increase: 2023-04-04

[MelvinBot]

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

[MelvinBot]

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

[sonialiap]

If user A writes a message, user B cannot delete it. I am confused about the steps. Asking for clarification in slack

[sonialiap]

Steps updated

[MelvinBot]

Triggered auto assignment to @cristipaval (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[bernhardoj]

Proposal

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

The main composer does not show when we delete the message that is currently in editing mode from another device.

What is the root cause of that problem?

When we edit a message, we hide the main composer. However, when the message is deleted, we currently don't have a way to reshow the main composer.

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

Show the main composer when the edit message (composer) component unmount.

componentWillUnmount() {
   toggleReportActionComposeView(true, this.props.isSmallScreenWidth);
}

We can further improve this by showing the main composer if the message that is in edit mode is focused (this.state.isFocused).

[bernhardoj]

updated the proposal a little bit.

[mollfpr]

Thanks @bernhardoj, for the proposal!

It seems your solution is working.

We can further improve this by showing the main composer if the message that is in edit mode is focused

Can you elaborate on what the improvement is?

[bernhardoj]

The improvement code would look like this

componentWillUnmount() {
   if (this.state.isFocused) toggleReportActionComposeView(true, this.props.isSmallScreenWidth);
}

Let's say we have 2 message in editing mode,
..... (other message)
Edit Message 1 (focused)
Edit Message 2 <- we delete this from other device

Because we are deleting the edit message that is currently not focused, we don't show the main composer and the edit message 1 still have the focus. Just a small touch.

However, it would behave differently if we switch the condition:
..... (other message)
Edit Message 1 <- we delete this from other device
Edit Message 2 (focused)

When we delete the message 1, both message 1 and message 2 will be unmounted. This behavior also exists on main, could be the normal behavior of FlatList? Which means, edit message 2 will lose it's focus and call the toggleReportActionComposeView that show the main composer.

[mollfpr]

I understand what you mean. It's best to stay focused on deleting messages that are not being deleted. This will leave the hassle for the user to click again on the edit message. What do you think @cristipaval?

[sonialiap]

@cristipaval 👀 🙏 What are your thoughts on the above?

[cristipaval]

Good input here, @bernhardoj !
I agree with @mollfpr suggestion. Let's solve the current problem with the message deletion. The other corner case that you identified, would happen very rarely and it doesn't block the user at all.

[bernhardoj]

Got it, will open the PR soon.

[MelvinBot]

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

[MelvinBot]

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

• show the main composer when a message in editing is deleted #17221

If no regressions arise, payment will be issued on 2023-04-24. 🎊

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

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

[MelvinBot]

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:

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

[cristipaval]

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:

• [@mollfpr / @cristipaval] The PR that introduced the bug has been identified. Link to the PR:
• [@mollfpr / @cristipaval] 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:
• [@mollfpr / @cristipaval] 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:

@mollfpr Could you take care of this?

[mollfpr]

[ ] [@mollfpr / @cristipaval] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@mollfpr / @cristipaval] 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:
[ ] [@mollfpr / @cristipaval] 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:

I don't think this is a bug that causes by any PR. It's an edge case that we miss when implementing the toggleReportActionComposeView. Do you agree with me @cristipaval?

[cristipaval]

yes, I do agree that this is a bug that was not found since the feature was implemented from scratch.

[mollfpr]

@cristipaval Anything else I can help with?

[cristipaval]

Thank you @mollfpr !

@sonialiap is going to perform the payments soon.

[cristipaval]

I guess @sonialiap is ooo as it is labor day in Europe.

[sonialiap]

@hungvu193 job offer sent for reporting issue
@bernhardoj job offer sent for fix (50% for merging within 3 days will be added)
@mollfpr job offer sent for review (50% for merging within 3 days will be added)

[mollfpr]

Thank you @sonialiap, accepted the offer!

[cristipaval]

Are payments done here? Can we close this issue? 👀

[mollfpr]

Waiting for @sonialiap to end the contract on Upwork, then we can close this as completed.

[sonialiap]

@hungvu193 paid
@bernhardoj paid
@mollfpr paid
💰