Feature: Add the ability to configure tax rates on distance rates by nkdengineer · Pull Request #42141 · Expensify/App

nkdengineer · 2024-05-14T09:44:11Z

Details

Fixed Issues

$ #41574
PROPOSAL: #41574 (comment)

Tests

Go to Settings -> Workspace -> choose a Workspace with disable Taxes
Click Distance Rates -> Click Settings
Verify that: 'Track Tax' is locked and has a message below
Click More features in the message and navigate to More Features page
Enable Taxes
Back to Setting in Distance Rates
Verify that: 'Track Tax' button is not locked
Enable 'Track Tax'
Select a Rate
Verify that: have 2 new rows 'Tax Rate' and 'Tax reclaimable on'
Try to update 2 new rows
Verify that: the update is successful

Verify that no errors appear in the JS console

Offline tests

QA Steps

Go to Settings -> Workspace -> choose a Workspace with disable Taxes
Click Distance Rates -> Click Settings
Verify that: 'Track Tax' is locked and has a message below
Click More features in the message and navigate to More Features page
Enable Taxes
Back to Setting in Distance Rates
Verify that: 'Track Tax' button is not locked
Enable 'Track Tax'
Select a Rate
Verify that: have 2 new rows 'Tax Rate' and 'Tax reclaimable on'
Try to update 2 new rows
Verify that: the update is successful

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

android.mov

Android: mWeb Chrome

android-mweb.mov

iOS: Native

ios.mov

iOS: mWeb Safari

ios-mweb.mov

MacOS: Chrome / Safari

web.mov

MacOS: Desktop

desktop.mov

trjExpensify · 2024-05-22T21:48:31Z

The WebPR was deployed to staging if you want to resume on getting this merge ready!

trjExpensify · 2024-05-24T11:09:08Z

Is this ready for review?

MonilBhavsar · 2024-05-24T13:53:40Z

Mostly looks good. Looks like we need spanish translations here for some text entered in English

nkdengineer · 2024-05-24T14:16:49Z

@trjExpensify @MonilBhavsar I've completed bullets 1 and 2, but encountered some issues with the APIs in bullets 3 and 4. I've already raised the problem in the issue.

melvin-bot · 2024-05-27T18:07:55Z

@eVoloshchak 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]

nkdengineer · 2024-05-28T09:20:59Z

We need Spanish translations here for some text entered in English and asking in here

MonilBhavsar · 2024-05-28T13:38:46Z

src/pages/workspace/distanceRates/PolicyDistanceRateTaxReclaimableOnEditPage.tsx

+    const currency = rate.currency ?? CONST.CURRENCY.USD;
+    const currentTaxReclaimableOnValue = rate.attributes?.taxClaimablePercentage && rate.rate ? (rate.attributes.taxClaimablePercentage * rate.rate) / 100 : '';
+    const submitTaxReclaimableOn = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.POLICY_DISTANCE_RATE_TAX_RECLAIMABLE_ON_EDIT_FORM>) => {
+        Policy.updatePolicyDistanceRateValue(policyID, customUnit, [


I think we should make it 1:1:1 since this function is for updating the rate value. I'll add a new API, which we can use with similar parameters.

Can we please use UpdateDistanceTaxClaimableValue here and UpdateDistanceTaxRate where we update tax rate?

I added 2 function UpdateDistanceTaxClaimableValue && UpdateDistanceTaxRate, its use the same parameters and the same API with function updatePolicyDistanceRateValue
When the new API is done, please let me know so I can fix here

MonilBhavsar · 2024-05-29T13:34:26Z

@nkdengineer conflicts here.
I also commented on translations thread. We can update it now.
@eVoloshchak could you please take a look. Thanks all!

src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

src/pages/workspace/distanceRates/PolicyDistanceRateTaxRateSelectionModal.tsx

src/pages/workspace/distanceRates/PolicyDistanceRatesSettingsPage.tsx

src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

nkdengineer · 2024-06-05T13:38:16Z

Bug: Changing Tax rate and Tax reclaimable on stops working after you create a second distance rate
Bug: Tax reclaimable on number isn't fully displayed, it's rounded to one digit after the separator, while we allow entering three digits

@eVoloshchak Fixed these bug and update your suggestion. Ready for review again.

eVoloshchak · 2024-06-05T15:51:49Z

src/pages/workspace/distanceRates/PolicyDistanceRateDetailsPage.tsx

-                                onToggle={toggleRate}
-                                accessibilityLabel={translate('workspace.distanceRates.enableRate')}
+                <ScrollView contentContainerStyle={styles.flexGrow1}>
+                    <View>


Not critical: I think we can safely remove this <View>

@eVoloshchak Updated.

eVoloshchak · 2024-06-05T16:29:18Z

Bug: sometimes the amount is displayed as 0 when you try to edit it. I'm not sure what are the steps to reproduce this reliably, but I've hit this bug quite a few times just by updating the amount continuously. This might be a BE issue (did not verify this), but the responses look fine

Screen.Recording.2024-06-05.at.18.26.18.mov

Screen.Recording.2024-06-05.at.18.22.05.mov

MonilBhavsar · 2024-06-05T16:41:46Z

Not able to reproduce this ^

Screen.Recording.2024-06-05.at.10.10.47.PM.mov

MonilBhavsar · 2024-06-05T16:43:46Z

Noticed a small bug, we can fix it as followup -
If rate is 0.670 and tax reclaimable amount is also 0.670, then in Amount and Currency component, the reclaimable amount is displayed as 0.67, while the rate is displayed correctly as 0.670

nkdengineer · 2024-06-05T16:52:27Z

I can reproduce this bug. Some time the currentTaxReclaimableOnValue has too many digits after the decimal point that makes the amount form strip all digits after decimal. I fixed it now.

MonilBhavsar · 2024-06-05T16:58:29Z

Thanks for the quick fix @nkdengineer! Looks good to me.
@eVoloshchak are you around to take a look again? We can take a look and fix it as followup as we're not reliably able to reproduce #42141 (comment)

eVoloshchak

LGTM!

MonilBhavsar

Looks good! Let's ship it 🚀

MonilBhavsar · 2024-06-05T17:59:43Z

Nice teamwork on this @nkdengineer @eVoloshchak @trjExpensify 🤝

OSBotify · 2024-06-05T18:09:04Z

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

trjExpensify · 2024-06-05T18:47:36Z

Wahoo! Nice work ❤️

OSBotify · 2024-06-11T23:31:40Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.4.81-11 🚀

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

hungvu193 · 2024-07-11T04:17:24Z

src/pages/workspace/distanceRates/PolicyDistanceRateTaxRateEditPage.tsx

+                <SelectionList
+                    sections={[{data: taxRateItems}]}
+                    ListItem={RadioListItem}
+                    onSelectRow={onTaxRateChange}
+                    initiallyFocusedOptionKey={taxRateItems.find((item) => item.isSelected)?.keyForList}


it's recommended to use TaxPicker component
More context here: #43613 (comment)

rayane-d · 2024-10-19T18:04:42Z

src/pages/workspace/distanceRates/PolicyDistanceRatesSettingsPage.tsx

-                        />
-                    </OfflineWithFeedback>
-                    {policy?.areCategoriesEnabled && OptionsListUtils.hasEnabledOptions(policyCategories ?? {}) && (
+                <ScrollView contentContainerStyle={styles.flexGrow1}>


FYI, this bug: #48928 was missed in this PR. More details can be found in this proposal: #48928 (comment)

nkdengineer added 4 commits May 14, 2024 09:38

feat: add track tax

d71d7fc

feat: add tax rate

8d3eaf7

fix: add EnableDistanceRequestTax api

0cf9a2c

resolve conflict

8ae41f8

nkdengineer changed the title ~~Fix/41574~~ Feature: Add the ability to configure tax rates on distance rates May 15, 2024

nkdengineer mentioned this pull request May 15, 2024

[Payment Due 08/23][$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot #41574

Closed

nkdengineer added 3 commits May 23, 2024 14:55

Merge branch 'main' into fix/41574

548d3c0

fix: enableDistanceRequestTax params

496b791

fix: add navigation to more features

b32b350

nkdengineer added 4 commits May 27, 2024 16:26

Merge branch 'main' into fix/41574

9ad927d

feature: add tax claimable in distance rates

386bd5d

fix: add tax rate in distace rate unit

d2ab2c1

update en.ts

37b65f7

nkdengineer marked this pull request as ready for review May 27, 2024 18:07

nkdengineer requested a review from a team as a code owner May 27, 2024 18:07

melvin-bot bot removed the request for review from a team May 27, 2024 18:07

melvin-bot bot requested a review from eVoloshchak May 27, 2024 18:07

MonilBhavsar reviewed May 28, 2024

View reviewed changes

nkdengineer added 4 commits May 29, 2024 22:39

merge main

fc14c76

Merge branch 'main' into fix/41574

597bc0a

rename function update tax rate and tax claimable

d7cf692

update es.ts

2273707

eVoloshchak suggested changes May 29, 2024

View reviewed changes

MonilBhavsar requested a review from eVoloshchak June 5, 2024 13:45

eVoloshchak suggested changes Jun 5, 2024

View reviewed changes

remove unnecessary View

197268b

MonilBhavsar requested a review from eVoloshchak June 5, 2024 16:44

fix bug display amount

c0bd795

eVoloshchak approved these changes Jun 5, 2024

View reviewed changes

melvin-bot bot requested a review from MonilBhavsar June 5, 2024 17:50

MonilBhavsar approved these changes Jun 5, 2024

View reviewed changes

MonilBhavsar merged commit 8346110 into Expensify:main Jun 5, 2024

github-actions bot mentioned this pull request Jun 6, 2024

Deploy Checklist: New Expensify 2024-06-06 #43168

Closed

grgia mentioned this pull request Jun 6, 2024

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] Expense - App crashes when submitting expense on a newly created workspace #43171

Closed

6 tasks

MonilBhavsar mentioned this pull request Jun 6, 2024

Distance tax - Tax rate and tax reclaimable on rows are not grayed out when edited offline #43174

Closed

6 tasks

lanitochka17 mentioned this pull request Jun 6, 2024

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [REGRESSION] Android & iOS - Track tax - "Track tax" message does not scale according to device font size #43202

Closed

6 tasks

kavimuru mentioned this pull request Jun 7, 2024

[HOLD for payment 2024-06-20] [$250] Track tax - App crashes after assigning deleted tax rate to distance rate offline #43241

Closed

6 tasks

nkdengineer mentioned this pull request Jun 7, 2024

[CP Staging] Fix distance tax regression #43246

Merged

50 tasks

srikarparsi mentioned this pull request Jun 7, 2024

Track tax - Track tax switch does not appear grayed out when enabled/disabled offline #43318

Closed

6 tasks

hungvu193 reviewed Jul 11, 2024

View reviewed changes

hungvu193 mentioned this pull request Jul 11, 2024

[HOLD for payment 2024-07-10] [$250] [Track tax] Disabled tax rate can be selected #43613

Closed

6 tasks

rayane-d reviewed Oct 19, 2024

View reviewed changes

rayane-d mentioned this pull request Oct 19, 2024

[HOLD for payment 2024-10-25] [$250] Android & iOS - Spend category - Unable to select category when keyboard is up #48928

Closed

6 tasks

Conversation

nkdengineer commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

trjExpensify commented May 22, 2024

Uh oh!

trjExpensify commented May 24, 2024

Uh oh!

MonilBhavsar commented May 24, 2024

Uh oh!

nkdengineer commented May 24, 2024

Uh oh!

melvin-bot bot commented May 27, 2024

Uh oh!

nkdengineer commented May 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MonilBhavsar commented May 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nkdengineer commented Jun 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eVoloshchak commented Jun 5, 2024

Uh oh!

MonilBhavsar commented Jun 5, 2024

Uh oh!

MonilBhavsar commented Jun 5, 2024

Uh oh!

nkdengineer commented Jun 5, 2024

Uh oh!

MonilBhavsar commented Jun 5, 2024

Uh oh!

eVoloshchak left a comment

Choose a reason for hiding this comment

Uh oh!

MonilBhavsar left a comment

Choose a reason for hiding this comment

Uh oh!

MonilBhavsar commented Jun 5, 2024

Uh oh!

OSBotify commented Jun 5, 2024

Uh oh!

trjExpensify commented Jun 5, 2024

Uh oh!

OSBotify commented Jun 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nkdengineer commented May 14, 2024 •

edited

Loading