[$250] [CRITICAL] [CVP] Track Expense - App crashes when tracking an expense

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.17-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: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #43163

Action Performed:

1. Sign in with a new account
2. Go to FAB > Track Expense

Expected Result:

Track expense modal opens and user is able to track an expense

Actual Result:

App crashes

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6563076_1722927494306.video_2024-08-06_09-55-47.mp4

logs (2).txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019f114a28e4f405b1
• Upwork Job ID: 1820938917945788496
• Last Price Increase: 2024-08-13
• Automatic offers:
• ishpaul777 | Contributor | 103522001

[melvin-bot]

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

[lanitochka17]

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

[lanitochka17]

@alexpensify 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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019f114a28e4f405b1

[melvin-bot]

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

[trjExpensify]

@thienlnam here's another track expense flow bug, this time a crash and also seems to be referencing policyID -1 in the console error like #46338 (comment)

[jp928]

Not reproducible on latest code base.

[trjExpensify]

@danielrvidal how are you repro'ing this as soon as yesterday?

[danielrvidal]

I'm reproducing it on production still as of now. @jp928 did you try it on mobile web?

I'm on Safari, private tab if it helps at all

[jp928]

@danielrvidal
I tried with a new created account still fine to me. The console log shows it is a javascript error so it doesn't matter which browser it should reproducible cross all browsers. However, in the error the dodgy value policy_FAKE_ is the key of this bug.
I would assume it is a testing policy data for testing accounts. Sorry I couldn't test with your internal testing email boxes, but I would suggest to contact your backend team to investigate if you are adding special policy value to testing accounts, especially for the value of policy_FAKE_.

[danielrvidal]

in the error the dodgy value policy_FAKE_

@trjExpensify any idea where this would be coming from? I would think it should be fake since I was testing in production?

[melvin-bot]

@alexpensify, @thesahindia Huh... This is 4 days overdue. Who can take care of this?

[alexpensify]

Waiting for proposals here

[jp928]

Edited by proposal-police: This proposal was edited at 2024-08-13 11:06:39 UTC.

Proposal

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

Javascript throws fatal exception if the onyx collection key contains underscore, such as policy__FAKE_

What is the root cause of that problem?

In the code
https://github.com/Expensify/react-native-onyx/blob/397c2cba8fe5c2bdd5f0e6eaa6761043f1bf51a6/lib/OnyxUtils.ts#L409C3-L409C50

The last index of underscore is using for check if the key is valid so that policy__FAKE_ will be treated as invalid key.
Test case is like below:

function splitCollectionMemberKey(key) {
  let underscoreIndex = key.lastIndexOf('_');
  if (underscoreIndex + 1 === key.length) {
     underscoreIndex = key.lastIndexOf('__');
  }

  if (underscoreIndex === -1) {
      throw new Error(`Invalid ${key} key provided, only collection keys are allowed.`);
  };

  const collectionKey = key.substring(0, underscoreIndex + 1);
  const memberKey = key.substring(underscoreIndex + 1);
  return [collectionKey, memberKey];
}

The function isCollectionMemberKey will return false in this case.

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

Use indexOf instead of lastIndexOf to support the key contains multiple underscores, such as policy__FAKE_

What alternative solutions did you explore? (Optional)

Change the policy__FAKE_ from backend, such as policy_FAKE

[trjExpensify]

@trjExpensify any idea where this would be coming from? I would think it should be fake since I was testing in production?

Yeah, that's used for off-workspace DMs:

App/src/libs/ReportUtils.ts

Lines 1178 to 1179 in ff2b888

	* Checks if the supplied report belongs to workspace based on the provided params. If the report's policyID is _FAKE_ or has no value, it means this report is a DM.
	* In this case report and workspace members must be compared to determine whether the report belongs to the workspace.

[trjExpensify]

Thanks, @jp928. @thesahindia can you review that proposal? Also, CC'ing @hannojg for eyes from the blame. :)

[hannojg]

@jp928 trying to understand your proposal I don't think this really returns 'collection', 'member', right? What does it actually return?

[hannojg]

Hm, we are having these test cases in onyx which expect the underscore for the collection to be at the end. These tests are failing with your proposal:

[hannojg]

While it may not be perfect we maybe want to consider handling the case for double __ or for FAKE explicitly

[jp928]

While it may not be perfect we maybe want to consider handling the case for double __ or for FAKE explicitly

@hannojg SRY for confusing you. I will update my proposal. Cheers.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[trjExpensify]

@thesahindia can you chime in on the proposal please?

[melvin-bot]

📣 @ishpaul777 🎉 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 📖

[ishpaul777]

this has same root cause with #46805, A PR was deployed to production #46956 for fix. Can we request retest again?

[trjExpensify]

Can't reproduce on mWeb Safari anymore myself, @danielrvidal can try for good measure and then we can close.

[danielrvidal]

I can't reproduce it either. Sorry, I didn't mean to close it. But I can't repro either so maybe we close it?

[trjExpensify]

Yep, seems that linked PR fixed it. Closing!