[HOLD for payment 2024-07-10] [$250] Category - "Category no longer valid" shows up briefly after changing amount

[izarutskaya]

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: 1.4.83-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• "Members must categorize all expenses" in Categories is enabled.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit an expense with category.
4. Go to transaction thread.
5. Click Amount.
6. Change amount and save it.

Expected Result:

There will be no violation under Category after changing amount because Category is already selected.

Actual Result:

"Category no longer valid" shows up briefly after changing amount when the selected category is valid.

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

Bug6513040_1718362398903.bandicam_2024-06-14_18-44-14-234.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017067ba3b0cc2101a
• Upwork Job ID: 1803116505548368516
• Last Price Increase: 2024-06-18
• Automatic offers:
• allgandalf | Contributor | 102815523
• truph01 | Contributor | 102828819

Issue Owner

Current Issue Owner: @OfstadC

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #collect project.

[Nodebrute]

Proposal

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

"Category no longer valid" shows up briefly after changing amount

What is the root cause of that problem?

We are not passing policyCategories here

App/src/pages/iou/request/step/IOURequestStepAmount.tsx

Line 305 in f0b6355

IOU.updateMoneyRequestAmountAndCurrency({transactionID, transactionThreadReportID: reportID, currency, amount: newAmount, taxAmount, policy, taxCode});

so in here policyCategories will be null

App/src/libs/actions/IOU.ts

Line 5240 in 07b6b24

data = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, policy ?? null, policyTagList ?? null, policyCategories ?? null, true);

So that's why when here we check for policyCategories?.[categoryKey]?.enabled it will be undefined

App/src/libs/Violations/ViolationsUtils.ts

Line 190 in 07b6b24

const isCategoryInPolicy = categoryKey ? policyCategories?.[categoryKey]?.enabled : false;

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

We should pass policyCategories here

App/src/pages/iou/request/step/IOURequestStepAmount.tsx

Line 305 in f0b6355

IOU.updateMoneyRequestAmountAndCurrency({transactionID, transactionThreadReportID: reportID, currency, amount: newAmount, taxAmount, policy, taxCode});

Additionally we can also pass policyTags

We can do something like this

        IOU.updateMoneyRequestAmountAndCurrency({transactionID, transactionThreadReportID: reportID, currency, amount: newAmount, taxAmount, policy,policyTags,policyCategories, taxCode});

Note: This is just pseudocode.

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

We can also fix other instances of this to ensure we are passing all the data.

What alternative solutions did you explore? (Optional)

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[OfstadC]

I'm not able to reproduce on staging

I've tried different currencies, having a tag field, no tag field. Clicking in to edit the expense a few different ways - but I never see the category error
Most recent attempt:
https://github.com/Expensify/App/assets/11722379/90d898f4-5194-489b-bf8b-c3b11e0f5a27

[Nodebrute]

Hey @OfstadC, It's easily reproducible on staging. Try it with this precondition.
Precondition: "Members must categorize all expenses" in Categories is enabled.

[OfstadC]

Let me give it another shot! 😃

[OfstadC]

I missed the "...must categorize all expenses" detail 🤦‍♀️ . It is very reproducible 😅

[truph01]

Proposal

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

"Category no longer valid" shows up briefly after changing amount when the selected category is valid.

What is the root cause of that problem?

We always call getViolationsOnyxData when we update any money request field although we only get the violation for tag and category.

App/src/libs/actions/IOU.ts

Line 2697 in 3e1d94a

if (policy && PolicyUtils.isPaidGroupPolicy(policy) && updatedTransaction) {

App/src/libs/actions/IOU.ts

Line 2700 in 3e1d94a

ViolationsUtils.getViolationsOnyxData(

And then because we don't pass policyCategories here, this function returns the wrong violation for the category field.

App/src/pages/iou/request/step/IOURequestStepAmount.tsx

Line 305 in 3e1d94a

IOU.updateMoneyRequestAmountAndCurrency({transactionID, transactionThreadReportID: reportID, currency, amount: newAmount, taxAmount, policy, taxCode});

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

We can see that getViolationsOnyxData function only updates the violation for tag and category so we should only call this function if the tag or category is changed. With this check, we don't need to pass policyCategories or policyTagList when we update the other fields like amount, currency, description,... It also means we don't need to subscribe to policyCategories in the component just to pass it in the update function.

if (policy && PolicyUtils.isPaidGroupPolicy(policy) && updatedTransaction && ('tag' in transactionChanges || 'category' in transactionChanges)) {

App/src/libs/actions/IOU.ts

Line 2697 in 3e1d94a

if (policy && PolicyUtils.isPaidGroupPolicy(policy) && updatedTransaction) {

What alternative solutions did you explore? (Optional)

[OfstadC]

Added to Wave Collect 😃

[JmillsExpensify]

Nice, this is indeed #wave-control.

[JmillsExpensify]

I'd open it up to the community.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017067ba3b0cc2101a

[melvin-bot]

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

[OfstadC]

@thesahindia Could you please provide an update by EOD? Thank you!

[thesahindia]

@OfstadC, I won't be able to review this as I will be unavailable until Monday.

Asking for help in the channel.

[thesahindia]

@allgandalf, will be reviewing this. Please assign them @OfstadC.

[melvin-bot]

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

[allgandalf]

@truph01 , let us know when can we expect the PR, this looks a easy fix, lets get this merged by early next week

[truph01]

@allgandalf PR #44231 is ready

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: Category no longer valid shows up #44231

If no regressions arise, payment will be issued on 2024-07-10. 🎊

For reference, here are some details about the assignees on this issue:

• @allgandalf requires payment automatic offer (Contributor)
• @truph01 requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allgandalf] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allgandalf] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allgandalf] Determine if we should create a regression test for this bug.
• [@allgandalf] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@OfstadC] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/411747

[OfstadC]

@allgandalf Could you please update this today based on the checklist? Thank you! 😃

[allgandalf]

• The PR that introduced the bug has been identified. Link to the PR: Client-side violations for money request updates #34402

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/34402/files#r1671860609

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

• Precondition : "Members must categorize all expenses" in Categories is enabled.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit an expense with category.
4. Go to transaction thread.
5. Click Amount.
6. Change amount and save it.
7. Verify that there is no violation under Category after changing amount because Category is already selected.

Do we agree 👍 or 👎

[OfstadC]

Payment Summary:
Contributor: @allgandalf paid $250 via Upwork
Contributor: @truph01 paid $250 via Upwork

[OfstadC]

Leaving this open until regression testing is complete

[OfstadC]

Same - waiting on regression testing

[melvin-bot]

@deetergp, @OfstadC, @allgandalf, @truph01 Eep! 4 days overdue now. Issues have feelings too...

[allgandalf]

Chill a little melvin

[OfstadC]

I'm actually going to close this out. but I'll note here if the regression testing yields anything 😄