Skip to content

[C+ Checklist Needs Completion] [ECARD TRANSACTIONS] [$500] Report name on an expense report with non-reimbursable transactions is showing the workspace name instead of the requestor name (see "Free Workspace - ECard testing spent") #34622

Closed

Description

@kevinksullivan

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue reported by: @kevinksullivan
Slack conversation: https://expensify.slack.com/archives/C05DWUDHVK7/p1705356562602929

Action Performed:

  1. Create a free workspace and set it as your default
  2. Incur a charge on an Expensify Card

Expected Result:

The report title should be %firstName% spent

Actual Result:

The report title is using the workspace name instead of firstName of the requestor

image

Activity

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

    @melvin-bot

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

  3. moved this to Release 4: Card Experience in [#whatsnext] Wave 05 - Deprecate Freeon Jan 17, 2024
  4. grgia commented on Jan 19, 2024

    @grgia
    Contributor

    Solution Notes

    return displayAmount;
    };
    const getPreviewMessage = () => {
    if (isScanning) {
    return props.translate('common.receipt');
    }
    const payerOrApproverName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    if (isApproved) {
    return props.translate('iou.managerApproved', {manager: payerOrApproverName});
    }
    const managerName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    let paymentVerb = hasNonReimbursableTransactions ? 'iou.payerSpent' : 'iou.payerOwes';
    if (iouSettled || props.iouReport.isWaitingOnBankAccount) {
    paymentVerb = 'iou.payerPaid';

            const managerName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    
    

    on this line, we never want to use the workspace name if the report has non-reimbrsable transactions. it should always be %FirstName% spent

    so maybe we can try

            const managerName = (isPolicyExpenseChat && !hasNonReimbursableTransactions) ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    
    
  5. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jan 19, 2024
  6. melvin-bot commented on Jan 19, 2024

    @melvin-bot

    Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

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

    @melvin-bot

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

  9. abzokhattab commented on Jan 19, 2024

    @abzokhattab
    Contributor

    Proposal

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

    Report name on an expense report with non-reimbursable transactions is showing the workspace name instead of the requestor name

    What is the root cause of that problem?

    we dont have a check for non-reimbursable transactions here

    const managerName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    let paymentVerb = hasNonReimbursableTransactions ? 'iou.payerSpent' : 'iou.payerOwes';

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

    if the report has NonReimbursable Transactions we need to display the first name:

            const managerName = (isPolicyExpenseChat && !hasNonReimbursableTransactions) ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);

    OR we can also validate that the transaction is a card transaction by using

    const isCardTransaction = TransactionUtils.isCardTransaction(transaction);

    then the final condition would be:

            const managerName = (isPolicyExpenseChat && !hasNonReimbursableTransactions &&!isCardTransaction ) ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
  10. Tony-MK commented on Jan 19, 2024

    @Tony-MK
    Contributor

    Proposal

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

    [ECARD TRANSACTIONS] Report name on an expense report with non-reimbursable transactions is showing the workspace name instead of the requestor name (see "Free Workspace - ECard testing spent")

    What is the root cause of that problem?

    We have not implemented a condition in the getPreviewMessage function to handle titles for IOUs paid by an Expensify card.

    const managerName = isPolicyExpenseChat ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);

    Furthermore, we will only display the only firstName of the participant if and only if the report isPolicyExpenseChat.

    Hence, we end up displaying the policy name.

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

    In addition to !hasNonReimbursableTransactions, we should create a condition such as usedExpensifyCard to further check if every or some of the transactions were paid using an Expensify card.

    This will ensure that IOUs paid by other methods do not get affected.

    Hence, the changes to the code should look close to below.

    const usedExpensifyCard = !hasNonReimbursableTransactions &&
    TransactionUtils.getAllReportTransactions(report.iouReportID).every(
     (transaction) => TransactionUtils.isExpensifyCardTransaction(transaction)
    );
    
    const managerName = (isPolicyExpenseChat && !usedExpensifyCard) ? ReportUtils.getPolicyName(props.chatReport) : ReportUtils.getDisplayNameForParticipant(managerID, true);
    

    Finally, I suggest we change the MoneyReportHeader's title and subtitle and the LHNOptionRow's title and subtitle to be consistent with the report preview.

    Therefore, we will also have to change functions such as ReportUtils.getReportName and ReportUtils.getChatRoomSubtitle.

    This will ensure consistency with other components mentioned earlier, which require the same title as the report preview.

  11. 49 remaining items

  12. greg-schroeder commented on Feb 16, 2024

    @greg-schroeder
    Contributor

    Annoying. Okay then, I'll create a job manually.

    #36653

  13. deleted a comment from melvin-bot on Feb 16, 2024
  14. removed
    Help WantedApply this label when an issue is open to proposals by contributors
    on Feb 16, 2024
  15. deleted a comment from melvin-bot on Feb 16, 2024
  16. deleted a comment from melvin-bot on Feb 16, 2024
  17. mollfpr commented on Feb 16, 2024

    @mollfpr
    Contributor

    @greg-schroeder I’ll do a manual request in NewDot and complete the checklist later today!

  18. greg-schroeder commented on Feb 16, 2024

    @greg-schroeder
    Contributor

    Ah, nice, I didn't realize you were eligible for manual requests @mollfpr! I'll update our documentation on that - you can ignore the offer I sent you then.

  19. greg-schroeder commented on Feb 16, 2024

    @greg-schroeder
    Contributor

    (payment summary)

    $500 offer sent to @abzokhattab
    @mollfpr can make a manual request for $500

  20. changed the title [-][HOLD for payment 2024-02-15] [ECARD TRANSACTIONS] [$500] Report name on an expense report with non-reimbursable transactions is showing the workspace name instead of the requestor name (see "Free Workspace - ECard testing spent")[/-] [+][C+ Checklist Needs Completion] [ECARD TRANSACTIONS] [$500] Report name on an expense report with non-reimbursable transactions is showing the workspace name instead of the requestor name (see "Free Workspace - ECard testing spent")[/+] on Feb 16, 2024
  21. mollfpr commented on Feb 16, 2024

    @mollfpr
    Contributor

    [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
    [@mollfpr] 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:

    No offending PR was found.

    [@mollfpr] 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:

    The regression step should be good.

    [@mollfpr] Determine if we should create a regression test for this bug.
    [@mollfpr] 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.

    1. Login to OldDot
    2. Create an expense report and add expense
    3. Fill the field with any info unmark the Reimbursable checkbox, and save the expense
    4. Open the NewDot and go to the workspace report
    5. Ensure that the requester name, not the workspace name is shown in the preview and the LHN titles
    6. 👍 or 👎
  22. greg-schroeder commented on Feb 16, 2024

    @greg-schroeder
    Contributor

    Thanks! filed

  23. JmillsExpensify commented on Mar 2, 2024

    @JmillsExpensify
    Contributor

    $500 approved for @mollfpr based on summary above.

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

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions