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

[HOLD for payment 2021-12-06] [$500] New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat #5807

Closed
isagoico opened this issue Oct 13, 2021 · 21 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Oct 13, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open two browsers.
  2. Login with Account A in browser A.
  3. Login with account B in browser B.
  4. Close app on browser B.
  5. Send message from account A from browser A to account B.
  6. Copy the report url from browser a.
  7. Go to browser B.
  8. Paste the url of the Report you copied.

Expected Result:

New marked and badge should be dismissed if the user pressed on the badge.

Actual Result:

New marked and badge are not dismissed if the user pressed on the badge.

Workaround:

None needed, visual issue.

Platform:

Where is this issue occurring?

  • Web

Version Number: 1.1.7-3

Reproducible in staging?: Yes
Reproducible in production?: Yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

output_file.mp4

Expensify/Expensify Issue URL:

Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1633934312104700

View all open jobs on GitHub

view this job on upwork

@MelvinBot
Copy link

Triggered auto assignment to @chiragsalian (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@isagoico isagoico changed the title New message marker and new message badge are not dismissed when there are new messages when user opens report via URL - Reported by: @parasharrajat New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat Oct 13, 2021
@MelvinBot
Copy link

@chiragsalian Huh... This is 4 days overdue. Who can take care of this?

@chiragsalian
Copy link
Contributor

chiragsalian commented Oct 18, 2021

Cool, I'm able to reproduce the issue. Seems legit. It looks like we're not calling Report_UpdateLastRead once the notification badge is clicked.
Demoting to weekly, removing n6-hold and labeling it as external.

@MelvinBot MelvinBot removed the Overdue label Oct 18, 2021
@chiragsalian chiragsalian added Weekly KSv2 External Added to denote the issue can be worked on by a contributor and removed Daily KSv2 n6-hold labels Oct 18, 2021
@MelvinBot
Copy link

Triggered auto assignment to @jboniface (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Oct 18, 2021
@MelvinBot
Copy link

Whoops! This issue is 2 days overdue. Let's get this updated quick!

@MelvinBot
Copy link

Still overdue 6 days?! Let's take care of this!

@MelvinBot
Copy link

8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@MelvinBot
Copy link

12 days overdue. Walking. Toward. The. Light...

@chiragsalian
Copy link
Contributor

I'm pretty sure I unassigned just myself and not you @jboniface earlier but anyway must've been some mistake, so i'm adding you back.

@jboniface
Copy link

oh yeah, i missed this since it wasn't on my dashboard

@botify botify removed the Daily KSv2 label Nov 2, 2021
@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Nov 2, 2021
@MelvinBot
Copy link

Triggered auto assignment to @Beamanator (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@Beamanator
Copy link
Contributor

Noice, reproduction steps seem clear & straightforward. Ready for proposals

@mallenexpensify
Copy link
Contributor

We are placing all non-critical issues on hold while we're on a company offsite. The hold is expected to be lifted on 11/19 (but could go longer). For any PRs that are submitted before or during the hold, we will add a $250 bonus payment.

@kadiealexander kadiealexander changed the title New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat [$500] New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat Nov 9, 2021
@Beamanator
Copy link
Contributor

Not overdue, waiting for proposals

@MelvinBot MelvinBot removed the Overdue label Nov 12, 2021
@parasharrajat
Copy link
Member

parasharrajat commented Nov 14, 2021

Just getting back to speed. Sorry for the late proposal.

Proposal

  1. I think we just need to change the callback called on MarkerBadge onClick to scrollToListBottom method of the ReportActionsView.js instead of scrollToBottom.
    onClick={scrollToBottom}

    Previously, scrolling the list to the bottom will mark the latest last read but now I think this behavior is recently changed to keep the markasUnread messages after the app is put to the foreground from the background.

So now we need to explicitly mark the message read as well as scroll the list to the bottom which scrollToListBottom does.

scrollToListBottom() {
scrollToBottom();
Report.updateLastReadActionID(this.props.reportID);

@Beamanator
Copy link
Contributor

@parasharrajat that sounds like a great idea, nice and simple too - go ahead and submit a PR when you can 👍

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 16, 2021
@Beamanator
Copy link
Contributor

Not overdue, waiting to get merged into Production

@MelvinBot MelvinBot removed the Overdue label Nov 29, 2021
@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 29, 2021
@botify botify changed the title [$500] New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat [HOLD for payment 2021-12-06] [$500] New message marker and new message badge are not dismissed when user opens report via URL - Reported by: @parasharrajat Nov 29, 2021
@botify
Copy link

botify commented Nov 29, 2021

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.16-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-12-06. 🎊

@jboniface
Copy link

hey @parasharrajat, i invited you to this job, but it looks like you might have missed it on upwork. can you respond to the invite or re-apply, so I can pay you out?

@parasharrajat
Copy link
Member

parasharrajat commented Dec 6, 2021

@jboniface Oops! Sorry, I may have missed it. I didn't notice that URL is in the description. I have applied now.

@jboniface
Copy link

no worries. paid with hold and reporting bonuses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants