[Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date #53872

justinpersaud · 2024-12-10T16:47:26Z

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:

Create an out of pocket expense and submit to an approver
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?

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 · 2024-12-10T16:47:35Z

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 · 2024-12-10T16:50:42Z

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 · 2024-12-10T16:51:16Z

cc @heyjennahay @JmillsExpensify @puneetlath @dangrous

melvin-bot · 2024-12-11T00:20:43Z

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

melvin-bot · 2024-12-11T00:20:48Z

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

puneetlath · 2024-12-11T00:21:03Z

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

bernhardoj · 2024-12-11T03:54:10Z

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.

App/src/libs/ReportActionsUtils.ts

Lines 631 to 651 in f9ec4fa

    
           const {POLICY_CHANGE_LOG: policyChangelogTypes, ROOM_CHANGE_LOG: roomChangeLogTypes, ...otherActionTypes} = CONST.REPORT.ACTIONS.TYPE; 
        
           const supportedActionTypes: ReportActionName[] = [...Object.values(otherActionTypes), ...Object.values(policyChangelogTypes), ...Object.values(roomChangeLogTypes)]; 
        
           /** 
        
            * Checks if a reportAction is fit for display, meaning that it's not deprecated, is of a valid 
        
            * and supported type, it's not deleted and also not closed. 
        
            */ 
        
           function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key: string | number, canUserPerformWriteAction?: boolean): boolean { 
        
               if (!reportAction) { 
        
                   return false; 
        
               } 
        
               if (isReportActionDeprecated(reportAction, key)) { 
        
                   return false; 
        
               } 
        
               // Filter out any unsupported reportAction types 
        
               if (!supportedActionTypes.includes(reportAction.actionName)) { 
        
                   return false; 
        
               }

App/src/CONST.ts

Lines 1009 to 1013 in f9ec4fa

    
           ACTIONS: { 
        
               LIMIT: 50, 
        
               // OldDot Actions render getMessage from Web-Expensify/lib/Report/Action PHP files via getMessageOfOldDotReportAction in ReportActionsUtils.ts 
        
               TYPE: { 
        
                   ACTIONABLE_ADD_PAYMENT_CARD: 'ACTIONABLEADDPAYMENTCARD',

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.

App/src/pages/home/report/ReportActionItem.tsx

Lines 680 to 693 in f9ec4fa

    
           } else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.FORWARDED)) { 
        
               const wasAutoForwarded = ReportActionsUtils.getOriginalMessage(action)?.automaticAction ?? false; 
        
               if (wasAutoForwarded) { 
        
                   children = ( 
        
                       <ReportActionItemBasicMessage message=""> 
        
                           <RenderHTML html={`<comment><muted-text>${ReportUtils.getReportAutomaticallyForwardedMessage(action, reportID)}</muted-text></comment>`} /> 
        
                       </ReportActionItemBasicMessage> 
        
                   ); 
        
               } else { 
        
                   children = <ReportActionItemBasicMessage message={ReportUtils.getIOUForwardedMessage(action, report)} />; 
        
               } 
        
           } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED) { 
        
               children = <ReportActionItemBasicMessage message={translate('iou.rejectedThisReport')} />; 
        
           } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.HOLD) {

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

App/src/libs/ReportActionsUtils.ts

Lines 1385 to 1401 in f8f12c2

    
           function getReportActionMessageFragments(action: ReportAction): Message[] { 
        
               if (isOldDotReportAction(action)) { 
        
                   const oldDotMessage = getMessageOfOldDotReportAction(action); 
        
                   const html = isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SELECTED_FOR_RANDOM_AUDIT) ? Parser.replace(oldDotMessage) : oldDotMessage; 
        
                   return [{text: oldDotMessage, html: `<muted-text>${html}</muted-text>`, type: 'COMMENT'}]; 
        
               } 
        
               if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION)) { 
        
                   const message = getUpdateRoomDescriptionMessage(action); 
        
                   return [{text: message, html: `<muted-text>${message}</muted-text>`, type: 'COMMENT'}]; 
        
               } 
        
               const actionMessage = action.previousMessage ?? action.message; 
        
               if (Array.isArray(actionMessage)) { 
        
                   return actionMessage.filter((item): item is Message => !!item); 
        
               } 
        
               return actionMessage ? [actionMessage] : [];

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.

App/src/pages/home/report/ReportActionItemMessage.tsx

Line 42 in f8f12c2

const fragments = ReportActionsUtils.getReportActionMessageFragments(action);

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 · 2024-12-11T04:38:12Z

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:

App/src/libs/ReportActionsUtils.ts

Lines 1385 to 1390 in f9ec4fa

    
           function getReportActionMessageFragments(action: ReportAction): Message[] { 
        
               if (isOldDotReportAction(action)) { 
        
                   const oldDotMessage = getMessageOfOldDotReportAction(action); 
        
                   const html = isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SELECTED_FOR_RANDOM_AUDIT) ? Parser.replace(oldDotMessage) : oldDotMessage; 
        
                   return [{text: oldDotMessage, html: `<muted-text>${html}</muted-text>`, type: 'COMMENT'}]; 
        
               }

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 · 2024-12-11T19:13:42Z

@bernhardoj How did you manage to replicate it locally ?

fedirjh · 2024-12-11T19:15:06Z

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

bernhardoj · 2024-12-12T03:57:26Z

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

fedirjh · 2024-12-12T12:47:33Z

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

justinpersaud · 2024-12-12T16:04:05Z

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

fedirjh · 2024-12-12T16:08:24Z

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

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-12-12T16:08:35Z

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

fedirjh · 2024-12-12T16:16:19Z

@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 · 2024-12-13T11:14:06Z

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

next steps:

proposal was selected, waiting for fix

melvin-bot · 2024-12-13T11:14:19Z

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 · 2024-12-13T12:28:55Z

PR is ready

cc: @fedirjh

melvin-bot · 2024-12-16T09:11:34Z

⚠️ 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 · 2024-12-16T15:45:21Z

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

joekaufmanexpensify · 2024-12-20T21:52:04Z

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 · 2025-01-06T10:02:36Z

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 · 2025-01-06T15:34:46Z

@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 · 2025-01-06T15:37:58Z

Looks like automation didn't run

PR (#54102) was deployed to prod 3 weeks ago, this is ready for payment

sonialiap · 2025-01-06T15:42:19Z

Payment summary:

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

sonialiap · 2025-01-06T15:42:57Z

@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 · 2025-01-06T22:32:20Z

bernhardoj · 2025-01-07T05:51:19Z

Requested in ND.

JmillsExpensify · 2025-01-08T18:23:18Z

$250 approved for @bernhardoj

garrettmknight · 2025-01-29T21:11:42Z

$250 approved for @fedirjh

justinpersaud added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 10, 2024

melvin-bot bot assigned sonialiap Dec 10, 2024

puneetlath added the External Added to denote the issue can be worked on by a contributor label Dec 11, 2024

melvin-bot bot changed the title ~~Expense view does not show reimbursement date~~ [$250] Expense view does not show reimbursement date Dec 11, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 11, 2024

melvin-bot bot assigned fedirjh Dec 11, 2024

justinpersaud self-assigned this Dec 11, 2024

justinpersaud assigned bernhardoj Dec 12, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 12, 2024

sonialiap removed their assignment Dec 13, 2024

sonialiap removed the Bug Something is broken. Auto assigns a BugZero manager. label Dec 13, 2024

sonialiap added the Bug Something is broken. Auto assigns a BugZero manager. label Dec 13, 2024

melvin-bot bot assigned joekaufmanexpensify Dec 13, 2024

sonialiap self-assigned this Dec 13, 2024

bernhardoj mentioned this issue Dec 13, 2024

Added a support for REIMBURSED action #54102

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 13, 2024

melvin-bot bot removed the Weekly KSv2 label Jan 6, 2025

melvin-bot bot added the Monthly KSv2 label Jan 6, 2025

joekaufmanexpensify added Daily KSv2 and removed Monthly KSv2 labels Jan 6, 2025

joekaufmanexpensify removed their assignment Jan 6, 2025

joekaufmanexpensify added the Awaiting Payment Auto-added when associated PR is deployed to production label Jan 6, 2025

sonialiap changed the title ~~[$250] Expense view does not show reimbursement date~~ [Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date Jan 6, 2025

sonialiap closed this as completed Jan 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date #53872

[Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date #53872

justinpersaud commented Dec 10, 2024 •

edited by sonialiap

Loading

melvin-bot bot commented Dec 10, 2024

justinpersaud commented Dec 10, 2024

justinpersaud commented Dec 10, 2024

melvin-bot bot commented Dec 11, 2024

melvin-bot bot commented Dec 11, 2024

puneetlath commented Dec 11, 2024

bernhardoj commented Dec 11, 2024 •

edited by github-actions bot

Loading

jaydamani commented Dec 11, 2024 •

edited by github-actions bot

Loading

fedirjh commented Dec 11, 2024

fedirjh commented Dec 11, 2024

bernhardoj commented Dec 12, 2024

fedirjh commented Dec 12, 2024

justinpersaud commented Dec 12, 2024

fedirjh commented Dec 12, 2024

melvin-bot bot commented Dec 12, 2024

fedirjh commented Dec 12, 2024

sonialiap commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

bernhardoj commented Dec 13, 2024

melvin-bot bot commented Dec 16, 2024

joekaufmanexpensify commented Dec 16, 2024

joekaufmanexpensify commented Dec 20, 2024

melvin-bot bot commented Jan 6, 2025

joekaufmanexpensify commented Jan 6, 2025

sonialiap commented Jan 6, 2025

sonialiap commented Jan 6, 2025 •

edited

Loading

sonialiap commented Jan 6, 2025 •

edited

Loading

fedirjh commented Jan 6, 2025 •

edited by sonialiap

Loading

bernhardoj commented Jan 7, 2025

JmillsExpensify commented Jan 8, 2025

garrettmknight commented Jan 29, 2025

[Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date #53872

[Hold for payment 2024-12-25][$250] Expense view does not show reimbursement date #53872

Comments

justinpersaud commented Dec 10, 2024 • edited by sonialiap Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 10, 2024

justinpersaud commented Dec 10, 2024

justinpersaud commented Dec 10, 2024

melvin-bot bot commented Dec 11, 2024

melvin-bot bot commented Dec 11, 2024

puneetlath commented Dec 11, 2024

bernhardoj commented Dec 11, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

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

jaydamani commented Dec 11, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

fedirjh commented Dec 11, 2024

fedirjh commented Dec 11, 2024

bernhardoj commented Dec 12, 2024

fedirjh commented Dec 12, 2024

justinpersaud commented Dec 12, 2024

fedirjh commented Dec 12, 2024

melvin-bot bot commented Dec 12, 2024

fedirjh commented Dec 12, 2024

sonialiap commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

bernhardoj commented Dec 13, 2024

melvin-bot bot commented Dec 16, 2024

joekaufmanexpensify commented Dec 16, 2024

joekaufmanexpensify commented Dec 20, 2024

melvin-bot bot commented Jan 6, 2025

joekaufmanexpensify commented Jan 6, 2025

sonialiap commented Jan 6, 2025

sonialiap commented Jan 6, 2025 • edited Loading

sonialiap commented Jan 6, 2025 • edited Loading

fedirjh commented Jan 6, 2025 • edited by sonialiap Loading

BugZero Checklist:

Regression Test Proposal

Precondition:

Test:

bernhardoj commented Jan 7, 2025

JmillsExpensify commented Jan 8, 2025

garrettmknight commented Jan 29, 2025

justinpersaud commented Dec 10, 2024 •

edited by sonialiap

Loading

bernhardoj commented Dec 11, 2024 •

edited by github-actions bot

Loading

jaydamani commented Dec 11, 2024 •

edited by github-actions bot

Loading

sonialiap commented Jan 6, 2025 •

edited

Loading

sonialiap commented Jan 6, 2025 •

edited

Loading

fedirjh commented Jan 6, 2025 •

edited by sonialiap

Loading