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.36k stars 2.79k forks source link

[$250] LHN - Unread marker from second message when viewing conversation #46501

Closed lanitochka17 closed 1 month ago

lanitochka17 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.14-1 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4784111 Email or phone of affected tester (no customers): applausetester+83pronin@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Navigate to the conversation with the secondary user. Be sure to clear any unread messages if needed
  3. Keep the conversation in view of the most recent message
  4. As the secondary user - Send some messages to the main user

Expected Result:

Verify the unread marker is not displayed in the conversation history over the new messages Verify the conversation is not bold in LHN

Actual Result:

Conversation is displayed bold in LHN starting from second sent message Unread marker is displayed in the conversation history over messages once away and back to conversation

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/7e6fb0ef-d4d5-4b9e-ab04-ef91256a84f5

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b903d59d2cb795a3
  • Upwork Job ID: 1818325193138330554
  • Last Price Increase: 2024-07-30
  • Automatic offers:
    • rayane-djouah | Reviewer | 103403505
    • ishpaul777 | Contributor | 103403507
Issue OwnerCurrent Issue Owner: @rayane-djouah
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @puneetlath (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.
lanitochka17 commented 2 months ago

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

hannojg commented 2 months ago

Just out of curiosity - why is the title in Russian ?

lanitochka17 commented 2 months ago

@hannojg sorry, corrected

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

ishpaul777 commented 2 months ago

Maybe from https://github.com/Expensify/App/pull/45240, verifying by reverting here https://github.com/Expensify/App/pull/46526

Edit: its fixed with revert πŸ‘

nkdengineer commented 2 months ago

Proposal

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

Conversation is displayed bold in LHN starting from second sent message Unread marker is displayed in the conversation history over messages once away and back to conversation

What is the root cause of that problem?

The condition here https://github.com/Expensify/App/blob/3f025eb2649b99b78d16322032f527f9a8033a68/src/pages/home/report/ReportActionsList.tsx#L467 is wrong in the case of scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD. If that is true, we need to check reportAction.created > (userActiveSince.current ?? '') (or >=) instead of reportAction.created < (userActiveSince.current ?? ''), because the former means that the report action is created after the user was active on the report, thus should be shown, and isWithinVisibleThreshold and subsequently shouldDisplay should be true

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

Update https://github.com/Expensify/App/blob/3f025eb2649b99b78d16322032f527f9a8033a68/src/pages/home/report/ReportActionsList.tsx#L467 to

const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created > (userActiveSince.current ?? '') : true;

What alternative solutions did you explore? (Optional)

rayane-djouah commented 2 months ago

Confirmed that it's fixed by the revert πŸ‘

rayane-djouah commented 2 months ago

@nkdengineer - The isWithinVisibleThreshold condition that your proposal is pointing to is not a recent change, see the commit that introduced it

rayane-djouah commented 2 months ago

@hurali97 @mkhutornyi @grgia I've confirmed that this regression is caused by https://github.com/Expensify/App/pull/45240 as suggested here

puneetlath commented 2 months ago

Thanks @ishpaul777. Can you put your PR up for review? And I'll merge it and get it CPd.

ishpaul777 commented 2 months ago

https://github.com/Expensify/App/pull/46526 done!

goldman727 commented 2 months ago

Hello, i read your job post for github issue. that was fixed?

melvin-bot[bot] commented 2 months ago

πŸ“£ @tJonPope727! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
goldman727 commented 2 months ago

Contributor details Your Expensify account email: notmemeapp@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01a5523c1adec76313

melvin-bot[bot] commented 2 months ago

βœ… Contributor details stored successfully. Thank you for contributing to Expensify!

goldman727 commented 2 months ago

you have issues yet, please let me know.

puneetlath commented 2 months ago

Confirmed that the revert/CP fixed it. Removing blocker labels.

ishpaul777 commented 1 month ago

Am i eligible for payment for PR for this https://github.com/Expensify/App/pull/46526

melvin-bot[bot] commented 1 month ago

πŸ“£ @rayane-djouah πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 1 month ago

πŸ“£ @ishpaul777 πŸŽ‰ 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 πŸ“–

ishpaul777 commented 1 month ago

This is ready for payment @puneetlath : )

puneetlath commented 1 month ago

It looks like this is the PR: https://github.com/Expensify/App/pull/45240

It was raised by @ishpaul777 and reviewed by @mkhutornyi, so both are due payment.

Does that sound right?

ishpaul777 commented 1 month ago

No, PR was https://github.com/Expensify/App/pull/46526 and it was reviewed by you

puneetlath commented 1 month ago

Oh I see. Well glad I asked then πŸ˜…

puneetlath commented 1 month ago

Paid. Thanks @ishpaul777!

puneetlath commented 1 month ago

Paid. Thanks @ishpaul777!