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.

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

Distance rates - Change Rate to default leads to app crash #49459

Open
1 of 6 tasks
IuliiaHerets opened this issue Sep 19, 2024 · 20 comments
Open
1 of 6 tasks

Distance rates - Change Rate to default leads to app crash #49459

IuliiaHerets opened this issue Sep 19, 2024 · 20 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Reviewing Has a PR in review

Comments

@IuliiaHerets
Copy link

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.38-0
Reproducible in staging?: Y
Reproducible in production?: N
Issue was found when executing this PR: #49194
Email or phone of affected tester (no customers): applausetester+bp0911del@app;ause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

Preconditions:

  1. Track tax is enabled.
  2. Workspace has at least two distance rates that have tax rate and tax reclaimable on
  3. Each distance rate has different tax rate

Steps to reproduce:

  1. Go to ws chat > Submit expense
  2. Choose Distance > enter waypoints
  3. Submit expense
  4. Go to expense details > click on Rate
  5. Choose Default Rate (0,67 / mile)

Expected Result:

User is navigated to expense details page. Chosen rate is displayed.

Actual Result:

Android app: application crashes

Web/mWeb/Desktop app: user stays on Rate tab. Default Rate is checked, but previous rate is highlighted. User need to click/tap on default rate again.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

1909_2.txt

Bug6608698_1726740199922.Screenrecorder-2024-09-19-12-52-06-277.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Sep 19, 2024
Copy link

melvin-bot bot commented Sep 19, 2024

Triggered auto assignment to @dylanexpensify (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.

Copy link

melvin-bot bot commented Sep 19, 2024

Triggered auto assignment to @carlosmiceli (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 19, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 19, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Sep 19, 2024

📣 @abhisharma9876! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

Copy link

melvin-bot bot commented Sep 19, 2024

📣 @aroopcodder! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #vip-bills

@dylanexpensify
Copy link
Contributor

reviewing

@grgia
Copy link
Contributor

grgia commented Sep 19, 2024

@carlosmiceli
Copy link
Contributor

@Krishna2323 @aimane-chnaif Can you look into this?

@carlosmiceli
Copy link
Contributor

@mountiny This would qualify as revert-worthy if we don't solve it soon, right?

@carlosmiceli
Copy link
Contributor

carlosmiceli commented Sep 19, 2024

These are the firebase crash logs:

Fatal Exception: com.facebook.react.common.JavascriptException: TypeError: Cannot read property 'value' of undefined, js engine: hermes, stack:
getModifiedExpenseOriginalMessage@1:2641368
buildOptimisticModifiedExpenseReportAction@1:2657263
getUpdateMoneyRequestParams@1:2928286
updateMoneyRequestDistanceRate@1:2935771
selectDistanceRate@1:6750319
t12@1:6750366
selectRow@1:5374828
anonymous@1:5376736
anonymous@1:4219664
onSelectRow@1:5376708
onPress@1:4199364
t23@1:4218512
anonymous@1:4219664
_performTransitionSideEffects@1:791974
_receiveSignal@1:791562
onResponderRelease@1:790391
executeDispatch@1:1250761
executeDispatchesAndReleaseTopLevel@1:1256047
forEachAccumulated@1:1252277
anonymous@1:1256485
batchedUpdatesImpl@1:1317425
batchedUpdates$1@1:1255971
dispatchEvent@1:1256250

at com.facebook.react.modules.core.ExceptionsManagerModule.reportException(ExceptionsManagerModule.java:65)
at com.facebook.jni.NativeRunnable.run(NativeRunnable.java)
at android.os.Handler.handleCallback(Handler.java:958)
at android.os.Handler.dispatchMessage(Handler.java:99)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
at android.os.Looper.loopOnce(Looper.java:205)
at android.os.Looper.loop(Looper.java:294)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:235)
at java.lang.Thread.run(Thread.java:1012)

@mountiny
Copy link
Contributor

@carlosmiceli yes probably worth to revert #49194

@Krishna2323
Copy link
Contributor

@mountiny, sorry for delay, give me a moment to check this.

@Krishna2323
Copy link
Contributor

@mountiny @carlosmiceli, should I raise a PR?

Here we added taxAmount and taxCode in transactionChanges so it is also sent to the backend through params, object. We only need to update the values optimistically so I think we can remove the taxAmount and taxCode properties from the params object. WDYT?

App/src/libs/actions/IOU.ts

Lines 3166 to 3192 in 040c801

function updateMoneyRequestDistanceRate(
transactionID: string,
transactionThreadReportID: string,
rateID: string,
policy: OnyxEntry<OnyxTypes.Policy>,
policyTagList: OnyxEntry<OnyxTypes.PolicyTagLists>,
policyCategories: OnyxEntry<OnyxTypes.PolicyCategories>,
taxAmount?: number,
taxCode?: string,
) {
const transactionChanges: TransactionChanges = {
customUnitRateID: rateID,
...(taxAmount ? {taxAmount} : {}),
...(taxCode ? {taxCode} : {}),
};
const allReports = ReportConnection.getAllReports();
const transactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`] ?? null;
const parentReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReport?.parentReportID}`] ?? null;
let data: UpdateMoneyRequestData;
if (ReportUtils.isTrackExpenseReport(transactionThreadReport) && ReportUtils.isSelfDM(parentReport)) {
data = getUpdateTrackExpenseParams(transactionID, transactionThreadReportID, transactionChanges, true, policy);
} else {
data = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, policy, policyTagList, policyCategories, true);
}
const {params, onyxData} = data;
API.write(WRITE_COMMANDS.UPDATE_MONEY_REQUEST_DISTANCE_RATE, params, onyxData);
}

@grgia
Copy link
Contributor

grgia commented Sep 19, 2024

@Krishna2323 yes, could you please raise a PR?

@Krishna2323
Copy link
Contributor

Okay, raising a PR in few minutes.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Hourly KSv2 labels Sep 19, 2024
@Krishna2323
Copy link
Contributor

PR is up, adding videos in few minutes.

@carlosmiceli
Copy link
Contributor

PR fix is now merged, thanks @Krishna2323 !

@carlosmiceli carlosmiceli added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Weekly KSv2 labels Sep 19, 2024
@dylanexpensify
Copy link
Contributor

Woooo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants
@carlosmiceli @mountiny @grgia @dylanexpensify @Krishna2323 @IuliiaHerets and others