Fix manual distance expense from QAB showing amount as zero#84658
Fix manual distance expense from QAB showing amount as zero#84658
Conversation
When creating a manual distance expense via QAB (Quick Action Button), the skip-confirmation path hardcoded amount: 0 for both trackExpense and createDistanceRequest. For map-based distance, the backend recalculates from validWaypoints, but manual distance has no waypoints. This meant the expense displayed $0.00, especially noticeable offline. This computes the amount from distance and mileage rate before passing it to trackExpense/createDistanceRequest, using the same DistanceRequestUtils functions used elsewhere in the codebase. Fixed #82801 Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
The failing Analysis: The ESLint run reports 336 warnings (0 errors) across the full codebase, and the CI is configured to exit with failure on warnings. None of the flagged files are modified by this PR — this PR only changes Evidence: All recent |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot It’s not working. Could you please investigate why Here is the console log for your change, {"distance":18,"unit":"km","distanceInMeters":18000,"customUnitRateID":"_FAKE_P2P_ID_","mileageRates":{},"amount":0} |
getMileageRates(policy) returns {} for P2P expenses (no workspace
policy), causing amount to always be 0. Use DistanceRequestUtils.getRate()
which correctly handles P2P via getRateForP2P() with
CURRENCY_TO_DEFAULT_MILEAGE_RATE.
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
Good catch — I found the root cause and pushed a fix. Root Cause: The amount calculation code was calling
So Fix: Replaced the manual rate lookup with Commit: 7099eaf |
|
@MelvinBot mark this PR as Ready for review. |
|
Done — PR has been marked as Ready for review. |
|
@ahmedGaber93 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] |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
| amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, mileageRate?.rate ?? 0); | ||
| } |
There was a problem hiding this comment.
@MelvinBot Since we now have the rate, can we also compute the Merchant instead of pending inside this condition?
There was a problem hiding this comment.
Good idea — pushed a fix in d5f0295. For manual distance in the skip-confirmation path, the merchant is now computed via DistanceRequestUtils.getDistanceMerchant() (e.g., 20 mi @ $0.67 / mi) instead of showing "Pending...".
Changes:
- Added
toLocaleDigitandgetCurrencySymboltoMoneyRequestStepDistanceNavigationParams(needed bygetDistanceMerchantfor locale-aware formatting) - Moved
setMoneyRequestMerchantafter the manual distance block so it uses the computed merchant - Updated
trackExpenseandcreateDistanceRequestcalls to use themerchantvariable - Updated
IOURequestStepDistanceManual.tsxto pass the new params fromuseLocalize()anduseCurrencyListActions()
For non-manual distance requests (GPS/waypoint-based), the merchant still falls back to "Pending..." since the route may not yet be resolved at creation time.
There was a problem hiding this comment.
@MelvinBot Please initialize toLocaleDigit and getCurrencySymbol following the existing code below, without passing them down from MoneyRequestStepDistanceNavigationParams:
App/src/libs/TransactionUtils/index.ts
Lines 739 to 740 in 6a08455
There was a problem hiding this comment.
Done — pushed in 3ab015e. Removed toLocaleDigit and getCurrencySymbol from MoneyRequestStepDistanceNavigationParams and initialized them locally using the same pattern as TransactionUtils:
(digit) => toLocaleDigit(IntlStore.getCurrentLocale(), digit)(imported from@libs/LocaleDigitUtils+@src/languages/IntlStore)getCurrencySymbol(imported from@libs/CurrencyUtils)
Also cleaned up the unused useCurrencyListActions import and toLocaleDigit destructuring from IOURequestStepDistanceManual.tsx.
Instead of hardcoding merchant to 'Pending...' when skip-confirmation is used for manual distance, compute the actual merchant string (e.g., '20 mi @ $0.67 / mi') using DistanceRequestUtils.getDistanceMerchant(). This matches the behavior of the normal confirmation flow in MoneyRequestConfirmationList. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
@ahmedGaber93 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] |
…tStepDistanceNavigationParams These properties are only needed for the manual distance path. Making them optional fixes typecheck errors in callers that don't deal with manual distance (GPS, Map, Odometer, and tests). Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
Fixed the failing typecheck: made |
|
The original failing check was TypeScript Checks — the This was already fixed in commit The remaining failure is test (job 7), which is an infrastructure issue — the runner got a |
|
The failing Analysis: The job failed at the Git was unable to authenticate against GitHub after 3 retries. This is a runner/credential issue, not a code issue. Evidence: All code-related checks (ESLint, Prettier, typecheck, builds) are passing. The Storybook test runner never reached the point of actually running tests — it failed during repository checkout. Action needed: Please re-run the failed |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
The previous commit removed the setMoneyRequestMerchant call which updated the transaction's merchant field in Onyx. This caused the existing test to fail because the Onyx transaction no longer had the merchant set. The merchant is now set after computing it — either 'Pending...' for map-based distance, or the calculated merchant string for manual distance. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
Fixed the failing Root cause: The previous commit removed the Fix: Re-added the The other failing check (Storybook tests) is a GitHub Actions infrastructure issue ( |
|
The two failing checks (
Neither failure is related to the changes in Action needed: Please re-run the failed workflows. These are transient infrastructure errors that should resolve on retry. |
|
The failing checks are unrelated to this PR's code changes — both are transient CI infrastructure failures:
Neither failure is related to the changes in |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari20260312060930049.mp4 |
|
@MelvinBot Mark this PR as ready for review. |
|
The failing checks are unrelated to this PR's code changes. Both are transient CI infrastructure failures:
Evidence: This PR only modifies Recommendation: Re-run the failed workflows. These are transient infrastructure failures that should resolve on retry. |
|
This PR is already marked as ready for review (it's not in draft state). |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #82801 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
src/libs/actions/IOU/MoneyRequest.ts
Outdated
| distanceInMeters, | ||
| unit, | ||
| mileageRate?.rate ?? 0, | ||
| mileageRate?.currency ?? transaction?.currency ?? 'USD', |
There was a problem hiding this comment.
Let's use the const for the default value here
| mileageRate?.currency ?? transaction?.currency ?? 'USD', | |
| mileageRate?.currency ?? transaction?.currency ?? CONST.CURRENCY.USD, |
There was a problem hiding this comment.
Done — applied in 78fe023.
Co-authored-by: Francois Laithier <francoisl@users.noreply.github.com>
|
I investigated the failing checks on this PR. Both failures were unrelated to the PR's code changes (
All checks are now green (perf-tests is still pending but unrelated). |
|
🚧 @francoisl has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/francoisl in version: 9.3.37-0 🚀
|
|
Deploy Blocker #85173 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.37-10 🚀
|
Explanation of Change
When creating a manual distance expense via QAB (Quick Action Button), the skip-confirmation path in
handleMoneyRequestStepDistanceNavigationhardcodedamount: 0for bothtrackExpenseandcreateDistanceRequest. For map-based distance expenses, the backend recalculates the amount fromvalidWaypoints/route data — but for manual distance,validWaypointsisundefined(no route exists). This meant the expense displayed $0.00, especially noticeable when offline since the backend never processes the request to correct it.This fix computes the amount from the manual distance and mileage rate before passing it to
trackExpense/createDistanceRequest, using the sameDistanceRequestUtilsfunctions (convertToDistanceInMeters,getMileageRates,getDefaultMileageRate,getDistanceRequestAmount) used elsewhere in the codebase (e.g.,IOURequestStepDistanceMap.setDistanceRequestDataandMoneyRequestConfirmationList).Fixed Issues
$ #82801
PROPOSAL: #82801 (comment)
Tests
createDistanceRequestpathOffline tests
QA Steps
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))npm run compress-svg)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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari