Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$1000] Dev: Web - After deleting a thread message, the message "You" briefly shows up in the LHN before disappearing #22487

Closed
1 of 6 tasks
kbecciv opened this issue Jul 8, 2023 · 32 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Needs Reproduction Reproducible steps needed

Comments

@kbecciv
Copy link

kbecciv commented Jul 8, 2023

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


Action Performed:

  1. Open a chat
  2. Go to the "Assign task" section
  3. Enter a title and create task
  4. After creating Assign yourself that task
  5. Send a thread message and delete it
  6. Notice that the message "You" appears briefly in the LHN

Expected Result:

The message "You" should not appear in the LHN after deleting a thread message

Actual Result:

After deleting a thread message, the message "You" briefly shows up in the LHN before disappearing

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: Dev v1.3.38-3
Reproducible in staging?: no
Reproducible in production?: no
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

screen-recording-2023-07-08-at-21933-am_7euWaLII.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b93ac0fe26c1d0de
  • Upwork Job ID: 1678417861188857856
  • Last Price Increase: 2023-07-24
@kbecciv kbecciv added DeployBlockerCash This issue or pull request should block deployment Needs Reproduction Reproducible steps needed labels Jul 8, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Jul 8, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2023

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

@ayazhussain79
Copy link
Contributor

Updated Step 4:

  1. Open a chat
  2. Go to the "Assign task" section
  3. Enter a title and create task
  4. After creating Assign yourself that task
  5. Send a thread message and delete it
  6. Notice that the message "You" appears briefly in the LHN

@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2 labels Jul 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2023

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

@hungvu193
Copy link
Contributor

hungvu193 commented Jul 10, 2023

Proposal

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

Dev: Web - After deleting a thread message, the message "You" briefly shows up in the LHN before disappearing

What is the root cause of that problem?

We're rendering the TaskView here and it has actionType "CREATED"

if (props.action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED) {
if (ReportUtils.isTaskReport(props.report)) {
return (
<TaskView
report={props.report}
shouldShowHorizontalRule={!props.isOnlyReportAction}
/>
);
}

After user edited the task, we put the "TASKEDITED" to the optimisticData, this made this condition false, and it will return the lastMessageText instead of "No activity yet".
if (isCreatedAction(lastVisibleAction)) {
return '';
}

So while user deleting the last message after assigning task to someone, it will update lastMessageText inside optimisticData to You.
Screen Shot 2023-07-18 at 23 17 19
And later, when the data from backend return, it will display No activity yet since backend returned empty lastMessageText.
Screen Shot 2023-07-18 at 23 19 29

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

Since we didn't show message for edited task inside Report or TaskReport, we should add more condition here:

    const visibleActions = _.filter(actions, (action) => !isDeletedAction(action) && !isEditedTaskAction(action));

The isEditedTaskAction function will look like this:

/**
 * @param {Object} reportAction
 * @returns {Boolean}
 */
function isEditedTaskAction(reportAction) {
    return lodashGet(reportAction, 'actionName') === CONST.REPORT.ACTIONS.TYPE.TASKEDITED;
}

What alternative solutions did you explore? (Optional)

We should add a check to our editTaskAndNavigate, if our current taskAction type is "CREATED", we should update actionName to "CREATED" instead of "TASKEDITED".

@bfitzexpensify bfitzexpensify added the External Added to denote the issue can be worked on by a contributor label Jul 10, 2023
@melvin-bot melvin-bot bot changed the title Dev: Web - After deleting a thread message, the message "You" briefly shows up in the LHN before disappearing [$1000] Dev: Web - After deleting a thread message, the message "You" briefly shows up in the LHN before disappearing Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Jul 10, 2023
@neil-marcellini
Copy link
Contributor

Not overdue, I'm going to un-assign for now. Please re-assign me when a C+ approves a proposal.

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2023
@neil-marcellini neil-marcellini removed their assignment Jul 10, 2023
@bfitzexpensify
Copy link
Contributor

@aimane-chnaif proposal from @hungvu193 to review in #22487 (comment)

@melvin-bot
Copy link

melvin-bot bot commented Jul 17, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@aimane-chnaif
Copy link
Contributor

@hungvu193 thanks for your proposal but the root cause is not correct.

Awaiting proposals

@hungvu193
Copy link
Contributor

@aimane-chnaif can you please explain it? Please take a look at my evidence
https://github.com/Expensify/App/assets/16502320/88249d27-2083-40d6-8c3d-97fa866d45af

@aimane-chnaif
Copy link
Contributor

@hungvu193 why this bug happens only when delete message? Based on your RCA, "You" will be always shown when TASKEDITED is last action.

@hungvu193
Copy link
Contributor

hungvu193 commented Jul 18, 2023

@hungvu193 why this bug happens only when delete message? Based on your RCA, "You" will be always shown when TASKEDITED is last action.

Aha, I missed to explain the DeleteComment part. So when user deleted a comment, it updated lastMessageText inside optimisticData to You instead of empty string (Since our action was TASKEDITED). After that, data from backend returned and our lastMessageText was updated to No activity yet right before that.

I updated my proposal here (#22487 (comment)) btw.

@melvin-bot melvin-bot bot added the Overdue label Jul 20, 2023
@bfitzexpensify
Copy link
Contributor

@aimane-chnaif take a look at the updated proposal in #22487 (comment) when you get a moment - thanks!

@melvin-bot melvin-bot bot removed the Overdue label Jul 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 22, 2023

@bfitzexpensify @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Jul 24, 2023
@bfitzexpensify
Copy link
Contributor

Bump to review updated proposal @aimane-chnaif - thank you!

@melvin-bot melvin-bot bot removed the Overdue label Jul 24, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 24, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@aimane-chnaif
Copy link
Contributor

Bump to review updated proposal @aimane-chnaif - thank you!

@hungvu193's solution still doesn't work.

Screenshot 2023-07-24 at 5 22 38 PM

@aimane-chnaif
Copy link
Contributor

Waiting for better proposal

@hungvu193
Copy link
Contributor

Bump to review updated proposal @aimane-chnaif - thank you!

@hungvu193's solution still doesn't work.

Screenshot 2023-07-24 at 5 22 38 PM

Thanks for your patience. You're right. I missed this case. I updated my proposal again
#22487 (comment)

@melvin-bot melvin-bot bot added the Overdue label Jul 26, 2023
@bfitzexpensify
Copy link
Contributor

How is #22487 (comment) looking @aimane-chnaif?

@melvin-bot melvin-bot bot removed the Overdue label Jul 27, 2023
@jjcoffee
Copy link
Contributor

Unable to repro on latest main.

@hungvu193
Copy link
Contributor

Yeah, I think it's fixed. We can close

@aimane-chnaif
Copy link
Contributor

@hungvu193 which PR do you think it's fixed by?

@hungvu193
Copy link
Contributor

This one #23341 @aimane-chnaif .
It filtered the last visible action, similar my updated proposal.

@ayazhussain79
Copy link
Contributor

i have reported this bug 3 weeks ago so maybe a PR fixed this bug after i reported so am i eligible for reporting bonus ?

@melvin-bot
Copy link

melvin-bot bot commented Jul 29, 2023

@bfitzexpensify @aimane-chnaif this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@bfitzexpensify
Copy link
Contributor

Great, closing this out. @ayazhussain79 since you reported the other bug that led to this fix, we wouldn't pay out the reporting bonus twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Needs Reproduction Reproducible steps needed
Projects
None yet
Development

No branches or pull requests

8 participants