Skip to content

[HOLD for payment 2024-07-23][$250] We're requiring merchants when sending an invoice, when we shouldn't be.  #44126

Description

@danielrvidal

If you go to Send Invoice, and then input a user, we're requiring a merchant name. We should not be requiring the merchant. This is reproducable on all platforms for me.

image

cc @davidcardoza @cristipaval

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010f4b3b019ed717d4
  • Upwork Job ID: 1804299691171409677
  • Last Price Increase: 2024-06-28
  • Automatic offers:
    • rayane-djouah | Reviewer | 102954435
    • bernhardoj | Contributor | 102954436
Issue OwnerCurrent Issue Owner: @miljakljajic

Activity

  1. bernhardoj commented on Jun 21, 2024

    @bernhardoj
    Contributor

    Proposal

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

    Merchant is required for invoice request even though it shouldn't.

    What is the root cause of that problem?

    This happens after this PR where we change it so the merchant is required when creating or editing the invoice.

    const isMerchantRequired = (isPolicyExpenseChat || isTypeInvoice) && (!isScanRequest || isEditingSplitBill) && shouldShowMerchant;

    const isMerchantRequired = ReportUtils.isReportInGroupPolicy(report) || isTypeInvoice || transaction?.participants?.some((participant) => !!participant.isPolicyExpenseChat);

    Previously, the invoice was only required in edit invoice detail, but the PR above makes it consistent so creating an invoice also requires a merchant.

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

    Because we do not want to make it required, we need to "revert" that PR. We will remove the invoice type condition from both of these codes

    const isMerchantRequired = (isPolicyExpenseChat || isTypeInvoice) && (!isScanRequest || isEditingSplitBill) && shouldShowMerchant;

    const isMerchantRequired = ReportUtils.isReportInGroupPolicy(report) || isTypeInvoice || transaction?.participants?.some((participant) => !!participant.isPolicyExpenseChat);

    isPolicyExpenseChat is false for invoice, however, ReportUtils.isReportInGroupPolicy is true even for invoice because it's part of a policy.

    To fix it, we have 2 options:

    1. Replace isReportInGroupPolicy with isExpenseRequest. This condition is stricter by checking whether the request is an expense request or not which is true for workspace expense requests, but not for invoice or IOU request.

    2. Add !ReportUtils.isInvoiceRequest(report) && condition so it only could be true if it's not an invoice request.

    const isMerchantRequired = !ReportUtils.isInvoiceRequest(report) && (ReportUtils.isReportInGroupPolicy(report) || transaction?.participants?.some((participant) => !!participant.isPolicyExpenseChat));
    

    isInvoiceRequest will be a new function that is similar to isExpenseRequest

    function isInvoiceRequest(report: OnyxInputOrEntry<Report>): boolean {
        if (isThread(report)) {
            const parentReportAction = ReportActionsUtils.getParentReportAction(report);
            const parentReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`];
            return isInvoiceReport(parentReport) && !isEmptyObject(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction);
        }
        return false;
    }
    
  2. davidcardoza commented on Jun 21, 2024

    @davidcardoza
    Contributor

    Damn, it looks like the other PR linked was created to make it so that a merchant name is required on invoices. Unfortunately the invoicing project team didn't catch it because it never passed through the project Slack channel or the project board.

    That proposal looks good @bernhardoj

  3. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jun 21, 2024
  4. melvin-bot commented on Jun 21, 2024

    @melvin-bot
  5. changed the title [-]We're requiring merchants when sending an invoice, when we shouldn't be. [/-] [+][$250] We're requiring merchants when sending an invoice, when we shouldn't be. [/+] on Jun 21, 2024
  6. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jun 21, 2024
  7. melvin-bot commented on Jun 21, 2024

    @melvin-bot

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

  8. melvin-bot commented on Jun 21, 2024

    @melvin-bot

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

  9. rayane-d commented on Jun 23, 2024

    @rayane-d
    Contributor

    @bernhardoj's proposal looks good to me

    🎀👀🎀 C+ reviewed

  10. 18 remaining items

  11. bernhardoj commented on Jul 2, 2024

    @bernhardoj
    Contributor

    PR is ready

    cc: @rayane-djouah

  12. miljakljajic commented on Jul 11, 2024

    @miljakljajic
    Contributor

    What will the payment date for this one be?

  13. bernhardoj commented on Jul 19, 2024

    @bernhardoj
    Contributor

    It's deployed to prod 3 days ago, so it should be 23 July

  14. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed
    ReviewingHas a PR in review
    on Jul 19, 2024
  15. changed the title [-][$250] We're requiring merchants when sending an invoice, when we shouldn't be. [/-] [+][HOLD for payment 2024-07-23][$250] We're requiring merchants when sending an invoice, when we shouldn't be. [/+] on Jul 19, 2024
  16. cristipaval commented on Jul 19, 2024

    @cristipaval
    Contributor

    Not overdue

  17. miljakljajic commented on Jul 24, 2024

    @miljakljajic
    Contributor

    Both contributors paid!

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