fix: unify TBD-related logic for Distance requests

[paultsimura]

Details

• Change places where "TBD" was used to use "Route pending..." instead;
• Unify some pending-route-related logic to minimize potential discrepancies among different screens of the same transaction.
• Update the Merchant to "Route pending..." when editing waypoints

Fixed Issues

$ https://github.com/Expensify/App/issues/33362 PROPOSAL: https://github.com/Expensify/App/issues/33362#issuecomment-1864549994

Large Slack discussion: https://expensify.slack.com/archives/C01GTK53T8Q/p1705425683928599

Tests

Same as QA

Offline tests

Same as QA

QA Steps

1. Go offline
2. Click FAB -> Distance
3. Proceed to the confirmation page

• [ ] Verify the amount is blank
• [ ] Verify the distance shows "Route pending..."

4. Complete the distance request
5. Navigate (if not already) to the chat that contains the recently created Distance request

• [ ] Verify that the amount is displayed as Route pending...
• [ ] Verify that in the LHN, the preview shows Route pending...
• [ ] Verify the total Expense shows $0

6. Go Online (and wait a little)

• [ ] Verify that the amount and distance are updated with the data from the BE

7. Navigate to this Distance request
8. Go offline again
9. Click "Distance"
10. Add a waypoint and click "Save"

• [ ] Verify that the amount is blank
• [ ] Verify that the distance field says Route pending
• [ ] Verify the report title says Route pending

11. Click on the receipt thumbnail

• [ ] Verify that the distance on the e-Receipt shows Route pending and the amount is not shown

12. Without going back online, modify the amount manually (available only if you're the admin or manager of the workspace)

• [ ] Verify the amount field shows the manual amount
• [ ] Verify the distance still shows "Route pending..."

13. Go to the parent Expense report

• [ ] Verify the money request preview shows the manual amount and "Route pending..." in the subtitle
• [ ] Verify in LHN, the report preview shows the manually requested amount.

PR Author Checklist

• [x] I linked the correct issue in the ### Fixed Issues section above
• [x] I wrote clear testing steps that cover the changes made in this PR
  • [x] I added steps for local testing in the Tests section
  • [x] I added steps for the expected offline behavior in the Offline steps section
  • [x] I added steps for Staging and/or Production testing in the QA steps section
  • [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• [x] I included screenshots or videos for tests on all platforms
• [x] I ran the tests on all platforms & verified they passed on:
  • [x] Android: Native
  • [x] Android: mWeb Chrome
  • [x] iOS: Native
  • [x] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop
• [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• [x] I followed proper code patterns (see Reviewing the code)
  • [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • [x] I verified that comments were added to code that is not self explanatory
  • [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • [x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message: https://expensify.slack.com/archives/C01GTK53T8Q/p1705655037266699
  • [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • [x] I verified the JSDocs style guidelines (in STYLE.md) were followed
• [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• [x] I followed the guidelines as stated in the Review Guidelines
• [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• [x] I verified that if a function's arguments changed that all usages have also been updated correctly
• [x] If any new file was added I verified that:
  • [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• [x] If a new CSS style is added I verified that:
  • [x] A similar style doesn't already exist
  • [x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• [x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• [x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• [x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• [x] If the PR modifies the form input styles:
  • [x] I verified that all the inputs inside a form are aligned with each other.
  • [x] I added Design label so the design team can review the changes.
• [x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• [x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

https://github.com/Expensify/App/assets/12595293/b25a24b3-a3d5-46a7-8c67-863778971f82

Android: mWeb Chrome

https://github.com/Expensify/App/assets/12595293/608480e6-b7e3-4f31-b45f-503deef94298

iOS: Native

https://github.com/Expensify/App/assets/12595293/248bad6a-4038-444c-b665-405ec5fc8140

iOS: mWeb Safari

https://github.com/Expensify/App/assets/12595293/c896d207-dd5b-47e5-8f2f-3d5114a42fec

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/12595293/0f8d788a-0635-4d65-9e94-a24375661717

MacOS: Desktop

https://github.com/Expensify/App/assets/12595293/7bab1a3b-f2f1-4c39-893f-eff7dd62aed1

[melvin-bot[bot]]

@parasharrajat Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[neil-marcellini]

@paultsimura I'm going to convert this to a draft so we can tell when it's ready.

[paultsimura]

Ready for review. This one is quite large, I definitely may have missed smth. cc: @parasharrajat @neil-marcellini @tgolen

[parasharrajat]

@paultsimura Thanks for carrying the discussion on Slack while I was OOO. Could you please confirm that the changes are explained in OP? The Slack thread is quite large.

[paultsimura]

Could you please confirm that the changes are explained in OP?

Yes, I tried to cover as much details as possible in the testing steps

[tgolen]

@parasharrajat I'll let you have a first pass on the review for this before I take a look at it.

[parasharrajat]

Sure. Thanks. I will have an update by tomorrow.

[parasharrajat]

Reviewing...

[tgolen]

@paultsimura you picked up a conflict.

[paultsimura]

@paultsimura you picked up a conflict.

Resolved, thanks. The PR touches a lot of places, so new conflicts each day are almost guaranteed. @parasharrajat did you have any luck reviewing so far?

[parasharrajat]

Yeah, I looked at the code but had to switch to more urgent PRs related to violations. I will continue in 20mins on this one.

[parasharrajat]

Currently reviewing it... matching code changes carefully.

[s77rt]

Tested creating a distance request offline and still seeing TBD in rate and $0.00 in amount

[paultsimura]

Tested creating a distance request offline and still seeing TBD in rate and $0.00 in amount

@s77rt This PR is not merged yet. Or did you test the PR itself?

[s77rt]

@paultsimura Just retested the rate if fixed. The amount is empty. I think the expected behaviour has been cleared here https://expensify.slack.com/archives/C01GTK53T8Q/p1706271626780149?thread_ts=1705419422.226989&cid=C01GTK53T8Q. Thanks for quick response!

[paultsimura]

Thank you for reporting it here in the first place @s77rt 🙌🏽

[parasharrajat]

@paultsimura Could you please merge main?

[paultsimura]

@parasharrajat merged ✔️

[neil-marcellini]

@parasharrajat when do you expect to complete your review? I'll review after that.

[parasharrajat]

Will be completely it today.

[parasharrajat]

Back in a few minutes to complete the checklist.

[parasharrajat]

@paultsimura Are you also facing issues while sending money? I think it is fixed on main. Could you please merge main?

[paultsimura]

Merged ✔️

[parasharrajat]

Only one test is failing for me. I am not able to edit Amount.

https://github.com/Expensify/App/assets/24370807/da853888-3750-4d19-9007-c4dd30822c94

[parasharrajat]

I will be back in 2 hours and check it again. @paultsimura Could you please help me understand the above case.

I tested many scenarios and also reviewed the code for different percpectives, all looks good to me. I will fill up the checklist and videos after I am back.

[paultsimura]

Only one test is failing for me. I am not able to edit Amount.

Hey @parasharrajat, you're right, that's an interesting finding – apparently, only an Admin or a Manager of the policy can edit the Distance request's amount (quite logical tbh):

https://github.com/Expensify/App/blob/b6363778127003ce5ecb7375c30bb5d05d77b7d9/src/libs/ReportUtils.ts#L2099-L2105

Therefore, you have to request the money from your own workspace in order to be able to complete this test. I've updated the test with this information, thanks.

[parasharrajat]

Screenshots

:black_square_button: iOS / native

https://github.com/Expensify/App/assets/24370807/330677f4-4b2a-43bf-896b-47761772ae8e

:black_square_button: iOS / Safari

https://github.com/Expensify/App/assets/24370807/86e9ef40-e777-4d44-b2d7-9d9b22b69fe9

https://github.com/Expensify/App/assets/24370807/55a44527-1c33-4653-bdb6-f9a0004d8d0d

:black_square_button: MacOS / Desktop

https://github.com/Expensify/App/assets/24370807/0f5f88e3-de09-44a3-a31b-bf3c367faf84

:black_square_button: MacOS / Chrome

https://github.com/Expensify/App/assets/24370807/cba124d2-6c91-4561-85be-4c1770d3660c

https://github.com/Expensify/App/assets/24370807/4e4b2c27-316f-4a6c-ad11-a7377dc0a066

:black_square_button: Android / Chrome

https://github.com/Expensify/App/assets/24370807/cbb4a8e3-3321-4018-bc78-c5375ee7df9a

https://github.com/Expensify/App/assets/24370807/91da6038-49ea-4131-a5bc-a788e1ba5299

:black_square_button: Android / native

https://github.com/Expensify/App/assets/24370807/020c85fc-eeed-49b6-b2e3-ce30b579d204

[neil-marcellini]

Taking a look soon! @paultsimura fyi there are some merge conflicts, but I will review anyways of course.

[paultsimura]

Hey @neil-marcellini, thanks for your feedback. Regarding this matter:

NAB: Shouldn't the amount show as pending at 50% opacity, vs blank?

There was a rather long discussion about this, and we came to a conclusion to mimic the Scan flow where the amount is blank. That's because the 50% opacity means we know the value that'll be persisted once come back online. In this case, we don't know the expected amount, so showing some stale amount with 50% opacity would be incorrect.

refs: https://expensify.slack.com/archives/C01GTK53T8Q/p1705519393468329?thread_ts=1705425683.928599&cid=C01GTK53T8Q https://expensify.slack.com/archives/C01GTK53T8Q/p1705523687494059?thread_ts=1705425683.928599&cid=C01GTK53T8Q

Also, maybe @trjExpensify @tgolen could double-confirm

[neil-marcellini]

Ah shoot @paultsimura, conflicts again. Please lmk when they're fixed and I'll merge it.