Feat/36985 create new rate field #38543

koko57 · 2024-03-18T23:02:56Z

Details

Fixed Issues

$ #36985
PROPOSAL: -

Tests

P2P / Splits

Go to global create (+) > Submit expense
Change the request type to Distance
Choose and submit the start/finish points
Choose one user or add to a split and go to the confirmation page
Verify that the distance is displayed correctly
Verify that clicking the distance field navigates to the distance editing page
Go back to the confirmation page
Verify that the rate is displayed correctly
Verify that the field is non-interactive

** Workspace Requests**

Prerequisites:

Log in to expensify.com
Go to Settings > Workspaces
Create a new Collect workspace if needed
Click on the workspace to open its settings page
Open the JS console (command + option + j)
Paste in the following and hit enter to enable the workspace to be used in NewDot

p = Policy.getCurrent(); 
p.policy.isPolicyExpenseChatEnabled = "true"; 
p.save();

Click on Expenses in the left hand menu of the workspace settings and scroll down to the Distance card
Click “Add a mileage rate”, then enter a unique name and value
Click Members in the left hand menu and invite a member (required so that an expense chat is created for NewDot)
Log in as the member on new expensify

Go to global create (+) > Submit expense
Change the request type to Distance
Choose and submit the start/finish points
Select the collect workspace and go to the confirmation page
Verify that the distance is displayed correctly
Verify that clicking the distance field navigates to the distance editing page
Go back to the confirmation page
Verify that the default rate is used and displayed correctly
Verify that the field has a right arrow icon, is interactive and navigates to the rate selection page
Verify that both workspace rates are displayed correctly on the selection page, and the default is currently selected
Select the other rate
Verify you are navigated back to the confirmation page and the amount and the rate field are properly updated

Money Request View

Open the expense report
Verify that a request preview is displayed with the proper distance and rate
Open the request
Verify that the Distance field is displayed with the correct value
Verify that the Rate is displayed with correct value
Rate field is not interactive for now, it'll be changed later

Sign in/out

Create a distance request with a custom rate.
Sign out.
Sign back in.
Go to the money request view.
Verify that the data is shown properly.

Verify that no errors appear in the JS console

Offline tests

n/a

QA Steps

P2P / Splits

Go to global create (+) > Request money
Change the request type to Distance
Choose and submit the start/finish points
Choose one user or add to a split and go to the confirmation page
Verify that the distance is displayed correctly
Verify that clicking the distance field navigates to the distance editing page
Go back to the confirmation page
Verify that the rate is displayed correctly
Verify that the field is non-interactive

** Workspace Requests**

Prerequisites:

Log in to expensify.com
Go to Settings > Workspaces
Create a new Collect workspace if needed
Click on the workspace to open its settings page
Open the JS console (command + option + j)
Paste in the following and hit enter to enable the workspace to be used in NewDot

p = Policy.getCurrent(); 
p.policy.isPolicyExpenseChatEnabled = "true"; 
p.save();

Click on Expenses in the left hand menu of the workspace settings and scroll down to the Distance card
Click “Add a mileage rate”, then enter a unique name and value
Click Members in the left hand menu and invite a member (required so that an expense chat is created for NewDot)
Log in as the member on new expensify

Go to global create (+) > Request money
Change the request type to Distance
Choose and submit the start/finish points
Select the collect workspace and go to the confirmation page
Verify that the distance is displayed correctly
Verify that clicking the distance field navigates to the distance editing page
Go back to the confirmation page
Verify that the default rate is used and displayed correctly
Verify that the field has a right arrow icon, is interactive and navigates to the rate selection page
Verify that both workspace rates are displayed correctly on the selection page, and the default is currently selected
Select the other rate
Verify you are navigated back to the confirmation page and the amount and the rate field are properly updated

Money Request View

Open the expense report
Verify that a request preview is displayed with the proper distance and rate
Open the request
Verify that the Distance field is displayed with the correct value
Verify that the Rate is displayed with correct value
Rate field is not interactive for now, it'll be changed later

Sign in/out

Create a distance request with a custom rate.
Sign out.
Sign back in.
Go to the money request view.
Verify that the data is shown properly.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2024-03-29.at.12.56.06.mp4

Android: mWeb Chrome

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-28.at.21.30.34.mov

iOS: mWeb Safari

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-28.at.21.49.07.mov

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/28020445/3ac987f8-0e5c-482a-bb0e-23eeab7c8f02

MacOS: Desktop

Screen.Recording.2024-03-28.at.21.54.30.mp4

koko57 · 2024-04-18T21:34:40Z

@getusha done

getusha

lgtm & tests well

neil-marcellini

All good to go except one tiny thing which is passing the right iouType to usePermissions for p2p distance on the money request view.

I left a bunch of non blocking comments too. You can address some now if you like, or handle them all in a follow up.

src/components/MoneyRequestConfirmationList.tsx

src/components/ReportActionItem/MoneyRequestView.tsx

neil-marcellini · 2024-04-19T17:18:39Z

src/components/ReportActionItem/MoneyRequestView.tsx

+
+    const currency = policy ? policy.outputCurrency : PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
+
+    const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(currency) : rates[rateID as string] ?? {};


NAB: There's another complication here that we forgot about. Let's handle it in a follow up PR please, since I don't want to delay this one more and add more complexity.

If we're viewing an old request, and the defaultP2P rates have changed since the request was created, then we might display the wrong rate because the rate the request was created with is no longer in the default mapping.

Therefore when viewing a created request we should first try to get the default rate from transaction.comment.customUnit.defaultP2PRate. It stores the rate value number only.

@neil-marcellini ok I missed this one - will implement this

src/libs/DistanceRequestUtils.ts

neil-marcellini · 2024-04-19T17:32:03Z

src/libs/DistanceRequestUtils.ts


-    return `${formattedDistance} @ ${currencySymbol}${ratePerUnit} / ${singularDistanceUnit}`;
+function getRateForP2P(currency: string): RateAndUnit {


NAB: Noting that this is where we need to handle getting the rate from the transaction defaultP2PRate

@neil-marcellini you mean adding a comment what this function does?

Implementing this

neil-marcellini · 2024-04-19T17:34:33Z

src/libs/DistanceRequestUtils.ts

+    }
+
+    const distance = Number(merchant.split(' ')[0]);
+    if (!distance) {


NAB: It would also be a good idea to validate that the merchant has the expected format and return 0 if it does not. The regex we're using on the backend is found in a comment I posted here. Let's do this in a follow up.

@neil-marcellini FYI: I needed to change this regex a bit, /^[0-9.]+ \w+ @ (-|-$)?(\p{Sc}|\p{L}|\w){1,3} ?[0-9.]+$? \/ \w+$/u - added an "u" flag to catch unicode and this one: \p{L} bc the regex didn't work when we had PLN currency - "zł"

Ok great thanks. If you would please provide some examples where it breaks with the old regex, then I can get the backend fixed too so that the regexes match.

neil-marcellini

I found that requesting money with a custom rate on a workspace doesn't work. @getusha your test video shows it failing in a different way, so it looks like maybe you skipped this.

It looks like we're not sending the customUnitRateID param to the backend. After the updates, @getusha would you please also test the case of viewing an expense with a custom rate shortly after signing in?

customRate720.mov

koko57 · 2024-04-19T19:03:04Z

@getusha could you please retest?

neil-marcellini

Great, it looks like creating with a custom rate is fixed.

Screen.Recording.2024-04-19.at.12.10.31.PM.mov

Also viewing the money request after signing back in works well. All set! We will handle the rest in follow ups.

Screen.Recording.2024-04-19.at.12.11.43.PM.mov

Tim is out today and I want to get this merged for #urgency

OSBotify · 2024-04-19T19:21:46Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-04-23T23:42:16Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.64-6 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

shubham1206agra · 2024-06-05T10:24:55Z

src/libs/TransactionUtils.ts

-function hasRoute(transaction: OnyxEntry<Transaction>): boolean {
-    return !!transaction?.routes?.route0?.geometry?.coordinates;
+function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType: boolean): boolean {
+    return !!transaction?.routes?.route0?.geometry?.coordinates || (isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);


In saveWaypoint function, you forgot to clear comment.customUnit.quantity, which caused #40934

ikevin127 · 2024-06-15T21:02:25Z

This PR also created issue:

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] IOU – Disabled distance rate is present in the rate list in confirmation page #42284

which consists of 2 bugs:

Not checking if lastSelectedDistanceRates is disabled. Additionally, in the getDefaultMileageRate function -> we're not excluding disabled rates.
We're using getMileageRates to get rate list in IOURequestStepDistanceRate, but in this function we don't filter out disabled rates.

Neil acknowledged the oversight in #42284 (comment):

I guess we forgot about handling this during the initial implementation.

src/components/MoneyRequestConfirmationList.tsx

allgandalf · 2024-08-30T07:29:30Z

src/components/MoneyRequestConfirmationList.tsx

+    const customUnitRateID = TransactionUtils.getRateID(transaction) ?? '';
+
+    useEffect(() => {
+        if (customUnitRateID || !canUseP2PDistanceRequests) {


We shouldn't have returned early for P2P distance requests, instead we should had checked for canUseP2PDistanceRequests while assigning rateID, this caused #45856

koko57 added 18 commits February 26, 2024 17:14

feat: create new rate field

1db16d9

feat: create rate selection page

2c19aa3

fix: resolve conflicts

f3b17ad

feat: add translation for the rate page

b3e1f48

feat: wip - display rates for policy

6e94444

fix: resolve conflicts

e759979

feat: wip - get rate for p2p

cde1d19

feat: wip - new rate field logic

c3cb80b

fix: revert filtering changes

5bf399a

fix: resolve conflicts

857c320

fix: resolve conflicts

bf5e278

refactor: use canUseP2PDistanceRequests from usePermissions

ef7b30e

fix: remove mock rates

6f3afdd

feat: create function for getting personal policy

50174f3

feat: add rate field to MoneyRequestView

eb3aff8

fix: resolve conflicts

18e2c7c

Merge branch 'main' into feat/36985-create-new-rate-field

1ddc839

feat: change customRateID when participant changes, other improvements

7dc206c

koko57 mentioned this pull request Mar 18, 2024

[HOLD for payment 2024-05-02] CRITICAL: [P2P Distance] [$500] Create a new Rate field #36985

Closed

koko57 added 11 commits March 19, 2024 08:36

fix: resolve conflicts

7698b5a

fix: run prettier

a0227e0

fix: lint fix

eb350ee

fix: resolve conflicts

af94a75

fix: resolve conflicts

272c300

fix: add missing screen

0682afb

fix: show check mark on lastSelectedDistanceRate

6105b5d

fix: fix distance edit route

063a8b6

fix: path fix

7fe595a

fix: minor fix

9e68814

fix: enable fields for split, fix route

68eecf0

getusha approved these changes Apr 18, 2024

View reviewed changes

melvin-bot bot requested a review from neil-marcellini April 18, 2024 21:45

koko57 added 2 commits April 19, 2024 11:32

fix: resolve conflicts

29da5dc

fix: resolve conflicts

6bb3199

neil-marcellini requested changes Apr 19, 2024

View reviewed changes

fix: sending customUnitRateID

a93e729

neil-marcellini approved these changes Apr 19, 2024

View reviewed changes

neil-marcellini merged commit cba5f03 into Expensify:main Apr 19, 2024
16 checks passed

Beamanator mentioned this pull request Apr 22, 2024

🍒 Cherry pick PR #40648 to staging 🍒 #40673

Merged

48 tasks

github-actions bot mentioned this pull request Apr 22, 2024

Deploy Checklist: New Expensify 2024-04-22 #40702

Closed

95 tasks

This was referenced Apr 22, 2024

[HOLD for payment 2024-05-02] Distance - Distance row shows "0.00 miles" instead of "Pending" after changing distance #40712

Closed

Distance - Distance rate currency does not update after changing distance expense currency #40710

Closed

neil-marcellini mentioned this pull request Apr 22, 2024

[CP Staging] Feat/36985 create new rate field followups #40711

Merged

61 tasks

kbecciv mentioned this pull request Apr 22, 2024

[HOLD for payment 2024-05-02] Distance - Selected distance rate is not highlighted in rate list #40722

Closed

6 tasks

allgandalf mentioned this pull request Apr 23, 2024

[$250] [HOLD for payment 2024-05-06] Group split scan - Error appears for Merchant when clicking Split expense for the second time #40749

Closed

6 tasks

izarutskaya mentioned this pull request Apr 23, 2024

Distance - Mileage Rate changes from previous value when editing waypoints #40762

Closed

6 tasks

shubham1206agra reviewed Jun 5, 2024

View reviewed changes

shubham1206agra mentioned this pull request Jun 5, 2024

[HOLD for payment 2024-06-05] [$250] Distance - No error message appears when trying to create an impossible route #40934

Closed

6 tasks

ikevin127 mentioned this pull request Jun 15, 2024

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] IOU – Disabled distance rate is present in the rate list in confirmation page #42284

Closed

6 tasks

daledah reviewed Jul 25, 2024

View reviewed changes

src/components/MoneyRequestConfirmationList.tsx Show resolved Hide resolved

allgandalf mentioned this pull request Jul 26, 2024

[HOLD for payment 2024-09-03] [$250] Distance - Error displayed when creating distance expense in workspace that not opened yet #45856

Closed

6 tasks

allgandalf reviewed Aug 30, 2024

View reviewed changes

paultsimura mentioned this pull request Oct 21, 2024

[HOLD for payment 2024-10-25] [$250][P2P Distance] Rate currency doesn't match expense currency #46844

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/36985 create new rate field #38543

Feat/36985 create new rate field #38543

koko57 commented Mar 18, 2024 •

edited

Loading

koko57 commented Apr 18, 2024

getusha left a comment

neil-marcellini left a comment

neil-marcellini Apr 19, 2024

koko57 Apr 22, 2024

neil-marcellini Apr 19, 2024

koko57 Apr 22, 2024

neil-marcellini Apr 22, 2024

neil-marcellini Apr 19, 2024

koko57 Apr 22, 2024

neil-marcellini Apr 22, 2024

neil-marcellini left a comment

koko57 commented Apr 19, 2024

neil-marcellini left a comment

OSBotify commented Apr 19, 2024

OSBotify commented Apr 23, 2024

shubham1206agra Jun 5, 2024

ikevin127 commented Jun 15, 2024

allgandalf Aug 30, 2024


		const currency = policy ? policy.outputCurrency : PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;

		const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(currency) : rates[rateID as string] ?? {};


		return `${formattedDistance} @ ${currencySymbol}${ratePerUnit} / ${singularDistanceUnit}`;
		function getRateForP2P(currency: string): RateAndUnit {

Feat/36985 create new rate field #38543

Feat/36985 create new rate field #38543

Conversation

koko57 commented Mar 18, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

koko57 commented Apr 18, 2024

getusha left a comment

Choose a reason for hiding this comment

neil-marcellini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-marcellini left a comment

Choose a reason for hiding this comment

koko57 commented Apr 19, 2024

neil-marcellini left a comment

Choose a reason for hiding this comment

OSBotify commented Apr 19, 2024

OSBotify commented Apr 23, 2024

Choose a reason for hiding this comment

ikevin127 commented Jun 15, 2024

Choose a reason for hiding this comment

koko57 commented Mar 18, 2024 •

edited

Loading