[$250] Expense view does not show reimbursement date

[justinpersaud]

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: 9.0.73-0 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Anyone Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @justinpersaud Slack conversation (hyperlinked to channel name): #product

Action Performed:

1. Create an out of pocket expense and submit to an approver
2. Have the report approved

Expected Result:

The report should show when the reimbursement is estimated to happen, e.g. this is what it looks like in OldDot

Actual Result:

The message above is not displayed.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• [ ] Android: Standalone
• [ ] Android: HybridApp
• [ ] Android: mWeb Chrome
• [ ] iOS: Standalone
• [ ] iOS: HybridApp
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021866639204440111557
• Upwork Job ID: 1866639204440111557
• Last Price Increase: 2024-12-11

Issue Owner

Current Issue Owner: @fedirjh

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

[justinpersaud]

We believe the issue might be because the REIMBURSED reportAction is being filtered out of the view in NewDot.

The reportAction is REIMBURSED and here is what the message looks like:

{"accountNumber":"XXXXXX1234","addressName":"Fake address here","creditBankAccountNumber":"1111","expectedDate":"2024-11-07","isNewDot":false,"lastModified":"2024-10-31 23:47:28.549","method":"ACH","routingNumber":"1234567"}

[justinpersaud]

cc @heyjennahay @JmillsExpensify @puneetlath @dangrous

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[puneetlath]

Going ahead and making this external. A contributor should be able to help us add support for the REIMBURSED reportAction in NewDot.

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-12-11 04:07:34 UTC.

Proposal

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

The reimbursed action isn't visible in ND.

What is the root cause of that problem?

An action will be visible only if it's listed on the supported action list that is coming from ACTIONS.TYPE constant. https://github.com/Expensify/App/blob/f9ec4fa032a9a9d3edc0443f441246d81ed0e149/src/libs/ReportActionsUtils.ts#L631-L651 https://github.com/Expensify/App/blob/f9ec4fa032a9a9d3edc0443f441246d81ed0e149/src/CONST.ts#L1009-L1013

However, REIMBURSED is not listed on the const, so it's never shown.

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

Add REIMBURSED to the ACTIONS.TYPE. This will show the message like this:

It's bold and in multiple lines because the message array contains several items and all of them have strong property.

To show it in 1 line and in muted text color, we can handle it like the other cases by checking its action type and using ReportActionItemBasicMessage. https://github.com/Expensify/App/blob/f9ec4fa032a9a9d3edc0443f441246d81ed0e149/src/pages/home/report/ReportActionItem.tsx#L680-L693

} else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.REIMBURSED)) {
    children = <ReportActionItemBasicMessage message={ReportActionsUtils.getReportActionMessageText(action)} />;
} else {

OR another way (and cleaner IMO) is to return the message fragment here as 1 item array. https://github.com/Expensify/App/blob/f8f12c2dcb6d3b62663fc4edea50cd543e44e74d/src/libs/ReportActionsUtils.ts#L1385-L1401

if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.REIMBURSED)) {
    const message = getReportActionMessageText(action);
    return [{text: message, html: `<muted-text>${message}</muted-text>`, type: 'COMMENT'}];
}

getReportActionMessageFragments is being used in ReportActionItemMessage. https://github.com/Expensify/App/blob/f8f12c2dcb6d3b62663fc4edea50cd543e44e74d/src/pages/home/report/ReportActionItemMessage.tsx#L42

So, we don't need to render the custom ReportActionItemBasicMessage anymore.

it's not translatable, if we want to make it translatable, then we need to construct the message manually using the information available from the originalMessage, but I think it's missing the information about the user that we send the money to, unless we get it from the expense report ownerAccountID

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can test shouldReportActionBeVisible to return true for REIMBURSED action, but I think it's not needed since it mostly depends on the ACTIONS.TYPE const.

If we use getReportActionMessageFragments approach, then we can test it to make sure it returns the correct fragment for REIMBURSED action.

[jaydamani]

Edited by proposal-police: This proposal was edited at 2024-12-11 04:43:52 UTC.

Proposal

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

REIMBURSED action is not visible in new dot

What is the root cause of that problem?

There is no logic for rendering REIMBURSED. So this can be considered a new feature

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

Messages like reimbursement are rendered from ReportActionItemMessage which gets text from getReportActionMessageFragments using below logic:

https://github.com/Expensify/App/blob/f9ec4fa032a9a9d3edc0443f441246d81ed0e149/src/libs/ReportActionsUtils.ts#L1385-L1390

So, we need to add REIMBURSED action type in the array inside isOldDotReportAction and case for it in getMessageOfOldDotReportAction which would look like

case CONST.REPORT.ACTIONS.TYPE.REIMBURSED:
    return Localize.translateLocal('report.actions.type.reimbursed', originalMessage);

we would also need to add types in OldDotAction.ts

type OldDotOriginalMessageActionName =
    ...
    | 'REIMBURSED'
    ...

/**
 * 
 */
type OriginalMessageReimbursed = {
    /**
     * 
     */
    actionName: typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSED;
    /**
     * this can be taken as separate type during PR
     */
    originalMessage: {
        "accountNumber": string,
        "creditBankAccountNumber": string,
        "expectedDate": string,
        "isNewDot": boolean,
        "lastModified": string,
        "method": string,
        "routingNumber": string
    } & Record<string, unknown>
}

type OldDotOriginalMessageMap = {
    ...
    [CONST.REPORT.ACTIONS.TYPE.REIMBURSED]: OriginalMessageReimbursed;
    ...
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA since this is a migration issue

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[fedirjh]

@bernhardoj How did you manage to replicate it locally ?

[fedirjh]

@jaydamani Your proposal makes sense to me . Can you please include a testing section ?

[bernhardoj]

@fedirjh I just paid with expensify (with the test bank account).

[fedirjh]

@justinpersaud Should we translate the system message in new dot ?

[justinpersaud]

@fedirjh Since we're trying to get this change out sooner than later, we aren't concerned about translations right now.

[fedirjh]

Thanks, in this case I think we should move forward with @bernhardoj's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[fedirjh]

@justinpersaud Just to give credit to @jaydamani for his proposal. If we want to translate this later, then I think his approach is the standard one for translating oldDot messages.

[sonialiap]

I'm OOO Dec 16-20, adding a leave buddy

next steps:

• proposal was selected, waiting for fix

[melvin-bot[bot]]

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

[bernhardoj]

PR is ready

cc: @fedirjh

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[joekaufmanexpensify]

PR out on staging. Seems like based on the above this did not introduce a regression.

[joekaufmanexpensify]

As an fyi, I am OOO until the new year. Please ask in slack if anything urgent BZ related comes up. Otherwise, I will handle payment when I return!

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @justinpersaud, @sonialiap, @fedirjh, @bernhardoj, @joekaufmanexpensify eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[joekaufmanexpensify]

@sonialiap Looking at this one, I see I was just an OOO buddy, so going to unassign here now that the holidays are over. I think all that's left is to issue payment. LMK if there's anything else I can help with!

[sonialiap]

Looks like automation didn't run

PR (https://github.com/Expensify/App/pull/54102) was deployed to prod 3 weeks ago, this is ready for payment

[sonialiap]

Payment summary:

• @fedirjh reviewer $250 - please complete the checklist - payment via ND
• @bernhardoj contributor $250 - payment via ND

[sonialiap]

@fedirjh please complete the checklist. Automation didn't run so sharing it here:

Click to unfurl checklist

# BugZero Checklist:

- [ ] **[Contributor]** Classify the bug:

<details>
<summary>Bug classification</summary>
<!-- Please keep the "1a." text in tact on all of the options below so that the results can be easily parsed by a script. Each of the "Other" options should be last on the list which is why they have a "z" in the selector (eg. "1z."). This allows the list to grow if someone desires to add more options down the road. -->

Source of bug:
  - [ ] 1a. Result of the original design (eg. a case wasn't considered)
  - [ ] 1b. Mistake during implementation
  - [ ] 1c. Backend bug
  - [ ] 1z. Other:

Where bug was reported:
  - [ ] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  - [ ] 2b. Reported on staging (eg. found during regression or PR testing)
  - [ ] 2d. Reported on a PR
  - [ ] 2z. Other:

Who reported the bug:
  - [ ] 3a. Expensify user
  - [ ] 3b. Expensify employee
  - [ ] 3c. Contributor
  - [ ] 3d. QA
  - [ ] 3z. Other:

</details>

- [ ] **[Contributor]** 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:

- [ ] **[Contributor]** If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in [#expensify-open-source](https://app.slack.com/client/E047TPA624F/C01GTK53T8Q) 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:

- [ ] **[Contributor]** If it was decided to create a regression test for the bug, please propose the [regression test](https://github.com/Expensify/App/blob/main/contributingGuides/REGRESSION_TEST_BEST_PRACTICES.md) steps using the template below to ensure the same bug will not reach production again.

<details>
<summary>Regression Test Proposal Template</summary>
<!-- AFTER FILLING THIS OUT, be sure to remove the <details> and <summary> tags from this part of the checklist!!!!! -->

- [ ] **[BugZero Assignee]** Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

## Regression Test Proposal
### Precondition:
<!-- List the setup instructions necessary to perform the test -->

-

### Test:
<!-- List the steps that QA should perform -->

1.

Do we agree 👍 or 👎

[fedirjh]

BugZero Checklist:

• [x] [Contributor] Classify the bug:

Bug classification

Source of bug:

• [x] 1a. Result of the original design (eg. a case wasn't considered)
• [ ] 1b. Mistake during implementation
• [ ] 1c. Backend bug
• [ ] 1z. Other:

Where bug was reported:

• [x] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• [ ] 2b. Reported on staging (eg. found during regression or PR testing)
• [ ] 2d. Reported on a PR
• [ ] 2z. Other:

Who reported the bug:

• [ ] 3a. Expensify user
• [x] 3b. Expensify employee
• [ ] 3c. Contributor
• [ ] 3d. QA
• [ ] 3z. Other:

• [x] [Contributor] 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: This is new Feature

• [x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [x] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue: https://github.com/Expensify/Expensify/issues/457966

Regression Test Proposal

Precondition:

• The workspace has a bank account set up
• The submitter has a personal bank account to receive the payment

Test:

1. Submit a money request to a workspace
2. As the admin, pay the request with expensify
3. Verify a REIMBURSED action is shown (refresh if not immediately shown)
4. The REIMBURSED message will look like:

Concierge reimbursed this report on behalf of you from...

Do we agree 👍 or 👎

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj