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

[Waiting for payment][$1000] LHN - Face issue when two user click each another at same time #23437

Closed
2 of 6 tasks
kbecciv opened this issue Jul 23, 2023 · 39 comments
Closed
2 of 6 tasks
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 Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Jul 23, 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!


Action Performed:

  1. Take two device
  2. Search for user B in device A & Search for user A on device B
  3. You will see one account in search result
  4. click on that account from both device together
  5. It wont reflect profile image an if you send message it will show error

Expected Result:

It should behave properly and take user to original chat screen between each other

Actual Result:

It opens up two chat screen

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.42-26
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
Notes/Photos/Videos: Any additional supporting documentation

Image.from.iOS.MP4
Screen.Recording.2023-07-21.at.4.12.46.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689936480446639

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b8478f5b1e48991e
  • Upwork Job ID: 1684794155842043904
  • Last Price Increase: 2023-08-04
  • Automatic offers:
    • fedirjh | Reviewer | 25995540
    • tienifr | Contributor | 25995542
    • DinalJivani | Reporter | 25995544
@kbecciv kbecciv added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Jul 23, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 23, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 23, 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

@kadiealexander
Copy link
Contributor

I attempted to reproduce this and wasn't able to on staging app version 1.3.44-2. @DinalJivani can you please update the iOS app to the latest version and let me know if you can still reproduce this?

@DinalJivani
Copy link

DinalJivani commented Jul 25, 2023

I attempted to reproduce this and wasn't able to on staging app version 1.3.44-2. @DinalJivani can you please update the iOS app to the latest version and let me know if you can still reproduce this?

I tested this issue with latest iOS/Native app and that version is still not updated, So basically on iOS/Native I'm using latest version.

Also, The above video is linked to this mobile video watch both video together for better understanding

Image.from.iOS.MP4

@kadiealexander
Copy link
Contributor

kadiealexander commented Jul 26, 2023

I understand the two actions were taken simultaneously, and I did the same on my iOS device and web browser:

RPReplay_Final1690260567.MP4
2023-07-25_16-49-30.mp4

The correct existing chat was opened on both devices when selecting each user simultaneously from both devices.

@DinalJivani
Copy link

@kadiealexander
Try with new chat with new user instead of existing ones.

Both user shouldn't have chat before then if you perform this, it will open two chat screens.

@kadiealexander
Copy link
Contributor

Thanks @DinalJivani, reproduced:

RPReplay_Final1690521114.MP4

@kadiealexander kadiealexander added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Jul 28, 2023
@melvin-bot melvin-bot bot changed the title LHN - Face issue when two user click each another at same time [$1000] LHN - Face issue when two user click each another at same time Jul 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

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

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

melvin-bot bot commented Jul 28, 2023

Current assignee @kadiealexander is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

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

@tienifr
Copy link
Contributor

tienifr commented Jul 28, 2023

Proposal

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

It opens up two chat screen when 2 users open chat with each other at the same time.

What is the root cause of that problem?

When 2 users open chat with each other at the same time, it will create optimistic report for both users at the same time, 1 report create API call will succeed and the other will return 200 but with preexistingReportID of the successfully created report. The "late" report will be invalid and when we try to send a message to it, it will show errors.

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

Pls note we have a very similar case here where the report is removed from another device. But in this case we need to navigate to the correct report:

  1. In here, we need to check if the updated report contains preexistingReportID and the prevProps.report does not, that means this is a "late" report.
  2. If yes, we navigate to the ReportScreen of that preexistingReportID and remove the "late" report and all its report actions from Onyx
  3. We can set this.setState({isReportRemoved: true}); similar to here before removing the "late" report from Onyx to avoid FullPageNotFoundView to showing

This will make sure the user experience graceful in this case.

What alternative solutions did you explore? (Optional)

We can considered the "late" report as removed and add the preexistingReportID check here so it follows the same "report removed" logic, but this navigates to Concierge instead, so I still prefer the above.

@melvin-bot melvin-bot bot added the Overdue label Jul 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

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

@fedirjh
Copy link
Contributor

fedirjh commented Jul 31, 2023

1 report create API call will succeed and the other will return 200 but with preexistingReportID of the successfully created report.

@tienifr didn’t we notify the second user about the new report data via pusher?

Do we have the same bug when user 1 starts a discussion offline and user 2 starts a discussion online, When user 1 comes online, did this same bug occurs?

@melvin-bot melvin-bot bot removed the Overdue label Jul 31, 2023
@tienifr
Copy link
Contributor

tienifr commented Aug 1, 2023

@tienifr didn’t we notify the second user about the new report data via pusher?

@fedirjh we do notify via Pusher, a new "correct" report with a different reportID will still be updated (via Pusher) in the LHN. But we're still on the "late" report at the time without switching to the "correct" report.

Do we have the same bug when user 1 starts a discussion offline and user 2 starts a discussion online, When user 1 comes online, did this same bug occurs?

@fedirjh a different bug occurs where the user is navigated to Concierge instead. It's different because in that case the data is not updated via Pusher but via "ReconnectApp" API when switching back to online.

@melvin-bot melvin-bot bot added the Overdue label Aug 3, 2023
@kadiealexander
Copy link
Contributor

@fedirjh any thoughts on the above?

@melvin-bot melvin-bot bot removed the Overdue label Aug 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 4, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 6, 2023

@fedirjh @kadiealexander this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@NikkiWines
Copy link
Contributor

Yep, @tienifr's proposal looks solid! Assigning 🚀

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 7, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 7, 2023

📣 @fedirjh 🎉 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
Copy link

melvin-bot bot commented Aug 7, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 7, 2023

📣 @DinalJivani 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@tienifr
Copy link
Contributor

tienifr commented Aug 8, 2023

The PR's ready for review #24295.

@fedirjh
Copy link
Contributor

fedirjh commented Aug 17, 2023

Update: PR is on hold for #24041.

There're major Onyx updates in #24041. Now I can neither delete the report nor log in a new account. I think it's better to hold until #24041 is merged so that we can continue testing.

@NikkiWines
Copy link
Contributor

@kadiealexander looks like this issue was resolved via a separate PR (#24320) so the PR for this issue has been closed and we can wrap this issue up

@tienifr
Copy link
Contributor

tienifr commented Sep 7, 2023

Bump @kadiealexander to wrap this up when you have time.

@tienifr
Copy link
Contributor

tienifr commented Sep 27, 2023

@NikkiWines @fedirjh @kadiealexander Quick bump to remind of #23437 (comment). We're 3 weeks delay.

@NikkiWines
Copy link
Contributor

DM'd @kadiealexander about this so we can move it along

@kadiealexander
Copy link
Contributor

Oops sorry team I'm not sure how I missed this one! I don't think anyone needs payment if this was solved by a previous bug report, please reopen if you disagree.

@tienifr
Copy link
Contributor

tienifr commented Oct 3, 2023

Oops sorry team I'm not sure how I missed this one! I don't think anyone needs payment if this was solved by a previous bug report, please reopen if you disagree.

hi @kadiealexander, I think contributors are eligible for compensation here since the solution was valid at the point of assignment and the PR process already started, just that due to some other changes outside of our control, the solution became out of date.

Another recent similar case is here and here

Thanks!

cc @NikkiWines

@kadiealexander kadiealexander reopened this Oct 5, 2023
@kadiealexander
Copy link
Contributor

kadiealexander commented Oct 5, 2023

Payouts due:

Eligible for 50% #urgency bonus? No

Upwork job is here.

@kadiealexander
Copy link
Contributor

@fedirjh please accept the Upwork contract here.

@fedirjh
Copy link
Contributor

fedirjh commented Oct 5, 2023

please accept the Upwork contract here.

@kadiealexander The offer has expired. Can you please send me an offer?

@fedirjh
Copy link
Contributor

fedirjh commented Nov 8, 2023

@kadiealexander friendly bump!

@kadiealexander
Copy link
Contributor

Sent one here

@kadiealexander kadiealexander added Daily KSv2 and removed Weekly KSv2 labels Nov 9, 2023
@kadiealexander kadiealexander changed the title [$1000] LHN - Face issue when two user click each another at same time [Waiting for payment][$1000] LHN - Face issue when two user click each another at same time Nov 9, 2023
@fedirjh
Copy link
Contributor

fedirjh commented Nov 9, 2023

@kadiealexander Thank you. Accepted!

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 Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

6 participants