Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.53k stars 2.88k forks source link

Chat - Unread marker moves to first message sent offline when returning online #47481

Open IuliiaHerets opened 2 months ago

IuliiaHerets commented 2 months ago

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


Version Number: 9.0.20.6 Reproducible in staging?: Y Reproducible in production?: N Issue was found when executing this PR https://github.com/Expensify/App/pull/47393 Issue reported by: Applause Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/ and log in
  2. Open any chat
  3. Disable the internet connection
  4. Send a few message
  5. Mark any message unread, but not the first one that was sent
  6. Enable the internet connection

Expected Result:

The unread marker stays above the message that was selected unread

Actual Result:

The unread marker moves up to the first message that was sent while offline

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/52f3572e-d1cc-4016-a8cd-90c8cede3dad

View all open jobs on GitHub

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

IuliiaHerets commented 2 months ago

We think that this bug might be related to #vip-vsb

nkdengineer commented 2 months ago

Proposal

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

The unread marker moves up to the first message that was sent while offline

What is the root cause of that problem?

When we mark as unread an action offline, unreadMarkerTime is assigned as the client's created time of the action. Then when we online, BE returns the db created time of the action that is more than the client's created time. So the unread marker is moved up.

https://github.com/Expensify/App/blob/57ef25c998748332864ae98a43579989d6529c22/src/pages/home/report/ReportActionsList.tsx#L213

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

We should update shouldDisplayNewMarker function to check if we already have an unread marker action we only check the reportActionID is matched with unreadMarkerReportActionID or not

if (!unreadMarkerReportActionID) {
    // the current logic
} else {
    return reportAction.reportActionID === unreadMarkerReportActionID
}

https://github.com/Expensify/App/blob/57ef25c998748332864ae98a43579989d6529c22/src/pages/home/report/ReportActionsList.tsx#L225

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @iwiznia (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 2 months ago

:wave: 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.
iwiznia commented 2 months ago

Some conversation here https://expensify.slack.com/archives/C01GTK53T8Q/p1723735247950989

iwiznia commented 2 months ago

ok, coming from that thread.

MarkAsUnread is "wrong" and is using the client's time instead of passing the action and having the server pick the correct time

So solution is:

iwiznia commented 2 months ago

I'll make the API changes for that eventually, but doubt this will take precedence over other issues for a while...

dylanexpensify commented 2 months ago

Sounds good!

dylanexpensify commented 2 months ago

In view, but lower priority

melvin-bot[bot] commented 2 months ago

@iwiznia @dylanexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

dylanexpensify commented 2 months ago

same as last week

dylanexpensify commented 2 months ago

Same

dylanexpensify commented 1 month ago

Same

dylanexpensify commented 1 month ago

Ongoing

dylanexpensify commented 1 month ago

@iwiznia guessing we still got a while here, yeah?

iwiznia commented 1 month ago

Still not a priority yes

dylanexpensify commented 3 weeks ago

Reassigning as I'm heading on leave next week!

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

dylanexpensify commented 3 weeks ago

TY @johncschuster 🙇

johncschuster commented 3 weeks ago

Still low priority

melvin-bot[bot] commented 2 weeks ago

@iwiznia, @johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

johncschuster commented 2 weeks ago

Looks like this is still a lower priority

johncschuster commented 2 weeks ago

Downgrading to Weekly

johncschuster commented 5 days ago

Still LOW priority