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.49k stars 2.85k forks source link

Card - "*member* added shipping details" message appears when the details aren't set yet #50233

Open IuliiaHerets opened 2 weeks ago

IuliiaHerets commented 2 weeks 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!


Version Number: 9.0.44-8 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Log in with a new Expensifail account
  3. Create a workspace
  4. Navigate to Workspace settings - More features
  5. Enable "Expensify Card"
  6. Navigate to Workspace settings - Members
  7. Invite a Gmail address that doesn't have an account yet
  8. Navigate to Workspace settings - Expensify Card
  9. Click on the "Issue new card" button
  10. Add bank account details and finish the BA flow
  11. Click on the "Issue new card" button
  12. Select the new member
  13. Select "Physical card
  14. Add the card with any limit type, limit and name
  15. Navigate to the workspace chat with the member

Expected Result:

A different message should be visible as the member didn't add shipping details yet.

Actual Result:

"member added shipping details. Expensify Card will arrive in 2-3 business days." message appears for the admin.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/b5b2d6d9-2319-447e-9af6-81d5a35f17b4

View all open jobs on GitHub

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

IuliiaHerets commented 2 weeks ago

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

Nodebrute commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-10-04 12:42:49 UTC.

Proposal

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

"member added shipping details" message appears when the details aren't set yet

What is the root cause of that problem?

We should show this message here https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/languages/en.ts#L3044 And we only show this message when isAssigneeCurrentUser is true https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/libs/ReportActionsUtils.ts#L1750

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

We can remove this check from here isAssigneeCurrentUser https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/libs/ReportActionsUtils.ts#L1750

We also need to do some cleanup that can be handled in the pr. or we can show message of our choice.

What alternative solutions did you explore? (Optional)

nyomanjyotisa commented 2 weeks ago

Proposal

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

"member added shipping details" message appears when the details aren't set yet

What is the root cause of that problem?

We only display the issuedCardNoShippingDetails text if isAssigneeCurrentUser, otherwise we display the addedShippingDetails text https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/libs/ReportActionsUtils.ts#L1750-L1760

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

We need to display the issuedCardNoShippingDetails on all users

We can simply replace shouldShowAddMissingDetailsButton to missingDetails here https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/libs/ReportActionsUtils.ts#L1757

And remove the following code since it no longer used https://github.com/Expensify/App/blob/3c91e07f87838aa345beb812bb397e45c711aa5b/src/libs/ReportActionsUtils.ts#L1748-L1750

What alternative solutions did you explore? (Optional)

allgandalf commented 2 weeks ago

@shubham1206agra is this related to recent changes we made ?

shubham1206agra commented 2 weeks ago

@mountiny I think the current text needs to be more accurate. We should have a different text.

mountiny commented 2 weeks ago

Yeah, I see that we added the isAssigneeCurrentUser there, but that is incorrect.

I think what we can do is:

It's like an audit trail; I think it's okay to just leave the MISSING_ADDRESS report action as is, given that the next report action clarifies that the address was provided and shipped.

mountiny commented 2 weeks ago

This is a regression from PR from @shubham1206agra so I think it makes sense for them to raise a fix PR

allgandalf commented 1 week ago

@shubham1206agra any update here?