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

[$500] Conversation - Green line does not appear when chat is open #32506

Closed
6 tasks done
lanitochka17 opened this issue Dec 5, 2023 · 16 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

@lanitochka17
Copy link

lanitochka17 commented Dec 5, 2023

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: 1.4.8-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Log into Expensify on the main testing device as User A
  3. On another device, log in to another account as User B
  4. User A open conversation with User B
  5. User B send some messages to User A

Expected Result:

Green line should appear since new messages was sent

Actual Result:

Green line does not appear when chat is open

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6302181_1701800057293.Recording__1476.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0154028f533f92c290
  • Upwork Job ID: 1732106247324459008
  • Last Price Increase: 2023-12-12
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 5, 2023
Copy link

melvin-bot bot commented Dec 5, 2023

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

@melvin-bot melvin-bot bot changed the title Conversation - Green line does not appear when chat is open [$500] Conversation - Green line does not appear when chat is open Dec 5, 2023
Copy link

melvin-bot bot commented Dec 5, 2023

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 5, 2023
Copy link

melvin-bot bot commented Dec 5, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Dec 5, 2023

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

@paultsimura
Copy link
Contributor

paultsimura commented Dec 5, 2023

This looks like a regression from #27456 – the issue is within these lines:

const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
shouldDisplay = reportAction.actorAccountID !== Report.getCurrentUserAccountID() && isWithinVisibleThreshold;

@aimane-chnaif
Copy link
Contributor

@paultsimura you're so easy to say "regression". It's the purpose of that PR.
I think what we should fix here is that LHN is bolded

@paultsimura
Copy link
Contributor

You're so easy to say "regression"

My logic is the following:

  • There is a clearly described expected behavior in the issue.
  • The expected behavior is achieved if the linked changes are reverted.

Therefore, I assume that the offending PR is breaking the expected behavior and therefore it's a regression.

Is this logic incorrect, @aimane-chnaif?

@aimane-chnaif
Copy link
Contributor

It's technically correct.
I think we should update test rail for this. Expected Result and Actual Result are reversed

@paultsimura
Copy link
Contributor

Expected Result and Actual Result are reversed

Then this is a different talk. I usually tend to trust the Issue author with the expected behavior to be correct, therefore am "easy to say "regression".
Thank you for clarifying 🙂

@tienifr
Copy link
Contributor

tienifr commented Dec 6, 2023

Proposal

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

Conversation - Green line does not appear when chat is open

What is the root cause of that problem?

This PR makes the following changes:

const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
shouldDisplay = reportAction.actorAccountID !== Report.getCurrentUserAccountID() && isWithinVisibleThreshold;

We won't show the unread message indicator if reportAction.created >= userActiveSince.current, so the new message (that has the reportAction.created > userActiveSince.current) won't have the unread indicator

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

It's the expected behavior because in this PR, the author said:

  • From user B, send multiple messages to User A
  • Verify that the green line is not shown for these new messages (User A)

But if we want to change the expectation we can update the condition reportAction.created < userActiveSince.current to reportAction.created > userActiveSince.current

@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2023
@aimane-chnaif
Copy link
Contributor

Need to clarify the expected behavior

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 8, 2023
Copy link

melvin-bot bot commented Dec 12, 2023

@anmurali, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Dec 12, 2023

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

@anmurali
Copy link

@aimane-chnaif can we bring this chat to #expensify-bugs and clarify the expected behavior so we can either fix or close this issue?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 13, 2023
@aimane-chnaif
Copy link
Contributor

I think this is expected. This was created after #27456 is done so I assume TestRail is not updated accordingly?
cc: @MonilBhavsar

@melvin-bot melvin-bot bot removed the Overdue label Dec 18, 2023
@MonilBhavsar
Copy link
Contributor

Thanks for the ping. Yes, this is expected behavior. Since user A is active on a chat with user B, new marker should not appear. we can close this. I am going to create a master issue to update QA tests

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 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
None yet
Development

No branches or pull requests

6 participants