[HOLD for payment 2024-12-16] [HOLD for payment 2024-11-11] [$250] Categories -Category name for spend is not updated offline & a new category is created instead

[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.47-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): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories
3. Open Travel category (because it is linked to Airlines spend in Settings)
4. Change the category name and save it
5. Click Settings
6. Note that the category for "Airlines" is also updated
7. Go offline
8. Open Travel category, rename and save it
9. Click Settings
10. Click Airlines

Expected Result:

Travel category will gray out and the name is updated when the category is updated offline

Actual Result:

Travel category is not grayed out and the name is not updated
A new category with the updated name is created down in the list

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

Bug6630360_1728539282760.20241010_134219.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021844365273557975639
• Upwork Job ID: 1844365273557975639
• Last Price Increase: 2024-10-17
• Automatic offers:
• huult | Contributor | 104555728

Issue Owner

Current Issue Owner: @eVoloshchak

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (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]

@bfitzexpensify 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/~021844365273557975639

[melvin-bot]

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

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-10-10 13:52:55 UTC.

Proposal

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

1. Travel category is not grayed out and the name is not updated
2. A new category with the updated name is created down in the list

What is the root cause of that problem?

1. We haven't updated optimistic data for policy's mccGroup
2. We're showing the currently selected category regardless of whether it has not been deleted or not. In this case, it's the default category corresponding to that group

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

Update optimistic data for mccGroup to change the corresponding category name when renamePolicyCategory:

const policyCategories = Object.keys(allPolicyCategories.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`]);
const mccGroup = policy.mccGroup;

const updatedMccGroups = lodashCloneDeep(mccGroup);

Object.values(mccGroup).forEach((group) => {
    if (policyCategories.includes(group.category)) {
        updatedMccGroups[group.groupID] = policyCategory.newName;
    }
})

...

optimisticData: [
    {
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
        value: {
            mccGroup: {
                ...updatedMccGroups,
            }
        },
    },
],

failureData: [
    {
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
        value: {
            mccGroup,
        },
    },
]

We might need to update mccGroup's pendingAction as well.

[huult]

Edited by proposal-police: This proposal was edited at 2024-10-10 17:25:54 UTC.

Proposal

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

Category name for spend is not updated offline & a new category is created instead

What is the root cause of that problem?

We do not update the category in the mccGroup of the policy when renaming the policy category. Therefore, when we open Settings, we retrieve the mccGroup data without updating the categories whose names have changed before.

App/src/libs/actions/Policy/Category.ts

Line 593 in d93f29a

key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,

App/src/libs/actions/Policy/Category.ts

Line 637 in d93f29a

key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,

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

We must update the category in the mccGroup of the policy when renaming the policy category. It should be something like this:

//src/libs/actions/Policy/Category.ts#L546
+  const mccGroup = policy?.mccGroup ?? [];
+  const updateMCCGroup = lodashCloneDeep(mccGroup);

//src/libs/actions/Policy/Category.ts#L600
value: {
                    rules: {
                        approvalRules: updatedApprovalRules,
                        expenseRules: updatedExpenseRules,
                    },
+                    mccGroup: CategoryUtils.getCategoryMccGroup(updateMCCGroup, policyCategory.oldName, policyCategory.newName),
                },
//src/libs/actions/Policy/Category.ts#L641
value: {
                    rules: {
                        approvalRules,
                    },
+                  mccGroup: CategoryUtils.getCategoryMccGroup(updateMCCGroup, policyCategory.oldName, policyCategory.newName)
                },
// .src/libs/CategoryUtils.ts#L60
+ function getCategoryMccGroup(mccGroup, oldCategoryName: string, newCategoryName: string) {
+    for (const key in mccGroup) {
+        if (mccGroup[key].category === oldCategoryName) {
+            mccGroup[key].category = newCategoryName;
+           mccGroup[key].pendingAction = 'update';
+        }
+    }

+    return mccGroup;
+}

POC

Screen.Recording.2024-10-11.at.01.27.29.mp4

[melvin-bot]

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

[bfitzexpensify]

Couple of proposals ready for review @eVoloshchak

[melvin-bot]

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

[melvin-bot]

@eVoloshchak, @bfitzexpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[eVoloshchak]

@mkzie2, thank you for the proposal, however, when testing it, I've encountered a couple of problems:

• The category name is not updated in Settings
• The app crashes when you click on an old category name in Settings

Screen.Recording.2024-10-21.at.00.16.49.mov

[eVoloshchak]

@huult proposal looks and tests well, let's proceed with it

🎀👀🎀 C+ reviewed!

[melvin-bot]

Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[huult]

@youssef-lr I look forward to receiving your assignment

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[huult]

same above

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify Still overdue 6 days?! Let's take care of this!

[huult]

@eVoloshchak Please review my pull request. Thank you.

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[huult]

@eVoloshchak Please review my pull request. Thank you.

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify 12 days overdue. Walking. Toward. The. Light...

[huult]

@eVoloshchak Could you review the pull request so it can be completed? It has been open for 3 weeks and involves a small change, so your prompt review would be appreciated.

[eVoloshchak]

Will give this a final round of testing today, apologies for the delay

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify Still overdue 6 days?! Let's take care of this!

[huult]

Waiting for @youssef-lr to review. #51927

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot]

@eVoloshchak, @youssef-lr, @huult, @bfitzexpensify 12 days overdue now... This issue's end is nigh!

[huult]

The pull request is already merged into the staging branch

[melvin-bot]

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

• fix gray-out and name issue in offline category update #51927

If no regressions arise, payment will be issued on 2024-12-16. 🎊

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

• @eVoloshchak requires payment through NewDot Manual Requests
• @huult requires payment automatic offer (Contributor)

[melvin-bot]

@eVoloshchak @bfitzexpensify @eVoloshchak The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[bfitzexpensify]

Payment complete to @huult.

@eVoloshchak, please complete the BZ checklist and I'll post the payment summary for the C+ work.

[bfitzexpensify]

Bumping this @eVoloshchak

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: there isn't a PR that has caused this, this was a functionality not implemented initially
• 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: N/A
• 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: I don't think an additional discussion is needed, we already have the necessary item in PR author/reviewer checklists

Regression Test Proposal

1. Navigate to Workspace Settings > Categories.
2. Open the Travel category (because it is linked to the Airlines spend in Settings).
3. Change the category name and click Save.
4. Click Settings and verify that the Airlines category name is updated.
5. Go offline.
6. Open the Travel category, rename it, and click Save.
7. Click Settings and then open the Airlines category.
8. Verify that the Travel category has the updated name and is grayed out.

Do we agree 👍 or 👎

[bfitzexpensify]

Thanks!

Payment summary:

@eVoloshchak due $250 for C+ work via ND manual request.

[JmillsExpensify]

$250 approved for @eVoloshchak