Apply tax rule when selecting category#53332
Conversation
This should be fixed soon
Interesting, I'll take a look. Is it an existing bug? |
This is now on server. @bernhardoj could you please test and let me know |
Yes, it's fixed now.
Looks like it's a new issue. I tested by selecting a category with the default tax rate (0%) and change the tax rate manually to 5% and it works. |
Could you please clarify steps in detail. I am not completely getting it |
|
I have a workspace A with categories A and B. Category A has a tax rate of 5%, while category B has the default tax rate, 0%. This works fine:
This doesn't work fine
|
|
Okay thanks! Going to test that case on your branch |
|
Sorry, I was on wrong version |
|
Okay reproduced the bug, thanks! Checking... |
|
@bernhardoj I noticed, it is because taxAmount is passed negative number when categorizing. Do you please mind updating here? |
|
@MonilBhavsar Ah, I got it now. Updated |
|
Thanks! @mollfpr this can be reviewed and tested now |
Reviewer Checklist
Screenshots/VideosAndroid: NativeManual 53332.-.Android.-.Manual.mp4Split 53332.-.Android.-.Split.mp4Categorize 53332.-.Android.-.Categorize.mp4Android: mWeb ChromeManual 53332.-.mWeb-Chrome.-.Manual.mp4Split 53332.-.mWeb-Chrome.-.Split.mp4Categorize 53332.-.mWeb-Chrome.-.Categorize.mp4iOS: NativeManual Split Categorize iOS: mWeb SafariManual Split Categorize MacOS: Chrome / SafariManual 53332.-.Web.-.Manual.mp4Split 53332.-.Web.-.Split.mp4Categorize 53332.-.Web.-.Categorize.mp4MacOS: DesktopManual 53332.-.Desktop.-.Manual.mp4Split 53332.-.Desktop.-.Split.mp4Categorize 53332.-.Desktop.-.Categorize.mp4 |
|
@bernhardoj can we please add unit tests? |
|
I already handled the IOUTest in #54232 |
|
Could you please fix it here so we can merge this |
|
Done |
|
@bernhardoj there is a conflict to resolve |
|
@MonilBhavsar Fixed |
|
Sorry, more conflicts 😬 |
|
Fixed |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
2 similar comments
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
1 similar comment
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.0.78-0 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.78-6 🚀
|


Explanation of Change
Fixed Issues
$ #53220
PROPOSAL: #53220 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Prerequisite:
Manual:
Split:
7. Go back to workspace chat
8. Create a manual split request with the category A/B
9. Press the split preview and verify the tax rate is the tax rate of the selected category
10. Create a scan split request with the category A
11. Press the split preview and verify the tax rate is the tax rate A
12. Change the category to B and verify the tax rate changes to tax rate B
13. Split the scan request
14. Press the split preview again and verify the tax rate is the tax rate B
Categorize:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios.mweb.mp4
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop
desktop.mp4