Skip to content

[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 #50563

Closed

Description

@lanitochka17

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.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): applausetester+pso@applause.expensifail.com
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 OwnerCurrent Issue Owner: @eVoloshchak

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Oct 10, 2024
  2. melvin-bot commented on Oct 10, 2024

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

  3. lanitochka17 commented on Oct 10, 2024

    @lanitochka17
    Author

    @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

  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Oct 10, 2024
  5. changed the title [-]Categories -Category name for spend is not updated offline & a new category is created instead[/-] [+][$250] Categories -Category name for spend is not updated offline & a new category is created instead[/+] on Oct 10, 2024
  6. melvin-bot commented on Oct 10, 2024

    @melvin-bot
  7. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Oct 10, 2024
  8. melvin-bot commented on Oct 10, 2024

    @melvin-bot

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

  9. mkzie2 commented on Oct 10, 2024

    @mkzie2
    Contributor

    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.

  10. huult commented on Oct 10, 2024

    @huult
    Contributor

    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.

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

    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;
    +}
    DetailsPOC
    Screen.Recording.2024-10-11.at.01.27.29.mp4
  11. 57 remaining items

  12. melvin-bot commented on Dec 9, 2024

    @melvin-bot

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

  13. huult commented on Dec 9, 2024

    @huult
    Contributor

    The pull request is already merged into the staging branch

  14. changed the title [-][HOLD for payment 2024-11-11] [$250] Categories -Category name for spend is not updated offline & a new category is created instead[/-] [+][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[/+] on Dec 9, 2024
  15. melvin-bot commented on Dec 9, 2024

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

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

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

  16. melvin-bot commented on Dec 9, 2024

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

  17. bfitzexpensify commented on Dec 16, 2024

    @bfitzexpensify
    Contributor

    Payment complete to @huult.

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

  18. bfitzexpensify commented on Dec 24, 2024

    @bfitzexpensify
    Contributor

    Bumping this @eVoloshchak

  19. eVoloshchak commented on Dec 29, 2024

    @eVoloshchak
    Contributor
    • 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 👎

  20. bfitzexpensify commented on Dec 30, 2024

    @bfitzexpensify
    Contributor

    Thanks!

    Payment summary:

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

  21. JmillsExpensify commented on Jan 14, 2025

    @JmillsExpensify
    Contributor

    $250 approved for @eVoloshchak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorWeeklyKSv2

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions