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

Expense - The app crashes when the administrator opens the details of a paid expense #47940

Closed
3 of 6 tasks
lanitochka17 opened this issue Aug 23, 2024 · 9 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 23, 2024

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: 9.0.24-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: https://expensify.testrail.io/index.php?/tests/view/4888153
Email or phone of affected tester (no customers): sustinov@applausemail.com
Issue reported by: Applause - Internal Team

Action Performed:

Create a new HT administrator account, create a new WS and invite a new HT employee account. For convenience, it is better to perform the steps in two tabs opened simultaneously.
Step:

  1. Open https://staging.new.expensify.com/
  2. Log in under the employee's account
  3. Submit any manual expense in the WS room
  4. Log in under the administrator account
  5. Click on the expense payment button “Pay amount elsewhere”
  6. Quickly click on the expense itself to open the details
  7. Repeat steps 2-6 several times.

Expected Result:

The details of the paid expense should be opened by the adminstrator without crashing the application

Actual Result:

The app crashes when the administrator opens the details of a paid expense

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

Bug6580538_1724435181382.Recording__117__2_.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 23, 2024
Copy link

melvin-bot bot commented Aug 23, 2024

Triggered auto assignment to @Christinadobrzyn (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
Copy link
Author

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

@lanitochka17
Copy link
Author

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

@FitseTLT
Copy link
Contributor

FitseTLT commented Aug 23, 2024

Edited by proposal-police: This proposal was edited at 2024-08-23 18:52:49 UTC.

Proposal

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

The app crashes when the administrator opens the details of a paid expense

What is the root cause of that problem?

The root cause is here we are falling back an empty string for report.policyID here which will fetch the whole policy tag collection

policyTagList: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report?.policyID ?? ''}`,

Then policyTagsList will have array elements which are not objects with tag prop so here we call hasEnabledTags
const shouldShowTag = isPolicyExpenseChat && (transactionTag || OptionsListUtils.hasEnabledTags(policyTagLists));
const shouldShowBillable = isPolicyExpenseChat && (!!transactionBillable || !(policy?.disabledFields?.defaultBillable ?? true) || !!updatedTransaction?.billable);

hasEnabledTags expects all array elements to be in that structure so calls Object.value on tag prop of the array which doesn't exist here
const policyTagValueList = policyTagList.map(({tags}) => Object.values(tags)).flat();

So TypeError Cannot Conver null or undefined to object occurs and the app crashes

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

Fallback should be something like -1

        key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report?.policyID ?? '-1'}`,

We should similarly replace all other empty string fallbacks in other places too

What alternative solutions did you explore? (Optional)

@abzokhattab
Copy link
Contributor

I'm curious why the fallback values in this file are set to empty strings.

policy: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report?.policyID ?? ''}`,
},
policyCategories: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${report?.policyID ?? ''}`,
},
policyTagList: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report?.policyID ?? ''}`,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID ?? ''}`,
},

This change was introduced in this PR, so I'm tagging @kubabutkiewicz and @parasharrajat since they worked on it. Was this change intentional?

@parasharrajat
Copy link
Member

parasharrajat commented Aug 23, 2024

@abzokhattab Probably a mistake. Default is -1 but it expects a string so mistakenly we left '' string instead.

@abzokhattab
Copy link
Contributor

abzokhattab commented Aug 23, 2024

Thanks for clarifying, I thought there is a reason behind it, that's why I asked to avoid breaking intended functionalities.

@bernhardoj
Copy link
Contributor

Dupe of #47542

@Christinadobrzyn
Copy link
Contributor

Awesome, thanks @bernhardoj - closing as a dupe.

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
Projects
None yet
Development

No branches or pull requests

6 participants