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.

Sign up for GitHub

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

Jump to bottom

fix IOU with category can be created when admin disable the same category #35540

Merged
merged 4 commits into from
Feb 14, 2024
Merged

fix IOU with category can be created when admin disable the same category #35540

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69e9292
fix IOU with category can be created when admin disable the same cate…
dukenv0307 Feb 1, 2024
b30aa57
Merge branch 'main' into fix/35236
dukenv0307 Feb 2, 2024
c93e6d0
Merge branch 'main' into fix/35236
dukenv0307 Feb 7, 2024
04d3ae9
Merge branch 'main' into fix/35236
dukenv0307 Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/pages/iou/request/step/IOURequestStepConfirmation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ function IOURequestStepConfirmation({
IOU.setMoneyRequestBillable_temporaryForRefactor(transactionID, defaultBillable);
}, [transactionID, defaultBillable]);

useEffect(() => {
if (!transaction.category) {
return;
}
if (policyCategories[transaction.category] && !policyCategories[transaction.category].enabled) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App crashes on this line

Screenshot 2024-02-16 at 4 12 39 AM

Copy link
Contributor

@c3024 c3024 Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. @situchan

Till the last commit of this PR

https://github.com/dukenv0307/App/blob/c93e6d05da9c42dc074cfc699dae75e9138bd47e/src/pages/iou/request/step/IOURequestStepConfirmation.js#L70

policyCategories has a default value of {}. Last main merge changed it to null. My review was not marked as stale. I could not check again after this commit before this PR got merged.

@dukenv0307 Could you raise a follow-up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll raise a PR soon

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As no response for 4 days, I raised fix - #36858
This is now 🔥 as app crashes on main user flow and reached production.
@c3024 can you please review?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gonals @yuwenmemon, the crash was raised while the PR was in staging. It would have been ideal to create a deploy blocker to avoid this crash in production, especially now when we want a stable App version for the earnings call.

@c3024 @situchan, please raise a bug in #expensify-bugs when similar situations happen to have someone create a deploy blocker to avoid crashes into production in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting though, that Applause didn't report the crash as happening in production 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, you have to perform the reproduction steps real quick, so maybe it's not a big problem that it is in production, I think it's a rare case

IOU.resetMoneyRequestCategory_temporaryForRefactor(transactionID);
}
}, [policyCategories, transaction.category, transactionID]);
const defaultCategory = lodashGet(
_.find(lodashGet(policy, 'customUnits', {}), (customUnit) => customUnit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE),
'defaultCategory',
Expand Down
Loading