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.41k stars 2.8k forks source link

[HOLD for payment 2024-10-08] [$75] Migrate withWritableReportOrNotFound from withOnyx to useOnyx #49110

Open roryabraham opened 2 weeks ago

roryabraham commented 2 weeks ago

Coming from https://expensify.slack.com/archives/C01GTK53T8Q/p1725973460005309?thread_ts=1725905735.105989&cid=C01GTK53T8Q

Migrate src/pages/iou/request/step/withWritableReportOrNotFound.tsx to use useOnyx instead of withOnyx.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834285173135937117
  • Upwork Job ID: 1834285173135937117
  • Last Price Increase: 2024-09-12
  • Automatic offers:
    • brunovjk | Reviewer | 103949076
Issue OwnerCurrent Issue Owner: @brunovjk
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @miljakljajic (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 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

Upwork job price has been updated to $75

abhinaybathina commented 2 weeks ago

Proposal

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

Migrate from withOnyx HOC to useOnyx hook

What is the root cause of that problem?

withOnyx HOC is deprecated

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

Whatever props are coming from withOnyx HOC in withWritableReportOrNotFound should be replaced with useOnyx hook

What alternative solutions did you explore? (Optional)

Nodebrute commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-09-12 17:55:22 UTC.

Proposal

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

Migrate withWritableReportOrNotFound from withOnyx to useOnyx

What is the root cause of that problem?

Feature request

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

We can remove withOnyx from here and use useOnyx

const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID || -1}`);
    const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
    const [reportDraft] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID || -1}`);

Optional: we can also remove type props related to these. Note: we might need to do some cleanup

What alternative solutions did you explore? (Optional)

We can remove withOnyx from here and use useOnyx

const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID ?? -1}`);
    const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
    const [reportDraft] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID ?? -1}`);
brunovjk commented 2 weeks ago

Reviewing proposals today

roryabraham commented 2 weeks ago

going to treat these migrations on a first-come-first-serve basis while avoiding assigning the same person to multiple

melvin-bot[bot] commented 2 weeks ago

📣 @brunovjk 🎉 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[bot] commented 2 weeks ago

📣 @abhinaybathina 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 📖

abhinaybathina commented 2 weeks ago

Hi team, thanks for assigning this to me. I've checked upwork but I didn't receive any job offer. Can you please look into it? @roryabraham . Thanks!

abhinaybathina commented 2 weeks ago

Sorry never mind, I've applied to the Upwork job. Please expect the PR to be ready for review by today EOD. Thanks!

abhinaybathina commented 2 weeks ago

Here is the PR, please review. Thanks!

https://github.com/Expensify/App/pull/49200

brunovjk commented 2 weeks ago

going to treat these migrations on a first-come-first-serve basis while avoiding assigning the same person to multiple

Thank you @roryabraham and @abhinaybathina. I will review the PR first thing on Monday.

melvin-bot[bot] commented 1 week ago

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

miljakljajic commented 1 week ago

Heading out on parental leave - assigning to another BZ team member to take over.

Next steps: PR is being reviewed today or tomorrow as per https://github.com/Expensify/App/pull/49200#issuecomment-2363761717

abhinaybathina commented 6 days ago

Hi @roryabraham. Thanks for reviewing the PR and merging it. I've applied for this Upwork job but there is no update on Upwork. Can you please check?

melvin-bot[bot] commented 5 days ago

@garrettmknight, @abhinaybathina, @roryabraham, @brunovjk Whoops! This issue is 2 days overdue. Let's get this updated quick!

roryabraham commented 4 days ago

@abhinaybathina @garrettmknight will help handle the payment on Upwork. Thanks. As stated in our contributing guidelines, jobs are typically paid out after the code has been on production for a week without causing regressions.

This PR was deployed to staging yesterday.

brunovjk commented 2 days ago

PR on staging

abhinaybathina commented 2 days ago

I have tested on staging, working fine

garrettmknight commented 2 days ago

Payment Summary:

garrettmknight commented 2 days ago

@abhinaybathina please accept the offer.

abhinaybathina commented 2 days ago

@garrettmknight Accepted

brunovjk commented 2 days ago

I will complete the BZ checklist before payday. Thanks.