Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🍒 Cherry pick PR #42064 to staging 🍒 #42129

Merged
merged 3 commits into from
May 14, 2024
Merged

🍒 Cherry pick PR #42064 to staging 🍒 #42129

merged 3 commits into from
May 14, 2024

Conversation

os-botify[bot]
Copy link

@os-botify os-botify bot commented May 14, 2024 •
edited by Beamanator
Loading

🍒 Cherry pick #42064 to staging 🍒

Tests

  1. Set default tax as 0 and foreign tax as some value like 5%.
  2. Go to submit expense flow.
  3. Try to change the tax amount.
  4. It should change to the new amount if it is below the 5% limit.

OSBotify and others added 2 commits May 14, 2024 06:39
@OSBotify
Update version to 1.4.73-3
69ae25f 
(cherry picked from commit a5bd1f2)
@lakchote @OSBotify
Merge pull request #42064 from ShridharGoel/42407
718f9b3 
[CP Staging] Use selected currency when utilising tax rate

(cherry picked from commit 2304a1f)
@os-botify os-botify bot requested a review from a team as a code owner May 14, 2024 06:39
@os-botify
Copy link
Author

os-botify bot commented May 14, 2024

This pull request has merge conflicts and can not be automatically merged. 😞
Please manually resolve the conflicts, push your changes, and then request another reviewer to review and merge.
Important: There may be conflicts that GitHub is not able to detect, so please carefully review this pull request before approving.

@melvin-bot melvin-bot bot requested review from Julesssss and removed request for a team May 14, 2024 06:39
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented May 14, 2024

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

Beamanator
Beamanator reviewed May 14, 2024
View reviewed changes
src/pages/iou/request/step/IOURequestStepTaxAmountPage.tsx
@@ -150,12 +150,12 @@ function IOURequestStepTaxAmountPage({
<MoneyRequestAmountForm
isEditing={Boolean(backTo || isEditing)}
currency={currency}
amount={transactionDetails?.taxAmount}
taxAmount={getTaxAmount(transaction, policy, Boolean(backTo || isEditing))}
amount={Math.abs(transactionDetails?.taxAmount ?? 0)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This line specifically didn't come from #42064, but it was included in this cherry-pick PR for some reason - it looks like a fix to something else that exists in main which is why I kept it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hey this change is from this PR - https://github.com/Expensify/App/pull/42039/files - which we also wanted to CP, so good we added

@Beamanator Beamanator requested a review from lakchote May 14, 2024 07:04
MonilBhavsar
MonilBhavsar approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Straightforward and looks good!

@Beamanator Beamanator merged commit 8e8dc6d into staging May 14, 2024
4 of 9 checks passed
@Beamanator Beamanator deleted the Beamanator-cherry-pick-staging-42064-1 branch May 14, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Beamanator Beamanator Beamanator left review comments

@Julesssss Julesssss Julesssss approved these changes

@MonilBhavsar MonilBhavsar MonilBhavsar approved these changes

@lakchote lakchote Awaiting requested review from lakchote

Assignees

@Beamanator Beamanator

Labels
Engineering Hourly KSv2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Beamanator @Julesssss @MonilBhavsar @OSBotify @lakchote