[Tracking] Redesign Unread Markers

[JmillsExpensify]

Problem

We've struggled with on-going issues and regressions related to the reliability of the green "new" line that marks the boundary between new chats and old ones. At the moment we minimally have several issues that are being tackled separately, though the root cause is arguably the same. We're duplicating work, or worse, fixing things in one place to potentially break them in the other.

Solution

Let's devise a holistic plan for the "new" marker that definitely solves the root cause, and just as importantly, ensures that our solution creates stability around a feature that has been unstable so far. Knowing where a conversation ended and where it's starting anew is critical, because after all, we're a chat app.

Current Issues

• Message is not marked as read in LHN when opening same message again #13740
• [Hold][$2000] Unread message shows as bold in the LHN but green line does not appear #15019
• [$1000] "----------New" message removed after two replies. #15064
• [Hold for #15212][$1000] Add FailureData for NewDot MarkAsUnread command so that it can revert values correctly #15266
• Web - Chat - Read messages in open chat get "New" marker after page refresh #15742
• [Hold #15212] Unread message indicator persists on mobile version despite opening and reacting to message #17207
• [PAY HUZAIFA][HOLD ON #15212] [$1000] The 'new messages' floating indicator doesnt go away after clicking it #21288
• [HOLD #15212] Web - Green "New" line indicator does not disappear after marking as read from LHN #21453
• [Hold PR#25935][$1000] Android - Chat - New unread message is marked read when user reads old unread message offline #20959
• [Hold for #15212] Web - New green line appears on receiver side and doesn't disappear after deleting a message if it is flagged #22238

---

DESIGN DOC ➡️

Proposal

Redesign Unread marker(The "New" marker line)

whatsnext: https://expensify.slack.com/archives/CC7NECV4L/p1686749939851689
Pre-design1: https://expensify.slack.com/archives/C01GTK53T8Q/p1678250769318789
Pre-design2: https://expensify.slack.com/archives/C01GTK53T8Q/p1684836449852569

Tasks

• Post Proposal (full Problem/Solution statement) in #whatsnext
• Wait at least one full business day, and until the post has a majority (2/3) of positive reactions (👍)
• Paste Proposal in the space above with a link to the Slack thread
• Email strategy@expensify.com and paste in the Proposal
• Fill out the High-level overview of the problem, Timeline, and Terminology sections of the Design Doc
• Email strategy@expensify.com (continue the same email chain as before) with the link to your Design Doc
• Host a pre-design meeting (example) in an appropriate slack channel to discuss any necessary details in public before filling out the High-level of proposed solution section.
• Fill out the High-level of proposed solution section
• Email stategy@expensify.com again with links to the doc and pre-design conversation in Slack
• Add the DesignDocReview label to get the High-level of proposed solution section reviewed
• Respond to any questions or concerns and bring up blockers in Slack to get a consensus if necessary
• Confirm that the doc has the minimum necessary number of reviews before proceeding
• Host another pre-design meeting in the appropriate slack channel to ask for engineering feedback on the technical solution.
• Fill out the Detailed implementation of the solution and related sections.
• Re-add the DesignDocReview label to this issue
• Respond to any questions or concerns and bring up blockers in Slack to get consensus if necessary
• Confirm that the doc has the minimum necessary number of reviews before proceeding
• Email strategy@expensify.com one last time to let them know the Design Doc is moving into the implementation phase
• Implement the changes
• Send out a follow up email to strategy@expensify.com once everything has been implemented and do a Project Wrap-Up retrospective that provides:
  • Summary of what we accomplished with this project
  • What went well?
  • What could we have done better?
  • What did we learn?

[bondydaa]

Just trying to jot down some notes for us while I was looking into a bit more about the OpenReport API endpoint.

It doesn't appear when we did some refactors that the shouldMarkLastAsRead param was added to really help with the "new message indicator".

The param was added in this PR https://github.com/Expensify/Web-Expensify/pull/34642 which was a part of some refactors for ReconnectReport API and was basically added so we could ensure we didn't mark things as read when reloading report data in ReportActionsView.

The other PR where it was slightly touched was here https://github.com/Expensify/Web-Expensify/pull/34931 which made it so that OpenReport would also create a chat if it didn't exist.

As discussed in this massive thread: https://expensify.slack.com/archives/C01GTK53T8Q/p1676267128690339

There are sort of 2 problems right now with the "new message indicator":

1. Whenever we call OpenReport directly we always are updating the last read NVP for a report so even if there are unread messages we blindly say "now" is the last time it was read and will basically mean all messages have been read.
2. There is some complex logic we use for trying to determine the "last read" report action that requires us to find the reportActionID and pass it down from the ReportActionView down to the individual ReportActionItem component.

I'm not sure solving either of these individually will fix all of these so we should try address both now and get this functionality stable.

I think we should try to come at this problem from this point of view:
If this code didn't exist and we instead said "we need to write logic that ensures we highlight unread messages when the report is loaded" what would the solution be and does that align with what we currently have? (not that I think the current code is wrong I just want to make sure we aren't trying to force the solution to match what the code currently is)

I think the main question I think we should agree on is:
At what point should we mark a report or individual report action as "read"?

For me the more I dwell on it the more I think maybe we shouldn't update the last read when we load the report but after the user has take some action on the report or the component unmounts.

[chiragsalian]

Added #15266 to the list too since the solution gets trickier than expected because of the new marker line.

[JmillsExpensify]

Nice, the more the merrier!

[chiragsalian]

So whats the plan here, are we focusing on this because I'm confused if we should be expanding the logic around the new marker logic (example) or should we pause on other issues until this one is done since the logic can change here?

[JmillsExpensify]

Good question. This issue was primarily meant to help us track all the issues that we're experiencing with the new marker line. I wouldn't pause on anything in particular, we just want to make sure that we don't have duplicates on our hands, and related that by grouping the issues together we can identify the root cause(s) more effectively.

[bondydaa]

was just sync'ing with @puneetlath about this.

I think we should see about getting some of the expert contributors to help us get this done so I'm going to close out the individual issues linked on here so that we don't have it spreads across too many people getting bumped for updates and such and can keep the discussion all here.

[bondydaa]

threw out the idea to the Callstack team here https://expensify.slack.com/archives/C03UK30EA1Z/p1677274105842439

[JmillsExpensify]

@bondydaa What's going to be the process for compensating any issue reporters on the linked issues you closed?

[bondydaa]

oh hmm 🤷 no idea.

First thought is maybe we just pay them now? they are bugs that will be fixed eventually.

I guess the problem could be if people keep reporting the same thing over and over but I guess we just need to be diligent about making sure only new bugs that haven't already been reported get added to this issue?

[JmillsExpensify]

I think you should:

• Open them back up
• Make sure they're on HOLD
• Put a monthly label on them

[JmillsExpensify]

At that point, the most bumps someone could get from Melvin would be twelve times a year. Good news is that this project should last much much less longer than that, as this is a pretty critical part of the app, and it's broken right now. I'd expect us to resolve everything in single digit months.

[bondydaa]

👍 sounds good

[gedu]

Hey, can I take this one?

[gedu]

I keep looking into this, wrote down some use cases and main flows, and looking into best way to handle this so I can write a proposal

[MonilBhavsar]

Prepared a predesign draft. We can discuss and then post in the channel. https://docs.google.com/document/d/1laqFNEmt4P9MminYKv5qGwvlR5rBNTbY1zMnMGImKPg/edit

[MonilBhavsar]

We can close this after this WIP bug report is closed #31714

[MonilBhavsar]

Working on the bug #31714 along with waves

[JmillsExpensify]

What's the latest on this initiative?

[MonilBhavsar]

Still trying to close the last bug #31714 amidst wave issues

[JmillsExpensify]

Same as above. Still trying to close that last bug.

[MonilBhavsar]

Had a chat with Eduardo from callstack team for the bug and we're working on closing it

[MonilBhavsar]

App PR is in review #25771

[JmillsExpensify]

That PR is merged right? At that point are we good to close this tracking issue?

[hayata-suenaga]

The PR has been deployed to production

[MonilBhavsar]

Yes, closing and sending an email! 🚀