Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.9k forks source link

[Held requests] [$250] RBR doesn't show for submitter for remaining held expense #47002

Closed m-natarajan closed 1 month ago

m-natarajan commented 3 months ago

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: 9.0.17-1 Reproducible in staging?: y Reproducible in production?: y 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: @garrettmknight Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723054648634699

Action Performed:

  1. As the employee - Submit 2 Expenses
  2. As the approver - Hold 1 of those 2 expenses
  3. As the approver - approve only the unheld expense

Expected Result:

Employee should see RBR on the expense in LHN

Actual Result:

Employee doesn't see RBR on the expense in LHN unless they open the expense from the workspace chat.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/ad1f738a-61ca-4a8a-b814-15d0f5d6bf9e

https://github.com/user-attachments/assets/6df7e5ac-d9d3-47c0-8154-d36957cb078a

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01cce2b99466e28beb
  • Upwork Job ID: 1827431504594139000
  • Last Price Increase: 2024-09-28
Issue OwnerCurrent Issue Owner: @situchan
melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

@kevinksullivan Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 3 months ago

@kevinksullivan Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] commented 2 months ago

@kevinksullivan 10 days overdue. Is anyone even seeing these? Hello?

melvin-bot[bot] commented 2 months ago

@kevinksullivan this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 2 months ago

@kevinksullivan 12 days overdue now... This issue's end is nigh!

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

raza-ak commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-10-01 10:32:57 UTC.

Proposal

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

RBR doesn't show for submitter for remaining held expense

What is the root cause of that problem?

The root cause of the issue is that the component is receiving a filtered array of reportIDs instead of the complete set of reportIDs from all chat reports. This results in incomplete data being displayed in the component.

To resolve this, we need to ensure that all reportIDs from the chatReports are passed to the component, rather than just a subset that has been filtered.

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

We need to update the component function to pass all updated arrays with reportIDs so that they can be displayed on the LHN.

In the ReportIDsContextProvider (File: https://github.com/Expensify/App/blob/main/src/hooks/useReportIDs.tsx#L134), ensure that it returns all chat reports. Update the return statement to:

return {
  orderedReportIDs: getOrderedReportIDs(derivedCurrentReportID),
  currentReportID: derivedCurrentReportID ?? '-1',
  policyMemberAccountIDs,
  chatReports
};

Next, in the SidebarLinksData function (File: src/pages/home/sidebar/SidebarLinksData.tsx), pass all report IDs to optionListItems. We will need to make the following changes:

  1. Destructure the required values:

const { orderedReportIDs, currentReportID, policyMemberAccountIDs, chatReports } = useReportIDs();

  1. Update the isNonSystemReport function:
function isNonSystemReport(report: any): report is any {
  return report.chatType !== "system" && !!report.reportID;
}
  1. Generate the AllReportIDs array:
const AllReportIDs = Object.values(chatReports)
  .filter(isNonSystemReport)
  .map(report => report.reportID);
  1. Replace the optionListItems prop: Replace optionListItems={orderedReportIDs} with optionListItems={AllReportIDs} to render the reports in the sidebar.

https://github.com/user-attachments/assets/76a1db49-ff04-4655-9fa5-d03a86067156

trjExpensify commented 2 months ago

CC: @robertjchen for vis!

situchan commented 2 months ago

@raza-ak thanks for the proposal. The root cause is not correct. Please be more familiar with our codebase. Thanks

melvin-bot[bot] commented 2 months ago

@kevinksullivan, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

situchan commented 2 months ago

Awaiting proposals

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 months ago

@kevinksullivan, @situchan Eep! 4 days overdue now. Issues have feelings too...

wildan-m commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-09-04 04:06:11 UTC.

Proposal

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

RBR does not display remaining held expense for the submitter.

What is the root cause of that problem?

We show the RBR related to held request violation when the LHN option is one Transaction Thread Report.

https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/libs/SidebarUtils.ts#L316-L329

For workspace chat, oneTransactionThreadReportID will always be empty.

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

Add else condition here:

https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/libs/SidebarUtils.ts#L316-L329

Display the RBR if there is a violation related to the iouReportID from the report's action.

    } else if (!isEmptyObject(reportActions) && !isEmptyObject(transactionViolations)) {
        const filteredActions = Object.values(reportActions).filter(action => 
            ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW)
        );

        for (const action of filteredActions) {
            const iouReportID = ReportActionsUtils.getIOUReportIDFromReportActionPreview(action);
            if (ReportUtils.hasViolations(iouReportID, transactionViolations)) {
                result.brickRoadIndicator = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
                break;
            }
        }
    }

We can add additional checks (pre-filter) to determine if the RBR should only be visible to the submitter by comparing action.childOwnerAccountID with the current user's account ID.

Branch for this solution

What alternative solutions did you explore? (Optional)

N/A

wildan-m commented 2 months ago

Proposal Updated

kevinksullivan commented 2 months ago

Still waiting for an approved proposal. We have one for you @situchan !

melvin-bot[bot] commented 2 months ago

@kevinksullivan @situchan this issue is now 4 weeks old, please consider:

Thanks!

situchan commented 2 months ago

@wildan-m thanks for the proposal. The root cause is not clear to me.

Employee doesn't see RBR on the expense in LHN unless they open the expense from the workspace chat.

Can you please explain why the bug is gone if they open the expense report?

wildan-m commented 2 months ago

@situchan Ah, I see. I thought the issue was related to RBR not appearing in the WS chat, but it's actually a BE issue.

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 months ago

@kevinksullivan, @situchan Huh... This is 4 days overdue. Who can take care of this?

situchan commented 2 months ago

Not overdue

kevinksullivan commented 2 months ago

Still waiting on proposals. Looping in another BZ member as I'm going OOO

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

situchan commented 2 months ago

Not overdue

melvin-bot[bot] commented 1 month ago

@kevinksullivan, @VictoriaExpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

situchan commented 1 month ago

I think this is still waiting for proposals with correct RCA

VictoriaExpensify commented 1 month ago

Still waiting. I don't think this is a high enough priority to increase the bounty at this point. I'll post in Open Source to see if we can find any takers

VictoriaExpensify commented 1 month ago

https://expensify.slack.com/archives/C01GTK53T8Q/p1727154506176189

trjExpensify commented 1 month ago

Ah hm, when was this last tested? @robertjchen has been working on some changes recently that might have fixed this.

robertjchen commented 1 month ago

Hm, this is similar to the $0.00 in the LNH issue. I'll re-test here and see 🤔

melvin-bot[bot] commented 1 month ago

@kevinksullivan, @VictoriaExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 1 month ago

@VictoriaExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

robertjchen commented 1 month ago

Looking into this one from the backend.

robertjchen commented 1 month ago

Can no longer reproduce on prod- RBR appears to be displayed as needed, closing.

https://github.com/user-attachments/assets/2b725755-d9b1-4344-a937-9ea7fafcc765