[PAY 29/04] [$250] Remove deprecated ReportActionUtils.getAllReportActions() method

[tgolen]

This is coming from #27262. You can read the issue description there to get the context behind the problem being solved and the mess being cleaned up.

Problem

ReportActionUtils.getAllReportActions() is called from several view components and other action files which is an anti-pattern.

Why this is important to fix

It maintains a more pure and exact flow of data through the react application. If the view is using report action data, then it needs to subscribe to the data in Onyx so that it's guaranteed that the data will never be stale or out-of-date.

Solution

• Use withOnyx() to load the data in view components
• Use Onyx.connect() to load the data in other action files
• Remove the usage of ReportUtils.getAllReportActions() or at least prevent it from being exported by adding a test to https://github.com/Expensify/App/blob/main/tests/actions/EnforceActionExportRestrictions.ts

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01944da5c2a9fee46a
• Upwork Job ID: 1773061588854910976
• Last Price Increase: 2024-04-17
• Automatic offers:
• Ollyws | Reviewer | 0
• godofoutcasts94 | Contributor | 0

Issue Owner

Current Issue Owner: @bfitzexpensify

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[ghost]

Proposal

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

Remove deprecated ReportActionUtils.getAllReportActions() method

What is the root cause of that problem?

We have ReportActionUtils.getAllReportActions()here :

App/src/libs/SidebarUtils.ts

Line 86 in 9dbec73

const reportActions = ReportActionsUtils.getAllReportActions(report.reportID);

here. :

App/src/libs/WorkspacesSettingsUtils.ts

Line 60 in 9dbec73

const reportActions = ReportActionsUtils.getAllReportActions(report.reportID);

and

App/src/libs/WorkspacesSettingsUtils.ts

Lines 69 to 72 in 9dbec73

	if (report.parentReportID) {
	const itemParentReportActions = ReportActionsUtils.getAllReportActions(report.parentReportID);
	itemParentReportAction = report.parentReportActionID ? itemParentReportActions[report.parentReportActionID] : {};
	}

and

App/src/libs/actions/IOU.ts

Lines 4556 to 4566 in 9dbec73

	function hasIOUToApproveOrPay(chatReport: OnyxEntry<OnyxTypes.Report> | EmptyObject, excludedIOUReportID: string): boolean {
	const chatReportActions = ReportActionsUtils.getAllReportActions(chatReport?.reportID ?? '');
	return Object.values(chatReportActions).some((action) => {
	const iouReport = ReportUtils.getReport(action.childReportID ?? '');
	const policy = ReportUtils.getPolicy(iouReport?.policyID);
	const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy) || canApproveIOU(iouReport, chatReport, policy);
	return action.childReportID?.toString() !== excludedIOUReportID && action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW && shouldShowSettlementButton;
	});
	}

and here :

App/src/libs/ReportUtils.ts

Lines 4259 to 4265 in 9dbec73

	function shouldHideReport(report: OnyxEntry<Report>, currentReportId: string): boolean {
	const currentReport = getReport(currentReportId);
	const parentReport = getParentReport(!isEmptyObject(currentReport) ? currentReport : null);
	const reportActions = ReportActionsUtils.getAllReportActions(report?.reportID ?? '');
	const isChildReportHasComment = Object.values(reportActions ?? {})?.some((reportAction) => (reportAction?.childVisibleActionCount ?? 0) > 0);
	return parentReport?.reportID !== report?.reportID && !isChildReportHasComment;
	}

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

we need to remove it ReportActionUtils.getAllReportActions() and change it according to mentioned comment here

Add a test here to verify that we do not export getAllReportActions function in ReportUtils

it('does not export getAllReportActions', () => {
    // @ts-expect-error the test is asserting that it's undefined, so the TS error is normal
    expect(ReportUtils.getAllReportActions()).toBeUndefined();
});

What alternative solutions did you explore? (Optional)

N/A

[ghost]

Updated Proposal

[EzraEllette]

Proposed Solution:

1. Mark getAllReportActions as deprecated similarly to getParentReport
2. Remove export and add test.
3. Modify action files and view components to use Onyx.

[melvin-bot]

📣 @EzraEllette! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[EzraEllette]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~015fecd86e270cefef?mp_source=share

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[allgandalf]

Proposal

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

Remove deprecated ReportActionUtils.getAllReportActions() method

What is the root cause of that problem?

We need to get away with getAllReportActions

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

ReportActionUtils.getAllReportActions() is used in utils only and not in any component (From what i searched)

So for the libs action, in we should use Onyx.connect:
One example would be as follows:

App/src/libs/actions/ReportActions.ts

Line 81 in e542ba7

const childActions = ReportActionUtils.getAllReportActions(reportAction.childReportID);

This would be changed to:

const childActions: OnyxCollection<ReportActions> = {};
Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
    callback: (actions, key) => {
        if (!key || !actions) {
            return;
        }

        const reportID = CollectionUtils.extractCollectionItemID(key);
        childActions[reportID] = actions;
    },
});

Similar changes would be applied at all other places and then finally remove:

App/src/libs/ReportActionsUtils.ts

Lines 772 to 774 in e542ba7

	function getAllReportActions(reportID: string): ReportActions {
	return allReportActions?.[reportID] ?? {};
	}

What alternative solutions did you explore? (Optional)

N/A

[ghost]

Updated Proposal with test step as well

[Ollyws]

@godofoutcasts94 was first here so I think it's fair he gets this one.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @Ollyws 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @godofoutcasts94 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[tgolen]

@godofoutcasts94 Are you able to get a PR started for this?

[EzraEllette]

@tgolen If there is a deadline you need to meet, I can deliver a PR by EOD.

[ghost]

Yes definitely today I will do it, I will start a draft PR. Actually, I was out somewhere I will start today itself or latest by tomorrow morning @tgolen

[melvin-bot]

📣 @EzraEllette You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[EzraEllette]

A PR can be expected once I finish testing on all platforms and and double check that my contribution meets the guidelines (Midday CST).

[melvin-bot]

@tgolen @Ollyws @bfitzexpensify @EzraEllette this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[EzraEllette]

Quick update: I'm having a tough time getting the android build running, even after following the steps and doing some googling. I just need to test the android functionality before I create the PR.

[EzraEllette]

#40046

[Ollyws]

Any chance we could bump the compensation on this one? It was a bit more work than expected. Thanks!

[tgolen]

I'm OK bumping it up to $250. Thanks for asking

[melvin-bot]

Upwork job price has been updated to $250

[EzraEllette]

Thanks!

[EzraEllette]

Can someone trigger the bot to send me an offer on upwork?

[tgolen]

Is the offer up here still valid? #39091 (comment)

[EzraEllette]

I already applied to that job. I wonder if it is still assigned to godofoutcasts?

[tgolen]

@bfitzexpensify Could you help us out here, please?

[bfitzexpensify]

I've hired you on Upwork @EzraEllette.

Looks like the PR was deployed to prod two days ago.

Updating this to daily and updating the title to reflect the payout date.

[bfitzexpensify]

All done here.

[Ollyws]

@bfitzexpensify Also C+ payment due for me, thanks.