-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$1000] No new marker if tab opened via link to the specific chat #12598
Comments
Triggered auto assignment to @dylanexpensify ( |
cc @marcaaron curious for your eyes on this given this PR |
@dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
reviewing today! |
reviewing today mel! |
Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane ( |
Triggered auto assignment to @MonilBhavsar ( |
@dylanexpensify Let's give @MonilBhavsar and @sobitneupane a chance to look into this. |
I'm able to reproduce the issue. Looking into it |
Upon investigation, looks like I'm not receiving Pusher event at all when report is not visible and there are some Pusher related errors in the log. I'll sort it out and update here |
Thanks @MonilBhavsar! |
@MonilBhavsar this is one of the oldest issues in the /App repo. To help us clear out the large backlog of bugs, can you:
|
@sobitneupane, @MonilBhavsar, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
reviewing today! |
Current assignee @MonilBhavsar is eligible for the External assigner, not assigning anyone new. |
Marked external again so that we can get some new ideas/proposals on how to solve it. |
Upwork job price has been updated to $1000 |
@JmillsExpensify @MelvinBot Question What the new marker set should look like ? answer would helps to understand the issue. |
Proposal - We can solve this problem using timestamp which is present in every message.. we can use pooling to maintain the last timestamp when the tab was active.. and it has a beauty that it is always in increasing order at the time of creating the list we can check if the last timestamp is less than the message timestamp, if found we can show a message something like let me know if you found this solution helpfull for proposal Thanks |
@Ashwanikhurana Thanks for your proposal. Can you please repost the proposal using the proposal template? You can follow our guideline to know how you are supposed to propose a solution. |
ProposalPlease re-state the problem that we are trying to solve in this issue.No New Marker when User A send message to User B if User B is not active(not logged in or open app using URL). What is the root cause of that problem?I have forked and downloaded the APP local and traced the code and found
Above logic only compares ID for newMaker to appear. e.g(functionality work when right click and mark message as unread). What changes do you think we should make in order to solve the problem?add property isNewMessage : true with each message user sends. componentDidMount or useEffect() How to know if message array from another user have increased ? |
@harshjanipa Thanks for your proposal. It works as expected when a user is active and receives a message. But not if the user directly navigates to the chat via link. What's the reason for this behavior? Suggestion: Please make use of permalink to refer to the code where the problem lies. |
@JmillsExpensify, @sobitneupane, @MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@JmillsExpensify, @sobitneupane, @MonilBhavsar Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Waiting for proposals. |
@sobitneupane Would you please check this proposal again (or the updated one). Sorry for not providing much info in this comment, I think not much has changed since the initial proposal. |
@sobitneupane This proposal could solve this issue as well - #15019 (comment) |
Thanks @s77rt and @abdulrahuman5196 . Am I missing something or is it the case? |
@sobitneupane partly the same. After that issue get fixed we will have to apply this to actually fix this issue. |
@MonilBhavsar I think we should either hold this issue or combine this issue with #15019 one. |
I commented in Slack, but I favor combining this issue in the other issue and making sure that we solve both in the same go. This was also reported internally, so no issues there. |
Thoughts on that last comment @sobitneupane? I think we should close this issue out. |
Yes, we can close this issue. Let's make sure we solve this issue as well while handling #15019 issue. cc: @thesahindia |
Done! Added the steps to the linked issue. Thanks everyone! |
I agree with this. but I would say, let's add the workflow of this issue to the other issue and consider it while fixing it. |
@MonilBhavsar the case for this issue was already added to the linked issue. Screenshot from that OP for clarity. |
Separately from that, I love the idea of creating a tracking issue for the new marker line. We're experiencing multiple issues at this point and we're not going to holistically resolve until we look at everything holistically. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Issue was found when executing #12169
Action Performed:
Expected Result:
new marker is set correctly
Actual Result:
there is no new marker
Workaround:
unknown
Platform:
Where is this issue occurring?
Version Number: 1.2.25-0
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
Bug5813780_no_new_marker.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: