search

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.78k forks source link

[$1000] Hidden shows for new user while split bill #23662

Closed kavimuru closed 1 year ago

kavimuru commented 1 year 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!

Action Performed:

  1. click on FAB icon and select Split bill option
  2. enter amount and click Next button
  3. enter email which is not in list
  4. select that user and click on Next button
  5. click on that user from split bill list

    Expected Result:

    should show email instead of Hidden

    Actual Result:

    Hidden shows instead of email

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

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

https://github.com/Expensify/App/assets/43996225/cff4d1ad-2172-44da-a240-f9a9fa97d480

https://github.com/Expensify/App/assets/43996225/6180d3f9-aa28-4925-b3a1-0a5087d293d0

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d600bb94443bb79d
  • Upwork Job ID: 1684342773688193024
  • Last Price Increase: 2023-07-26
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

samh-nl commented 1 year ago

Proposal

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

Hidden shows for new user while split bill

What is the root cause of that problem?

If the selected user is new or has not previously interacted with you, there are no personal details stored in Onyx yet. This leads to 'Hidden' being shown on the profile.

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

When selecting a user whom we have not interacted with yet, we should optimistically create the user.

What alternative solutions did you explore? (Optional)

There are a few other possibilities:

  1. We make the behavior similar to when creating a task, where clicking on the user navigates you back to the user selector instead of the user's profile.
  2. The ProfilePage currently only looks for information in personalDetailsList. This could be expanded to include fallback data sources, such as iou.participants. This is a less generic solution.
  3. We pass the information via react-navigation. However, this may be a less elegant approach.
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

Thanos30 commented 1 year ago

Proposal

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

Splitting the bill with a user we don't have a chat open with, shows hidden in info.

What is the root cause of that problem?

The root cause of this issue is that on PersonalPage.js, we are trying to get the data of the selected participant for the split from the personal details stored in Onyx. Since this is a new user, the only data that Onyx API returns for that user is accountID and avatar. Therefore, the displayName property takes the value hidden.

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

This issue could be solved in a few ways, I will try to keep this simple.

My initial thought would be to make an API request to populate the new user's personal data on Onyx PERSONAL_DETAILS key when we are done with adding participants on the participants split selector. I looked around the code and, correct me if I am wrong, this is only happening on the openReport API call.

Not sure if this is avoided on purpose, please let me know

What alternative solutions did you explore? (Optional)

Another solution I found to be interesting, is using the data on the IOU key of Onyx. This key holds data about the participants of the split.

We could check if this value does contain data for a participant with the same accountID we are trying to fetch, and fill it in to get the login and be able to get the displayName, since that data contains both accountID and login.

Please let me know if any of the solutions are acceptable, so I can prepare a Proposal based on one of them, or if I should investigate further solutions

Thanks 🙏

parasharrajat commented 1 year ago

I think the simplest and clean solution would be to prevent viewing the Details for a user to be invited. We will prevent navigating to the details page for such users. This can be done easily via return early when isOptimisticAccount = true. https://github.com/Expensify/App/blob/f8bc30500acaed95d4668657cf18558deee79194/src/components/MoneyRequestConfirmationList.js#L213

Confirming this https://expensify.slack.com/archives/C01GTK53T8Q/p1690455544553389.

parasharrajat commented 1 year ago

Ok, it seems that it is a duplicate of https://github.com/Expensify/App/issues/22691 @lschurr

Julesssss commented 1 year ago

Agree this is a dupe