Open lanitochka17 opened 1 month ago
Triggered auto assignment to @greg-schroeder (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.
@greg-schroeder 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
We think that this bug might be related to #wave-collect - Release 1
Job added to Upwork: https://www.upwork.com/jobs/~0183948967c6155377
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External
)
Edited by proposal-police: This proposal was edited at 2024-08-07 15:46:57 UTC.
App crashes when deleting a distance expense when there are two distance expenses
When deleting a distance request tags become undefined here https://github.com/Expensify/App/blob/0a33186c80eb821d67d75b44bca527640204852c/src/libs/OptionsListUtils.ts#L1355-L1356
We should use nullish coalescing operator to return empty array when tags is undefined in Ojbect.values
const policyTagValueList = policyTagList.map(({tags}) => Object.values(tags ?? [])).flat();
Result:
https://github.com/user-attachments/assets/c84c2d01-fb82-4c0f-8bcd-20476a216ad6
The data we are working with in hasEnabledTags might be an array of objects, with the first key being Tag.
Because of this, destructuring {tags} in policyTagList returns undefined. https://github.com/Expensify/App/blob/0a33186c80eb821d67d75b44bca527640204852c/src/libs/OptionsListUtils.ts#L1356
In most cases, this doesn't cause problems because in:
we rely on isPolicyExpenseChat or transactionTag. However, when neither is present, it falls back to hasEnabledTags.
So why? I don't know, unfortunately, after clearing the cache I can no longer reproduce this bug. This fix might work, but I want to understand why the user might receive a list like the one in the screenshot. I'll keep investigating, as I think the problem might be much larger. Anyway, if I find anything, I'll let you know.
Edited by proposal-police: This proposal was edited at 2024-08-08 10:22:42 UTC.
The app crashes.
This issue occurs when the app's Onyx data contains report_undefined
data. Suppose the app has:
report_undefined: {reportName: "Chat Report"}
and we are on an expense report page with two distance expenses. In this scenario, the transactionThreadReportID
in:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/pages/home/ReportScreen.tsx#L342-L345
is undefined
. However, because report_undefined
has the value {reportName: "Chat Report"}
, the condition at:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/pages/home/report/ReportActionItemContentCreated.tsx#L151
is still true
, so the component at:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/pages/home/report/ReportActionItemContentCreated.tsx#L162-L165
is displayed in the expense report.
Consequently, the policyTagLists
data in:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/components/ReportActionItem/MoneyRequestView.tsx#L674-L674
might look like this (retrieving all tags data from all policies rather than just the target policy):
{
"policyTags_AAF1F0C56F4E3906": {
"Tag": {
"name": "Tag",
"orderWeight": 0,
"tags": []
}
},
"policyTags_59758A8741EE68F2": {
"Tag": {
"name": "Tag",
"orderWeight": 0,
"tags": []
}
}
}
Now, if the logic OptionsListUtils.hasEnabledTags(policyTagLists)
is executed at:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/components/ReportActionItem/MoneyRequestView.tsx#L212,
the app will crash. However, because isPolicyExpenseChat
is false
, OptionsListUtils.hasEnabledTags(policyTagLists)
is not called.
When the first expense is deleted, the transactionThreadReportID
is no longer undefined
, and the transactionThreadReport
is updated. At this point:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/components/ReportActionItem/MoneyRequestView.tsx#L212,
isPolicyExpenseChat
becomes true
, and since OptionsListUtils.hasEnabledTags(policyTagLists)
is called with outdated policyTagLists
data, the app crashes.
Ensure that there is no report_undefined
Onyx data. In my RCA, I noted that report_undefined
can appear when we call:
https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/pages/iou/request/step/IOURequestStepDistance.tsx#L217.
Update the following code: https://github.com/Expensify/App/blob/53eee53ac8bfbdfd4d8eeaf16940e3b165483373/src/pages/iou/request/step/IOURequestStepDistance.tsx#L217 to:
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_WAYPOINT.getRoute(action, CONST.IOU.TYPE.SUBMIT, transactionID, report?.reportID ?? reportID, index.toString(), Navigation.getActiveRoute()));
In this update, I added the fallback value report?.reportID ?? reportID
to ensure the report parameter in ROUTES.MONEY_REQUEST_STEP_WAYPOINT
is not undefined
.
{transactionThreadReport.reportID && transactionThreadReport && !isEmptyObject(transactionThreadReport) ? (
@mollfpr, @greg-schroeder Eep! 4 days overdue now. Issues have feelings too...
@mollfpr can you review this proposal, please?
Yes, awaiting proposal review
Reviewing π
@etCoderDysto @dominictb Do you guys still able to reproduce the issue?
@mollfpr I am not able to reproduce this.
@mollfpr Actually, with the OP's reproduce steps, I cannot reproduce. But based on my RCA, I still can reproduce the bug if there is report_undefined
key in onyx.
To create a report_undefined
key in onyx, we can just go to this url https://dev.new.expensify.com:8082/r/undefined
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@dominictb I didn't get the crash on staging with the above suggestion.
@mollfpr Here is the video from my side:
https://github.com/user-attachments/assets/8330c078-a65b-441d-9938-61dbf15c6677
@dominictb I can reproduce the issue now and it seems your RCA is correct.
We can go with @dominictb proposal's but using both main and alternative solution.
π π π C+ reviewed!
Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@mollfpr @greg-schroeder @aldo-expensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
π£ @dominictb π 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 π
Just want to note the reproduce steps for this bug:
PR is in review, next up is @mollfpr!
PR still going through review
Bump @aldo-expensify I believe you're up for a re-review here?
Thank you, I had missed the notification before. PR merged.
This issue has not been updated in over 15 days. @mollfpr, @greg-schroeder, @aldo-expensify, @dominictb eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
PR hit staging yesterday, Melv.
Just awaiting deploy to prod!
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: .0.17-1 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 Email or phone of affected tester (no customers): sarahhailu1268+dhsdfsdhgshg@gmail.com Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
The app will not crash and the expense is successfully deleted
Actual Result:
App crashes
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/57f7d4e0-f012-4cb0-9cae-b8bd42181cce
logs (2).txt
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @dominictb