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.54k stars 2.89k forks source link

[$250] [Search v2.2] – All messages from pinned chat are displayed in search pinned section #49570

Closed IuliiaHerets closed 1 month ago

IuliiaHerets commented 1 month 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.39-0 Reproducible in staging?: Y Reproducible in production?: N/A - new feature, doesn't exist in prod Issue was found when executing this PR: https://github.com/Expensify/App/pull/49186 Email or phone of affected tester (no customers): gocemate+a2244@gmail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to LHN> Pin any 1:1 chat that have messages from other user
  2. Go to Search> Chat> Pinned
  3. Take a look at chats in this section> Open few messages and take a look at the green line

Expected Result:

Pinned chat should be present in Pinned section and when open a message green line should remain on the first unread message

Actual Result:

All messages between users from pinned chat are displayed in search pinned section. When open few messages green line change it's position

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/bb0a7573-0660-4eac-8eac-b974961ca548

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021838084013513034465
  • Upwork Job ID: 1838084013513034465
  • Last Price Increase: 2024-09-30
  • Automatic offers:
    • rayane-djouah | Contributor | 104082378
Issue OwnerCurrent Issue Owner: @s77rt
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @JmillsExpensify (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.

github-actions[bot] commented 1 month 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.
melvin-bot[bot] commented 1 month ago

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

IuliiaHerets commented 1 month ago

We think that this bug might be related to #wave-control

luacmartins commented 1 month ago

I don't think this is a deploy blocker. All messages showing up is the expected behavior. Only thing that needs to be fixed is the unread marker jumping around.

luacmartins commented 1 month ago

I can take this issue as part of the Search project

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

rayane-djouah commented 1 month ago

Looking for proposals!

abzokhattab commented 1 month ago

Hi team,

I'm a bit confused about this point:

Pinned chat should be present in the Pinned section Actual Result: All messages between users from the pinned chat are displayed in the Pinned section search.

As far as I know, we don't have the ability to pin individual messages in a chat; we can only pin the entire chat/report itself.

Do you mean that when a user pins a chat, only one entry (the pinned chat) should appear in the Pinned section? And when clicking on it, it should simply open the chat thread with all its messages?

luacmartins commented 1 month ago

@abzokhattab as I mentioned here, that part is expected. We should only fix the unread marker jumping around.

QichenZhu commented 1 month ago

Looks like @wildan-m's proposal on another issue can fix the unread maker issue here.

rayane-djouah commented 1 month ago

@wildan-m Are you interested in looking into this issue?

wildan-m commented 1 month ago

@rayane-djouah sure. Thanks for mentioning that @QichenZhu

wildan-m commented 1 month ago

Proposal

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

What is the root cause of that problem?

We are not updating unread marker time when the last unread report action deleted

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

We can update unread marker time when some of the report actions deleted, we can add new condition to indicate some report action deleted add this condition: mostRecentReportActionCreated >= prevMostRecentReportActionCreated

Change this code to:

    const mostRecentReportActionCreated = sortedVisibleReportActions[0]?.created ?? '';
    const prevMostRecentReportActionCreated = usePrevious(mostRecentReportActionCreated);
    useEffect(() => {

        if (unreadMarkerReportActionID) {
            return;
        }

        if (mostRecentReportActionCreated <= unreadMarkerTime && mostRecentReportActionCreated >= prevMostRecentReportActionCreated) {
            return;
        }

        setUnreadMarkerTime(mostRecentReportActionCreated);

What alternative solutions did you explore? (Optional)

To maintain the unread marker visible after repeated search clicks, lastReadTime should not be updated in openReport. A backend modification is needed to introduce a new parameter shouldUpdateLastReadTime set to true for routes other than SCREENS.SEARCH.REPORT_RHP.

Also wrap setUnreadMarkerTime in src/pages/home/report/ReportActionsList.tsx with the REPORT_RHP check


    const [unreadMarkerTime, setUnreadMarkerTime] = useState(report.lastReadTime ?? '');

    const setUnreadMarkerTimeIfNotSearchReport = (lastReadTime: string) => {
        if(route.name === SCREENS.SEARCH.REPORT_RHP)
        {
            return;
        }
        setUnreadMarkerTime(lastReadTime)
    }
wildan-m commented 1 month ago

Proposal Updated

melvin-bot[bot] commented 1 month ago

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

luacmartins commented 1 month ago

@rayane-djouah let me know what you think of the updated proposal https://github.com/Expensify/App/issues/49570#issuecomment-2375568811

rayane-djouah commented 1 month ago

@luacmartins This bug is no longer reproducible because it was caused by PR #48445, which was reverted in PR #49770. I think we can close this issue.

@IuliiaHerets Could you please retest and confirm whether this issue is no longer reproducible?

https://github.com/user-attachments/assets/5e08d3f9-9f3d-4419-8f8b-5fc8bc5bd6af

luacmartins commented 1 month ago

Cool, I agree. Let's close this out once @IuliiaHerets confirms.

IuliiaHerets commented 1 month ago

@luacmartins This bug report contains two issues.

  1. All messages are displayed in Pinned section ( not sure if that's expected, it is still repro)
  2. Green line appears on every open message from pinned chats (this is not repro any more)

https://github.com/user-attachments/assets/15f7d99f-e44b-48c0-bb27-d033ca10cbd8

luacmartins commented 1 month ago

Thanks for confirming!

All messages are displayed in Pinned section ( not sure if that's expected, it is still repro)

Yes, this is expected

luacmartins commented 1 month ago

Cool, seems like the issue here is resolved. Closing.