[$250] Migrate MoneyRequestConfirmationList to useOnyx

[neil-marcellini]

Problem

In this PR I made some changes to MoneyRequestConfirmationList, which trigged the ESLint changed files check to error saying that we need to migrate withOnyx to useOnyx

Solution migrate to useOnyx

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021844152421070937252
• Upwork Job ID: 1844152421070937252
• Last Price Increase: 2024-10-09
• Automatic offers:
• shubham1206agra | Reviewer | 104434485
• NJ-2020 | Contributor | 104434486

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

Triggered auto assignment to @abekkala (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]

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

[melvin-bot]

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

[Shahidullah-Muffakir]

Proposal

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

We are trying to migrate the MoneyRequestConfirmationList component from using the withOnyx HOC to the useOnyx hook.

What is the root cause of that problem?

The current implementation uses withOnyx, which is an older pattern for connecting components to Onyx data.

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

In this file:

App/src/components/MoneyRequestConfirmationList.tsx

Line 82 in bfdd19b

type MoneyRequestConfirmationListProps = MoneyRequestConfirmationListOnyxProps & {

1. Remove the withOnyx HOC from the MoneyRequestConfirmationList component.
2. Add the useOnyx hook inside the MoneyRequestConfirmationList component to fetch these data:

App/src/components/MoneyRequestConfirmationList.tsx

Lines 978 to 983 in bfdd19b

	export default withOnyx<MoneyRequestConfirmationListProps, MoneyRequestConfirmationListOnyxProps>({
	policyCategories: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
	},
	policyCategoriesDraft: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID}`,

[NJ-2020]

Proposal

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

Migrate MoneyRequestConfirmationList to useOnyx

What is the root cause of that problem?

N/A

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

Change from withOnyx here:

App/src/components/MoneyRequestConfirmationList.tsx

Lines 978 to 1008 in bfdd19b

	export default withOnyx<MoneyRequestConfirmationListProps, MoneyRequestConfirmationListOnyxProps>({
	policyCategories: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
	},
	policyCategoriesDraft: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID}`,
	},
	policyTags: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
	},
	defaultMileageRate: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
	selector: DistanceRequestUtils.getDefaultMileageRate,
	},
	mileageRates: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
	selector: (policy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(policy),
	},
	policy: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
	},
	policyDraft: {
	key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`,
	},
	lastSelectedDistanceRates: {
	key: ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES,
	},
	currencyList: {
	key: ONYXKEYS.CURRENCY_LIST,
	},
	})(

to useOnyx:

const [policyCategoriesReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID || '-1'}`);
const [policyCategoriesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID || '-1'}`);
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID || '-1'}`);
const [defaultMileageRate] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID || '-1'}`, {
    selector: DistanceRequestUtils.getDefaultMileageRate,
});
const [mileageRatesReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID || '-1'}`, {
    selector: (policy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(policy),
});
const [policyReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID || '-1'}`);
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID || '-1'}`);
const [lastSelectedDistanceRates] = useOnyx(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES);
const [currencyList] = useOnyx(ONYXKEYS.CURRENCY_LIST);

And remove the MoneyRequestConfirmationListOnyxProps type

App/src/components/MoneyRequestConfirmationList.tsx

Line 82 in bfdd19b

type MoneyRequestConfirmationListProps = MoneyRequestConfirmationListOnyxProps & {

App/src/components/MoneyRequestConfirmationList.tsx

Lines 53 to 57 in bfdd19b

	type MoneyRequestConfirmationListOnyxProps = {
	/** Collection of categories attached to a policy */
	policyCategories: OnyxEntry<OnyxTypes.PolicyCategories>;
	/** Collection of draft categories attached to a policy */

And the parameters from the function
And also remove this

App/src/components/MoneyRequestConfirmationList.tsx

Lines 1038 to 1039 in bfdd19b

	lodashIsEqual(prevProps.defaultMileageRate, nextProps.defaultMileageRate) &&
	lodashIsEqual(prevProps.lastSelectedDistanceRates, nextProps.lastSelectedDistanceRates) &&

App/src/components/MoneyRequestConfirmationList.tsx

Line 1041 in bfdd19b

lodashIsEqual(prevProps.currencyList, nextProps.currencyList) &&

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[abekkala]

@anmurali I'll be ooo until Mon Oct 21; then I can take this back.

STATUS: proposals have been posted, waiting on one to be chosen

[melvin-bot]

@abekkala, @anmurali, @neil-marcellini, @shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[neil-marcellini]

I like @NJ-2020's proposal best so far since it's sufficiently detailed while the one before it was not quite detailed enough. Hiring! @shubham1206agra please try to review proposals more promptly next time or re-assign.

[melvin-bot]

📣 @shubham1206agra 🎉 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]

📣 @NJ-2020 🎉 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 📖

[NJ-2020]

PR ready

cc: @shubham1206agra

[abekkala]

I'm back from ooo - unassinging @anmurali

[abekkala]

PR not yet deployed

Fix: @NJ-2020
PR Review: @shubham1206agra

[melvin-bot]

This issue has not been updated in over 15 days. @abekkala, @neil-marcellini, @shubham1206agra, @NJ-2020 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!

[abekkala]

looks like this one had a regression

[shubham1206agra]

This is not merged yet.

[neil-marcellini]

Somehow this was migrated elsewhere, so our work here is done. I don't think any payment is warranted unfortunately, but please feel free to explain if you disagree.

[shubham1206agra]

@neil-marcellini Payment is required here as per https://docs.google.com/document/d/1BvohU05MTaHnjOD_vwJv_aqDAirv-ChkyRnKCAvOVyQ/edit?tab=t.0#bookmark=id.by6zfdtawcqn

[neil-marcellini]

Ah I see. Thanks for letting me know. Re-opening to handle payment.

[abekkala]

@neil-marcellini Maybe I was confused by the PR (or most likely looking at the wrong PR) There are several PRs linked into this issue.

This PR: Migrate MoneyRequestConfirmationList to useOnyx #50848 was Closed and never merged. are you saying that the following payments should still be sent?

Fix: @NJ-2020 [$250]
PR Review: @shubham1206agra [$250]

[shubham1206agra]

@abekkala Yes

[abekkala]

PAYMENT SUMMARY:

Fix: @NJ-2020 [$250] Offer
PR Review: @shubham1206agra [$250] previous offer expired as it was never accepted. New one sent here.

[abekkala]

@NJ-2020 payment sent and contract ended - thank you! 🎉

[shubham1206agra]

@abekkala Offer accepted

[abekkala]

@shubham1206agra payment sent and contract ended - thank you! 🎉