[hold for payment 2024-08-27] [$250] Distance -Distance rate is not grayed out after editing "Tax rate" & "Tax reclaimable" offline

IuliiaHerets commented 1 month ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: v9.0.19-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Exp https://expensify.testrail.io/index.php?/tests/view/4849591 Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

Tax is enabled.
Distance rates are enabled.
Track tax in DIstance rates is enabled.

Go to staging.new.expensify.com
Go to workspace settings.
Go to Distance rates.
Go offline.
Tap on any rate.
Edit "Tax rate" and "Tax reclaimable on".
Return to distance rate list.

Expected Result:

The distance rate will be grayed out after editing "Tax rate" or "Tax reclaimable on" offline.

Actual Result:

The distance rate is not grayed out after editing "Tax rate" or "Tax reclaimable on" offline.

Workaround:

Unknown

Platforms:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/71db7762-531e-4430-8249-533e9c29e63f

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01495987645fc6b7df
Upwork Job ID: 1823734401301545382
Last Price Increase: 2024-08-14
Automatic offers:

ikevin127 | Reviewer | 103534987
FitseTLT | Contributor | 103534988

Issue Owner

Current Issue Owner: @ikevin127

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

IuliiaHerets commented 1 month ago

We think that this bug might be related to #wave-collect - Release 1

IuliiaHerets commented 1 month ago

@joekaufmanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

FitseTLT commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-08-13 14:31:45 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Distance rate is not grayed out after editing "Tax rate" & "Tax reclaimable" offline

What is the root cause of that problem?

Here we are missing taxClaimablePercentage and taxRateExternalID https://github.com/Expensify/App/blob/cc2efc6f7a4f17bc0ee6cdb4b8fb3fcf592c3cbb/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx#L115-L120

What changes do you think we should make in order to solve the problem?

We need to add pendingFields taxClaimablePercentage and taxRateExternalID to the checks here https://github.com/Expensify/App/blob/cc2efc6f7a4f17bc0ee6cdb4b8fb3fcf592c3cbb/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx#L115-L120


                        value.pendingFields?.taxRateExternalID ??
                        value.pendingFields?.taxClaimablePercentage ??

What alternative solutions did you explore? (Optional)

joekaufmanexpensify commented 4 weeks ago

Reproduced. I also verified that we generally do grey out the main option row if there is an offline change to a setting associated with it.

melvin-bot[bot] commented 4 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~01495987645fc6b7df

melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 (External)

ikevin127 commented 4 weeks ago

♻️ Reviewing proposal today.

ikevin127 commented 4 weeks ago

@FitseTLT's proposal LGTM. The RCA is correct and the proposed solution fixes the issue according to the Expected result.

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 4 weeks ago

📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 4 weeks ago

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

joekaufmanexpensify commented 4 weeks ago

@FitseTLT is there an ETA for PR here?

joekaufmanexpensify commented 3 weeks ago

PR pending

FitseTLT commented 3 weeks ago

PR ready

joekaufmanexpensify commented 3 weeks ago

TY! I see it's merged now

ikevin127 commented 3 weeks ago

⚠️ PR was just deployed to production ~10 minutes ago but automation failed to add the hold -> this should be on [HOLD for Payment 2024-08-27] according to today's production deploy from https://github.com/Expensify/App/pull/47591#issuecomment-2299886099.

cc @joekaufmanexpensify