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

Chat - New marker displayed briefly when chat is opened #25673

Closed
2 of 6 tasks
lanitochka17 opened this issue Aug 22, 2023 · 36 comments
Closed
2 of 6 tasks

Chat - New marker displayed briefly when chat is opened #25673

lanitochka17 opened this issue Aug 22, 2023 · 36 comments
Assignees
Labels
Engineering Monthly KSv2 Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 22, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #18637

Action Performed:

  1. Log in as user B on the main device and as user A on another device
  2. Open the chat between users B and A on the main device
  3. Send the message as user A to user B from the secondary device

Expected Result:

The marker "New" is not displayed for both users

Actual Result:

The marker "New" is displayed briefly for user B on the main device (android)

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.56-2

Reproducible in staging?: Yes

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

Bug6173006_Video_2023-08-22_00-46-02-1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Aug 22, 2023
@OSBotify
Copy link
Contributor

👋 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 Aug 22, 2023

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

@dukenv0307
Copy link
Contributor

dukenv0307 commented Aug 22, 2023

Proposal

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

Chat - New marker displayed briefly when chat is opened

What is the root cause of that problem?

When user A received a message, this useEffect is triggered because the length of sortedReportActions is changed.
Here we always call readNewestAction API if the report is unread without checking the last action is created by current user or not

if (ReportUtils.isUnread(report)) {
if (scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) {
Report.readNewestAction(report.reportID);
} else {
readActionSkipped.current = true;
}

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

We should check if the last action is not created by current user, we will not call readNewestAction API

if (ReportUtils.isUnread(report)) {
if (scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) {
Report.readNewestAction(report.reportID);
} else {
readActionSkipped.current = true;
}

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Aug 22, 2023
@srikarparsi
Copy link
Contributor

Hi @MonilBhavsar and @allroundexperts, do you think you could take a look at this deploy blocker. It seems to have been caused by this PR here.

@roryabraham
Copy link
Contributor

roryabraham commented Aug 22, 2023

This is an deploy blocker so it supposed to be hourly. Not sure why melvin-bot decided to change that, but he's not supposed to. Anyways, reassigning this one to you @srikarparsi

Edit: Created an issue for this: https://github.com/Expensify/Expensify/issues/310178

@tgolen

This comment was marked as outdated.

@roryabraham
Copy link
Contributor

@melvin-bot
Copy link

melvin-bot bot commented Aug 23, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@roryabraham roryabraham self-assigned this Aug 23, 2023
@roryabraham
Copy link
Contributor

Not seeing enough movement here and it's the middle of the night for @MonilBhavsar so I'm going to jump in and try to help push this forward

@marcaaron
Copy link
Contributor

If the revert also solves this -> #25688 (comment)

Should we just revert?

@roryabraham
Copy link
Contributor

roryabraham commented Aug 23, 2023

I think I seeing that @JmillsExpensify said that the change is needed for wave1?

Edit: found the comment: https://expensify.slack.com/archives/C07J32337/p1692809918214309?thread_ts=1692799992.392829&cid=C07J32337

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2023
@MonilBhavsar
Copy link
Contributor

Bumped to weekly, as happening rarely

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2023
@MonilBhavsar
Copy link
Contributor

Making it daily again as we can reproduce by sending successive messages and focussing on and off the chat screen. Also, we want to close the project soon

@melvin-bot melvin-bot bot removed the Overdue label Sep 13, 2023
@MonilBhavsar MonilBhavsar added Daily KSv2 and removed Weekly KSv2 Needs Reproduction Reproducible steps needed labels Sep 13, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@MonilBhavsar
Copy link
Contributor

@gedu has a got a draft up. I need to review. Been tackling critical issue in parallel.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

@gedu, @allroundexperts, @MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot
Copy link

melvin-bot bot commented Sep 26, 2023

@gedu, @allroundexperts, @MonilBhavsar Still overdue 6 days?! Let's take care of this!

@MonilBhavsar
Copy link
Contributor

PR is being reviewed

@melvin-bot melvin-bot bot removed the Overdue label Sep 27, 2023
@MonilBhavsar MonilBhavsar added the Reviewing Has a PR in review label Sep 27, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

@gedu, @allroundexperts, @MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

@gedu, @allroundexperts, @MonilBhavsar Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2023

@gedu, @allroundexperts, @MonilBhavsar Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@melvin-bot
Copy link

melvin-bot bot commented Oct 12, 2023

@gedu, @allroundexperts, @MonilBhavsar 10 days overdue. Is anyone even seeing these? Hello?

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Oct 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

This issue has not been updated in over 14 days. @gedu, @allroundexperts, @MonilBhavsar eroding to Weekly issue.

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Nov 9, 2023
Copy link

melvin-bot bot commented Nov 9, 2023

This issue has not been updated in over 15 days. @gedu, @allroundexperts, @MonilBhavsar eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@MonilBhavsar
Copy link
Contributor

This is fixed. No action required here. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Monthly KSv2 Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests