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

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

Closed
3 of 6 tasks
IuliiaHerets opened this issue Sep 21, 2024 · 24 comments
Closed
3 of 6 tasks
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

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 21, 2024

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: #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:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6610298_1726863372004.Recording__4010.mp4

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
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Sep 21, 2024
Copy link

melvin-bot bot commented Sep 21, 2024

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.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 21, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 21, 2024
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.

Copy link

melvin-bot bot commented Sep 21, 2024

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

@IuliiaHerets
Copy link
Author

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

@luacmartins
Copy link
Contributor

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 luacmartins added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Sep 22, 2024
@luacmartins luacmartins changed the title Search – All messages from pinned chat are displayed in search pinned section [Search v2.2] – All messages from pinned chat are displayed in search pinned section Sep 22, 2024
@luacmartins
Copy link
Contributor

I can take this issue as part of the Search project

@luacmartins luacmartins assigned luacmartins and unassigned nkuoch Sep 22, 2024
@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Sep 23, 2024
@melvin-bot melvin-bot bot changed the title [Search v2.2] – All messages from pinned chat are displayed in search pinned section [$250] [Search v2.2] – All messages from pinned chat are displayed in search pinned section Sep 23, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 23, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 23, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

📣 @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 📖

@luacmartins luacmartins added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 23, 2024
@rayane-djouah
Copy link
Contributor

Looking for proposals!

@abzokhattab
Copy link
Contributor

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
Copy link
Contributor

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

@QichenZhu
Copy link
Contributor

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

@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2024
@rayane-djouah
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2024
@wildan-m
Copy link
Contributor

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

@wildan-m
Copy link
Contributor

wildan-m commented Sep 26, 2024

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
Copy link
Contributor

Proposal Updated

  • adjust solution with latest source code link
  • Add alternative solution

@melvin-bot melvin-bot bot added the Overdue label Sep 30, 2024
Copy link

melvin-bot bot commented Sep 30, 2024

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

@luacmartins
Copy link
Contributor

@rayane-djouah let me know what you think of the updated proposal #49570 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Oct 1, 2024
@rayane-djouah
Copy link
Contributor

@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?

Screen.Recording.2024-10-02.at.3.06.19.PM.mov

@luacmartins
Copy link
Contributor

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

@IuliiaHerets
Copy link
Author

@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)
Recording.4219.mp4

@luacmartins
Copy link
Contributor

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
Copy link
Contributor

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

@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #expense Oct 2, 2024
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
Projects
Status: Done
Development

No branches or pull requests

9 participants