[HOLD for payment 2024-03-25] [$500] Can't unselect category when editing a split bill #37146

m-natarajan · 2024-02-23T16:18:16Z

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: 1.4.43-14
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @DylanDylann
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1708412951140219

Action Performed:

Go to workspace expense chat
Click + button beside to composer
Click Split Bill
Click Scan tab
Choose a receipt (any image)
Go to confirmation page
Enter a category
Click Split
Unselect category

Expected Result:

Category should be removed

Actual Result:

Category still remain

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Screen.Recording.2024-02-20.at.14.05.21.mov

Recording.2774.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~018323df92a9ee320a
Upwork Job ID: 1761062942437457920
Last Price Increase: 2024-03-08

melvin-bot · 2024-02-23T16:18:20Z

Job added to Upwork: https://www.upwork.com/jobs/~018323df92a9ee320a

melvin-bot · 2024-02-23T16:18:25Z

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

melvin-bot · 2024-02-23T16:18:28Z

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

dukenv0307 · 2024-02-23T16:21:13Z

Proposal

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

What is the root cause of that problem?

We are using this logic to unselect a category

App/src/pages/iou/request/step/IOURequestStepCategory.js

Lines 90 to 91 in c71387c

    
           const isSelectedCategory = category.searchText === transactionCategory; 
        
           const updatedCategory = isSelectedCategory ? '' : category.searchText;

But in here

App/src/pages/iou/request/step/IOURequestStepCategory.js

Line 95 in c71387c

    
           IOU.setDraftSplitTransaction(transaction.transactionID, {category: category.searchText});

We don't use updatedCategory

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

We should use updatedCategory like this

IOU.setDraftSplitTransaction(transaction.transactionID, {category: updatedCategory});

What alternative solutions did you explore? (Optional)

brandonhenry · 2024-02-24T05:31:03Z

this one is a regression from 0d3a9f4 @dukenv0307 was the owner on that one

dukenv0307 · 2024-02-24T05:38:39Z

@brandonhenry It is not a regression as mentioned here

cc @DylanDylann

DylanDylann · 2024-02-24T05:49:30Z

Hi everyone, I saw this bug while reviewing this PR. I agree that this bug happened before and It isn't caused by this PR. But I am not sure whether it's an intentional or a bug. It is reason why I reported new bug for it

DylanDylann · 2024-02-24T05:51:48Z

I have more context here, If we need to speed up this issue, I could take over this one

wildan-m · 2024-02-24T06:18:14Z

@DylanDylann how can we make the category mandatory? I've asked it here, but haven't received an answer yet.

What I've tried: Created a control workspace in oldot and enabled the must-use category, but with no success.

wildan-m · 2024-02-25T16:44:51Z

Proposal

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

Unable to deselect options while editing a split bill. The options may include category or tag.

What is the root cause of that problem?

For the category issue, we are currently utilizing category.searchText instead of updatedCategory for updating the category.

App/src/pages/iou/request/step/IOURequestStepCategory.js

Lines 94 to 98 in 5eed632

    
           if (isEditingSplitBill) { 
        
               IOU.setDraftSplitTransaction(transaction.transactionID, {category: category.searchText}); 
        
               navigateBack(); 
        
               return; 
        
           }

For the tag, the problem lies in our use of transaction instead of splitDraftTransaction.

App/src/pages/iou/request/step/IOURequestStepTag.js

Lines 70 to 71 in 5eed632

    
           const transactionTag = TransactionUtils.getTag(transaction); 
        
           const tag = TransactionUtils.getTag(transaction, tagIndex);

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

We can revise this line:

App/src/pages/iou/request/step/IOURequestStepCategory.js

Line 95 in 5eed632

    
           IOU.setDraftSplitTransaction(transaction.transactionID, {category: category.searchText});

to

            IOU.setDraftSplitTransaction(transaction.transactionID, {category: updatedCategory});

And modify the transaction here:

App/src/pages/iou/request/step/IOURequestStepTag.js

Lines 70 to 71 in 5eed632

    
           const transactionTag = TransactionUtils.getTag(transaction); 
        
           const tag = TransactionUtils.getTag(transaction, tagIndex);

to

    const transactionToUse = isEditing && isSplitBill ? splitDraftTransaction : transaction;
    const transactionTag = TransactionUtils.getTag(transactionToUse);
    const tag = TransactionUtils.getTag(transactionToUse, tagIndex);

Where splitDraftTransaction is

        splitDraftTransaction: {
            key: ({route}) => {
                const transactionID = lodashGet(route, 'params.transactionID', 0);
                return `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`;
            },
        },

This might also need to be implemented for taxes, but I have noticed that the tax functionality is not working correctly yet. I frequently encountered a "hmm not found" error when selecting the tax.

Branch to test.

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-02-27T15:16:36Z

@puneetlath, @eVoloshchak Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

puneetlath · 2024-02-28T14:54:03Z

@eVoloshchak thoughts on the proposals?

dukenv0307 · 2024-02-28T16:53:07Z

@eVoloshchak My proposal is enough to fix the original bug. But in @wildan-m's proposal, he mentioned a completely new bug. The bug in the tag page is that we can't update the new tag in the split bill page. The solution for this new bug in @wildan-m's proposal will cause the regression like this one

If we also decide to fix new bug in here. This is my suggestion

   // const transactionTag = TransactionUtils.getTag(transaction);  // REMOVE
    // const tag = TransactionUtils.getTag(transaction, tagIndex); // REMOVE
    // const isSplitBill = iouType === CONST.IOU.TYPE.SPLIT;   // REMOVE
    const isEditing = action === CONST.IOU.ACTION.EDIT;
    const isEditingSplitBill = isEditing && iouType === CONST.IOU.TYPE.SPLIT;
    const {tag: transactionTag} = ReportUtils.getTransactionDetails(isEditingSplitBill && !lodashIsEmpty(splitDraftTransaction) ? splitDraftTransaction : transaction);     
// splitDraftTransaction is import from ONYX

And using transactionTag instead of tag in all places

wildan-m · 2024-03-01T01:54:53Z

@dukenv0307 Thanks for your concern. I can't reproduce this category regression with tag, so provide empty check for tag might be optional.

If the OP knows that this similar unselect issue occured for tag as well, I believe He/She will include it in the bug description. For me it does make sense to solve similar issue in one PR, similar to what we have done here.

eVoloshchak · 2024-03-01T15:47:35Z

Unable to test this properly due to a bug on split page, the whole page is flickering and category isn't possible to select
Will circle back to this tomorrow to see if the bug is resolved so I can properly review this

Screen.Recording.2024-03-01.at.16.45.13.mov

melvin-bot · 2024-03-01T16:00:03Z

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

eVoloshchak · 2024-03-04T22:07:43Z

For the tag, the problem lies in our use of transaction instead of splitDraftTransaction.

@wildan-m, the behavior is different than for category, this does look like a separate bug, I think we should handle it in a separate issue

I think we should proceed with @dukenv0307's proposal

🎀👀🎀 C+ reviewed!

melvin-bot · 2024-03-04T22:07:53Z

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

dukenv0307 · 2024-03-07T06:57:55Z

@puneetlath Please help to take a look at this one when you have a chance

melvin-bot · 2024-03-12T22:02:49Z

❌ There was an error making the offer to @dukenv0307 for the Contributor role. The BZ member will need to manually hire the contributor.

melvin-bot · 2024-03-18T16:11:14Z

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

melvin-bot · 2024-03-18T16:11:17Z

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

fix: unselect category when editing a split bill #38205

If no regressions arise, payment will be issued on 2024-03-25. 🎊

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

@eVoloshchak requires payment through NewDot Manual Requests
@dukenv0307 requires payment (Needs manual offer from BZ)
@DylanDylann requires payment (Needs manual offer from BZ)

melvin-bot · 2024-03-18T16:11:18Z

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:

[@eVoloshchak / @dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
[@eVoloshchak / @dukenv0307] 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:
[@eVoloshchak / @dukenv0307] 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:
[@eVoloshchak / @dukenv0307] Determine if we should create a regression test for this bug.
[@eVoloshchak / @dukenv0307] 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.
[@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

puneetlath · 2024-03-22T15:49:00Z

@eVoloshchak friendly reminder about the checklist so that we can pay on Monday.

puneetlath · 2024-03-25T14:59:14Z

@dukenv0307 offer is here: https://www.upwork.com/nx/wm/offer/101566868

@eVoloshchak still waiting on the checklist.

eVoloshchak · 2024-03-25T15:02:16Z

The PR that introduced the bug has been identified. Link to the PR: [CP Staging] Make sure that split bills follow the correct routes for editing category/tag #32541
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/32541/files#r1544750475
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, this is a simple bug that was missed during testing, I think a regression test is what's necessary here

eVoloshchak · 2024-03-25T15:04:05Z

Regression Test Proposal

Go to workspace expense chat
Click + button beside the composer
Click Split Bill
Click Scan tab
Choose a receipt (any image)
Go to confirmation page
Enter a category
Click Split
Click on the created IOU request -> Category
Unselect the category
Verify that category is removed

Do we agree 👍 or 👎

puneetlath · 2024-03-25T16:20:23Z

(resetMoneyRequestCategory_temporaryForRefactor) it was planned to fix this in the subsequent PR(s), but that wasn't the case

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

Can we comment on the PR in that case? It's not ideal to create a temporary method and then never finish it. Especially since it led to a bug.

puneetlath · 2024-03-27T19:07:15Z

@eVoloshchak bump!

eVoloshchak · 2024-03-29T18:21:32Z

Apologies, I was wrong about the PR that has caused this.
As discussed in https://github.com/Expensify/App/pull/35458/files#r1494844943, this is the old logic carried over from EditSplitBillPage. Updated the checklist

puneetlath · 2024-04-01T14:57:15Z

Payment summary:

C - $500 - @dukenv0307 - paid on Upwork
C+ - $500 - @eVoloshchak - to be paid on NewDot

@eVoloshchak please request on NewDot. Thanks everyone!

JmillsExpensify · 2024-04-01T17:15:16Z

$500 approved for @eVoloshchak

dukenv0307 · 2024-10-08T15:07:46Z

C - $500 - @dukenv0307 - paid on Upwork

@puneetlath Looks like the contract for me is still not paid out, can you kindly double check?

puneetlath · 2024-10-08T15:22:07Z

Weird, you're right. I went ahead and paid it.

dukenv0307 · 2024-10-08T15:34:24Z

@puneetlath Thank you 🙏

m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 23, 2024

melvin-bot bot changed the title ~~Can't unselect category when editing a split bill~~ [$500] Can't unselect category when editing a split bill Feb 23, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 23, 2024

melvin-bot bot assigned eVoloshchak Feb 23, 2024

melvin-bot bot assigned puneetlath Feb 23, 2024

melvin-bot bot added the Overdue label Feb 25, 2024

melvin-bot bot removed the Overdue label Feb 28, 2024

melvin-bot bot added the Overdue label Mar 4, 2024

melvin-bot bot removed the Overdue label Mar 4, 2024

melvin-bot bot removed the Overdue label Mar 12, 2024

puneetlath assigned dukenv0307 Mar 12, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 12, 2024

dukenv0307 mentioned this issue Mar 13, 2024

fix: unselect category when editing a split bill #38205

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 Weekly KSv2 labels Mar 13, 2024

melvin-bot bot changed the title ~~[$500] Can't unselect category when editing a split bill~~ [HOLD for payment 2024-03-25] [$500] Can't unselect category when editing a split bill Mar 18, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Mar 18, 2024

eVoloshchak mentioned this issue Mar 29, 2024

[CP Staging] Make sure that split bills follow the correct routes for editing category/tag #32541

Merged

46 tasks

puneetlath closed this as completed Apr 1, 2024

[HOLD for payment 2024-03-25] [$500] Can't unselect category when editing a split bill #37146

[HOLD for payment 2024-03-25] [$500] Can't unselect category when editing a split bill #37146

Comments

m-natarajan commented Feb 23, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Feb 23, 2024

melvin-bot bot commented Feb 23, 2024

melvin-bot bot commented Feb 23, 2024

dukenv0307 commented Feb 23, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

brandonhenry commented Feb 24, 2024 • edited Loading

dukenv0307 commented Feb 24, 2024

DylanDylann commented Feb 24, 2024 • edited Loading

DylanDylann commented Feb 24, 2024

wildan-m commented Feb 24, 2024

wildan-m commented Feb 25, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Feb 27, 2024

puneetlath commented Feb 28, 2024

dukenv0307 commented Feb 28, 2024

wildan-m commented Mar 1, 2024

eVoloshchak commented Mar 1, 2024

melvin-bot bot commented Mar 1, 2024

eVoloshchak commented Mar 4, 2024

melvin-bot bot commented Mar 4, 2024

dukenv0307 commented Mar 7, 2024

melvin-bot bot commented Mar 12, 2024

melvin-bot bot commented Mar 18, 2024

melvin-bot bot commented Mar 18, 2024

melvin-bot bot commented Mar 18, 2024

puneetlath commented Mar 22, 2024

puneetlath commented Mar 25, 2024

eVoloshchak commented Mar 25, 2024 • edited Loading

eVoloshchak commented Mar 25, 2024

puneetlath commented Mar 25, 2024

puneetlath commented Mar 27, 2024

eVoloshchak commented Mar 29, 2024

puneetlath commented Apr 1, 2024

JmillsExpensify commented Apr 1, 2024

dukenv0307 commented Oct 8, 2024

puneetlath commented Oct 8, 2024

dukenv0307 commented Oct 8, 2024

m-natarajan commented Feb 23, 2024 •

edited by melvin-bot bot

Loading

dukenv0307 commented Feb 23, 2024 •

edited

Loading

brandonhenry commented Feb 24, 2024 •

edited

Loading

DylanDylann commented Feb 24, 2024 •

edited

Loading

wildan-m commented Feb 25, 2024 •

edited

Loading

eVoloshchak commented Mar 25, 2024 •

edited

Loading