[HOLD for payment 2024-03-20] [HOLD for payment 2024-03-19] [$500] Tag - Tag list shows old tag as selected when newer tag is selected in Split scan request

[kbecciv]

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.50-0
Reproducible in staging?: y
Reproducible in production?: n
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• User is an employee of Collect workspace.
• The Collect workspace has tags.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Go to + > Split bill > Scan.
4. Create a scan split with a tag.
5. Click on the split preview.
6. Click Tag.
7. Select a different tag.
8. Click Tag.

Expected Result:

The Tag list will show the newly-selected tag on the top

Actual Result:

The Tag list shows the original tag selected in Step 4 on the top.

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

Bug6409569_1710160072688.tag.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017e2d605b4f1b0cbe
• Upwork Job ID: 1767210397356195840
• Last Price Increase: 2024-03-11
• Automatic offers:
• DylanDylann | Reviewer | 0

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @mountiny (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kbecciv]

We think that this bug might be related to #wave6-collect-submitters
cc @greg-schroeder

[mountiny]

Seems like this might be related to @FitseTLT @puneetlath @neil-marcellini @ishpaul777 #37879

[ishpaul777]

looking into it...

[nkdengineer]

Proposal

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

The edited tag isn't applied in split bill request

What is the root cause of that problem?

In split bill requests, we save the draft data in splitDraftTransaction field instead of transaction field. But in here, we always get tag from transaction field

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

Line 90 in bf1ff4c

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

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

We should use information from the splitDraftTransaction field if isEditingSplitBill is true like we did in IOURequestStepCategory

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

Line 76 in bf1ff4c

const {category: transactionCategory} = ReportUtils.getTransactionDetails(isEditingSplitBill && !lodashIsEmpty(splitDraftTransaction) ? splitDraftTransaction : transaction);

   const isEditingSplitBill = isEditing && iouType === CONST.IOU.TYPE.SPLIT;
    // const tag = TransactionUtils.getTag(transaction, tagIndex);    // remove
    const tag = TransactionUtils.getTag(isEditingSplitBill && !lodashIsEmpty(splitDraftTransaction) ? splitDraftTransaction : transaction, tagIndex);

What alternative solutions did you explore? (Optional)

[nkdengineer]

@ishpaul777 This issue isn't caused by #37879

[nkdengineer]

While migrating the logic to IOURequestStepPage in here, they missed this case

[ishpaul777]

@nkdengineer are you suggesting the issue is not a deploy blocker, means also repro on production ?

[DylanDylann]

Agree with @nkdengineer. I noticed this bug and reported here. Could I take over this issue as a C+ role?

[ishpaul777]

Actually i can handle this one if @mountiny agrees..

[kbecciv]

@mountiny The QA team is uncertain whether selecting one multilevel tag will deselect another. Should we treat this as the same issue, or do we need to create a new one?

20240311_213333.mp4

[DylanDylann]

@mountiny For more information about this bug, I handled many bugs that relate to new money request refactoring:

• [$250] Remove MoneyRequestDatePage.js and copy any changes since Nov 27 into IOURequestStepDate.js #34608
• [HOLD for payment 2024-03-04] [$250] Remove MoneyRequestCategoryPage.js and copy any changes since Nov 27 into IOURequestStepCategory.js #34606
• [HOLD for payment 2024-02-26] [$250] Remove MoneyRequestMerchantPage.js and copy any changes since Nov 27 into IOURequestStepMerchant.js #34611
• [HOLD for payment 2024-02-19] [$250] Remove MoneyRequestDescriptionPage.js and copy any changes since Nov 27 into IOURequestStepDescription.js #34609

In these issues, we need to use splitDraftTransaction instead of "transaction" if isEditingSplitBill is true like @nkdengineer's solution. It seems @dukenv0307 missed this logic in this PR that caused this issue

From the above information, @nkdengineer's proposal is a good solution

[DylanDylann]

About the additional bug, we also should handle here

@nkdengineer For the tag section, we allow multiple tags so we also need to set the index for the tag in splitDraftTransaction and use TransactionUtils.getTag to get the correct index tag.

[nkdengineer]

Thanks, I am updating proposal to cover the additional bug

[nkdengineer]

Updated proposal to fix the additional case

[DylanDylann]

Look good. @nkdengineer's updated proposal fixes both issues

cc @mountiny

[luacmartins]

This is fixed in staging!

[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 1.4.50-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] Fix/38042: Tag list show old tag #38073

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

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

• @DylanDylann requires payment automatic offer (Reviewer)
• @nkdengineer requires payment (Needs manual offer from BZ)

[melvin-bot]

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

[melvin-bot]

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

• [CP Staging] Fix/38042: Tag list show old tag #38073

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

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

• @DylanDylann requires payment automatic offer (Reviewer)
• @nkdengineer requires payment (Needs manual offer from BZ)

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

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

[sakluger]

Hey @nkdengineer, could you please apply to the Upwork job (https://www.upwork.com/jobs/~017e2d605b4f1b0cbe), or alternatively, link to your Upwork profile here so that I can send you an offer?

@DylanDylann could you please complete the BZ checklist?

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR
[@DylanDylann] 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
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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::

User is an employee of Collect workspace.
The Collect workspace has tags.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Go to + > Split bill > Scan.
4. Create a scan split with a tag.
5. Click on the split preview.
6. Click Tag.
7. Select a different tag.
8. Click Tag.
9. The Tag list will show the newly-selected tag on the top

Do we agree 👍 or 👎

[luacmartins]

We'll add test steps once the feature is fully implemented. No need to add them now.

[sakluger]

Summarizing payment on this issue:

Contributor: @nkdengineer $500, sent offer via Upwork (https://www.upwork.com/nx/wm/offer/101491657)
Contributor+: @DylanDylann $500, paid via Upwork

@nkdengineer please let me know once you've accepted the offer.

[nkdengineer]

@sakluger I've accepted the offer, thanks.

[sakluger]

Thank you! All contracts have been paid out.