[HIGH][$250] Send invoice - Category is missing in confirmation page but it is present in invoice view

[m-natarajan]

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.68-0 Reproducible in staging?: Yes Reproducible in production?: No (New feature) 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: Applause Internal Team Slack conversation:

Action Performed:

Pre condition:Create a new workspace and use to send Invoice

1. Go to staging.new.expensify.com
2. Go to FAB > Send invoice.
3. Enter amount and select a participant.
4. Note that "Category" is not present in the confirmation page.
5. Create the invoice.
6. Click on the preview in workspace chat.

   Expected Result:

   "Category" row should be present in the confirmation page if it is present in the invoice details page.

Actual Result:

"Category" row is not present in the confirmation page when it is present in the invoice details page.

Workaround:

Unknown

Platforms:

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

• [X] Android: Native
• [X] Android: mWeb Chrome
• [X] iOS: Native
• [X] iOS: mWeb Safari
• [X] MacOS: Chrome / Safari
• [X] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/6edfd22c-ecbe-4cf2-b55d-d8e9f97cf5fe

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017a0bd1ec84ee8690
• Upwork Job ID: 1785394941425930240
• Last Price Increase: 2024-08-07
• Automatic offers:
• Pujan92 | Reviewer | 0
• gijoe0295 | Contributor | 0

Issue Owner

Current Issue Owner: @youssef-lr

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[github-actions[bot]]

:wave: 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.

[m-natarajan]

We think this bug might be related to #wave-collect - Release 1

[m-natarajan]

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

[robertjchen]

Does not appear to be a serious deploy blocker to me and can be addressable by external contributors 👍

[cristipaval]

vip-billpay related. @danielrvidal could you please help prioritize?

[danielrvidal]

Given this is core functionality for anyone with a workspace, I'm marking it as CRITICAL as it might be needed in the demos.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~017a0bd1ec84ee8690

[melvin-bot[bot]]

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

[danielrvidal]

@Pujan92 This is a high-priority one. Once you get proposals, can you make sure to accelerate review to get this picked up? Thank you!

[brunovjk]

Proposal

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

Inconsistency in the display of the "Category" field when creating and viewing Send invoice request.

What is the root cause of that problem?

The problem here is about inconsistency, with the new Send invoice flow reaching staging we do not cover the case of "individual sending the invoice would see the category field as the invoice is owned by their workspace."

https://github.com/Expensify/App/blob/99017956d92998f3e301d6a4124669a9e5e44d93/src/components/ReportActionItem/MoneyRequestView.tsx#L168 https://github.com/Expensify/App/blob/99017956d92998f3e301d6a4124669a9e5e44d93/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L137

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

We need to update/standardize how we handle isPolicyExpenseChat when creating a report and viewing it.

I propose we modify the logic controlling the "Category" field's visibility within the IOURequestStepConfirmation to be the same as MoneyRequestView:

    // if the policy of the report is either Collect or Control, then this report must be tied to workspace chat
    const isPolicyExpenseChat = ReportUtils.isReportInGroupPolicy(report);

Or to be more sure, still in IOURequestStepConfirmation we do:

    const isPolicyExpenseChat = useMemo(() => {
        return ReportUtils.isReportInGroupPolicy(report) || ReportUtils.isPolicyExpenseChat(ReportUtils.getRootParentReport(report));
    }, [report]);

What alternative solutions did you explore? (Optional)

N/A

[gijoe0295]

Proposal

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

"Category" row is not present in the confirmation page when it is present in the invoice details page.

What is the root cause of that problem?

In invoice confirmation step, we used a "optimistic" report, thus it is {} causing isPolicyExpenseChat to be false:

https://github.com/Expensify/App/blob/43c0ad72d8eb403401951c2f48f9c0b2500f7107/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L122

But in MoneyRequestView, now we have a "realreport, thusisPolicyExpenseChat` is evaluated properly:

https://github.com/Expensify/App/blob/22e6fcbb9a1a65897c9d2d40222a528fb6bb4cc4/src/components/ReportActionItem/MoneyRequestView.tsx#L167

And eventually cause the inconsistency of group policy-related fields like category or tag or tax...

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

1. Add a direct check for group policy type based on policy besides the check based on report because it could be optimistically created:

https://github.com/Expensify/App/blob/43c0ad72d8eb403401951c2f48f9c0b2500f7107/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L122

const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(ReportUtils.getRootParentReport(report)) || ReportUtils.isGroupPolicy(policy?.type ?? ''), [report, policy]);

2. And pass the correct policyID:

https://github.com/Expensify/App/blob/a7f8e686c4d4496e9974b8372df1c477f500193d/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L566

policyID={report?.policyID ?? policy?.id}

3. In IOURequestStepCategory, add the ReportUtils.isGroupPolicy check to shouldShowCategory check as well:

https://github.com/Expensify/App/blob/84a54db10fdc3b11c287186a61d43e90d7c1f1a5/src/pages/iou/request/step/IOURequestStepCategory.tsx#L76

4. Also get the correct policyID here following the approach used in IOURequestStepConfirmation instead of getting it from report:

https://github.com/Expensify/App/blob/a7f8e686c4d4496e9974b8372df1c477f500193d/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L588

5. Add pass correct policyID here

We need to apply step 3 to 5 to other field pages as well.

What alternative solutions did you explore? (Optional)

NA

[Pujan92]

@cristipaval @danielrvidal Needs confirmation regarding the expected result, Categories should be displayed for the non-policy expense chat invoices or not? Bcoz we usually show it for the policyExpenseChat only.

~I am more towards @brunovjk's proposal where they suggested using the consistent condition in both places and in this case, it won't show categories in MoneyRequestView as the rootParent Report isn't of type policyExpenseChat.~

@brunovjk As per this comment any report connected to group policy should be considered as workspace chat. Considering this we should display those options on the confirmation page.

https://github.com/Expensify/App/blob/5f504301f9be63b728f78fdf2afd085a2872fbdd/src/components/ReportActionItem/MoneyRequestView.tsx#L167-L168

[Pujan92]

In invoice confirmation step, we used a "optimistic" report, thus it is {} causing isPolicyExpenseChat to be false:

@gijoe0295 wondering why the issue is not there for the submit expense.

[gijoe0295]

wondering why the issue is not there for the submit expense

@Pujan92 After we selected the participants, the reports would no longer be "optimistic" because theirs reportIDs were already known beforehand by getFilteredOptions here. Note that user couldn't submit an expense to a completely new workspace. By that, we can ensure that we can always get the correct report from Onyx (for workspaces).

However, for invoices:

• For new invoice chats, they are not created optimistically until the IOU creation flow is completed so it's always {}
• For existing ones, we could retrieve them using getInvoiceChatByParticipants like:

https://github.com/Expensify/App/blob/5f504301f9be63b728f78fdf2afd085a2872fbdd/src/libs/actions/IOU.ts#L1619-L1621

I have thought about this as an alternative solution but struggled with the new invoice chats case.

[Pujan92]

Makes sense @gijoe0295. Your solution seems to work where we need to use policy.id in case the report is not available. Still, I am tagging @VickyStash to get their input on any other straightforward alternative.

[danielrvidal]

@Pujan92 the expected behavior is that the individual sending the invoice would see the category field as the invoice is owned by their workspace. But the individual receiving the invoice should not. Does that make sense? Let me know if I can answer anything else.

[Pujan92]

Thanks @danielrvidal.

We can proceed with @gijoe0295's proposal as their solution will fix the issue. cc: @cristipaval

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[Pujan92]

@brunovjk It won't work because optimistic report isn't available in the IOURequestStepConfirmation, it only gets created after the confirmation. So in IOURequestStepConfirmation report will be undefined for the invoice flow. Only policy details we may have and that can be used as suggested by @gijoe0295 .

[brunovjk]

Awesome!!! Thank you very much :D :D

[melvin-bot[bot]]

📣 @Pujan92 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

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

[danielrvidal]

@gijoe0295 can you make sure to prioritize this one tomorrow? We need this to be fixed asap so if it could be deployed to staging tomorrow that would be best. Thank you!

[Pujan92]

@gijoe0295 will you raise a PR soon or facing any issues?

[gijoe0295]

Yes PR will be ready in 1 or 2 hours.

[Pujan92]

Thanks :)

[gijoe0295]

@danielrvidal @Pujan92 ~Do we need to apply these changes for tag, tax rate and tax amount fields?~ We must because isPolicyExpenseChat is true now and thus all these fields would appear.

However, seems like the BE currently does not support these params in SendInvoice API. I tried to enable and set those fields in the FE, the payload was correct but SendInvoice just returned empty for those fields.

[Pujan92]

However, seems like the BE currently does not support these params in SendInvoice API. I tried to enable and set those fields in the FE, the payload was correct but SendInvoice just returned empty for those fields.

@cristipaval Can you plz confirm whether the SendInvoice API is accepting these extra params(category, tax, ..) or not?

[gijoe0295]

Category is working fine. But not taxCode, taxAmount and tag.

[Pujan92]

Ah, Ok. As all those fields appear when isPolicyExpenseChat is true we need to apply this logic to all components. Better if you share the payload here which @cristipaval can check and confirm with the backend.

[gijoe0295]

Here's SendInvoice payload:

senderWorkspaceID: 16F69CC60A3B7FD7
accountID: 16096394
amount: 111100
currency: VND
merchant: Daewoo
category: Advertising
-> tag: Marketing
-> taxCode: id_TAX_RATE_1
-> taxAmount: 5290
date: 2024-05-03
invoiceRoomReportID: 3478060593787131
createdChatReportActionID: 3956993894585089773
invoiceReportID: 4861381044027010
reportPreviewReportActionID: 7027023322402651949
transactionID: 2436279338783823015
transactionThreadReportID: 1525400476644226
receiverEmail: gijoe0295+4@gmail.com
email: gijoe0295+1@gmail.com

Then tag turned blank, no taxRate or taxCode:

[davidcardoza]

The tag, taxRate, and taxCode parameters can be left blank. These are parameters that we display in the UI of NewDot.

[gijoe0295]

@davidcardoza The problem here is after the changes in this issue's PR https://github.com/Expensify/App/pull/41524, user is allowed to edit category, tag, taxRate and taxCode while creating the invoice, just like a policy expense. category is working fine but not the others. We could temporarily disable tag, taxRate and taxCode in the FE but I think the best fix here is to allow them in the BE.

[davidcardoza]

Got it. Yeah, we will want to provide the ability for users who create an invoice to add and edit tags and taxes rates when they become available in the product. So I agree with your approach.

[gijoe0295]

@danielrvidal So would an internal engineer be assigned here for the work in the BE? Should we hold the FE changes until BE work is completed?

[davidcardoza]

Sorry to clarify, we shouldn't wait for the changes in BE. We should temporarily disable the fields mentioned. We can re-enable them when they become available.

[gijoe0295]

Thanks for the clarification!

[Pujan92]

Sorry to clarify, we shouldn't wait for the changes in BE. We should temporarily disable the fields mentioned. We can re-enable them when they become available.

@cristipaval @davidcardoza As @gijoe0295 mentioned here, in case we disable those fields(tag, tax) then it may show the RBR(violation) to the user if any of them is mandatory and user can't fix that due to we are hiding/disabling those fields.

If we can't put this on hold till the backend changes then the option which looks more feasible to me is

• Hide these fields in the Confirmation screen of the Send invoice flow
• In the money request view show these fields and let user update as that API works

Let us know your thoughts to proceed

[davidcardoza]

If we can't put this on hold till the backend changes then the option which looks more feasible to me is

• Hide these fields in the Confirmation screen of the Send invoice flow
• In the money request view show these fields and let user update as that API works

This approach makes sense. Once the fields become available in the product, we will include them in this send invoice workflow and stop hiding them. However, our immediate priority is to perfect the invoice flow to meet our internal deadline. For now, let's hide these fields in the confirmation screen for the send invoice flow. In the money request view, however, we will display the fields.

[Pujan92]

Thanks @davidcardoza for the confirmation. @cristipaval PR is ready for your review.

[cristipaval]

@gijoe0295 could you please quickly reopen a PR?

[gijoe0295]

Yes, PR will be ready in 1 hour.

[davidcardoza]

Thank you!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/41723

If no regressions arise, payment will be issued on 2024-05-20. :confetti_ball:

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

• @Pujan92 requires payment automatic offer (Reviewer)
• @gijoe0295 requires payment automatic offer (Contributor)

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

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

[sonialiap]

Payment summary @Pujan92 Reviewer $250 - paid ✔️ - please complete the checklist :) @gijoe0295 Contributor $250 - paid ✔️

[cristipaval]

Making this one Weekly, because @Pujan92 is ooo