Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2025-02-06] [HOLD for payment 2025-02-05] [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx #49103

Open
roryabraham opened this issue Sep 12, 2024 · 77 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Sep 12, 2024

Currently Held on #51942

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

Migrate src/libs/Navigation/AppNavigator/AuthScreens.tsx to use useOnyx instead of withOnyx.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834281376353079480
  • Upwork Job ID: 1834281376353079480
  • Last Price Increase: 2024-09-12
  • Automatic offers:
    • c3024 | Reviewer | 103949192
    • BhuvaneshPatil | Contributor | 103949194
Issue OwnerCurrent Issue Owner: @zanyrenney
@roryabraham roryabraham added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 12, 2024
@roryabraham roryabraham self-assigned this Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

@roryabraham roryabraham added the External Added to denote the issue can be worked on by a contributor label Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

@melvin-bot melvin-bot bot changed the title Migrate AuthScreens to useOnyx [$250] Migrate AuthScreens to useOnyx Sep 12, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

@roryabraham roryabraham changed the title [$250] Migrate AuthScreens to useOnyx [$100] Migrate AuthScreens to useOnyx Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

Upwork job price has been updated to $100

@BhuvaneshPatil
Copy link
Contributor

BhuvaneshPatil commented Sep 12, 2024

Dibs

@BhuvaneshPatil
Copy link
Contributor

Do we need to submit proposal?

@nkdengineer
Copy link
Contributor

Proposal

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

Migrate AuthScreens to useOnyx

What is the root cause of that problem?

This is an improvement

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

Remove the withOnyx here and use useOnyx hook. Also remove the type props here

const [session] = useOnyx(ONYXKEYS.SESSION);
const [lastOpenedPublicRoomID] = useOnyx(ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID);
const [initialLastUpdateIDAppliedToClient] = useOnyx(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT);

export default withOnyx<AuthScreensProps, AuthScreensProps>({

What alternative solutions did you explore? (Optional)

@zanyrenney
Copy link
Contributor

@roryabraham @c3024 want to take a look at the proposals here please?

@c3024
Copy link
Contributor

c3024 commented Sep 13, 2024

It is a simple enough task. I think it is fine to give it to @BhuvaneshPatil if he has not been assigned any other useOnyx migration. Otherwise @nkdengineer can be assigned.

🎀 👀 🎀 C+ Reviewed

Copy link

melvin-bot bot commented Sep 13, 2024

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

@BhuvaneshPatil
Copy link
Contributor

Thanks @c3024 ,
I am not assigned to any migration task

@roryabraham
Copy link
Contributor Author

Both @BhuvaneshPatil and @nkdengineer are now assigned to one other migration each, so I'm going to treat this one on a first-come-first-serve and give it to @BhuvaneshPatil

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 13, 2024
Copy link

melvin-bot bot commented Sep 13, 2024

📣 @c3024 🎉 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

Copy link

melvin-bot bot commented Sep 13, 2024

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

@BhuvaneshPatil
Copy link
Contributor

Copy link

melvin-bot bot commented Sep 16, 2024

@BhuvaneshPatil, @roryabraham, @zanyrenney, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@roryabraham
Copy link
Contributor Author

assigning this issue to @deetergp since he's been reviewing the PR(s)

@roryabraham roryabraham removed the Reviewing Has a PR in review label Jan 27, 2025
@roryabraham
Copy link
Contributor Author

latest PR was merged then reverted just a few minutes ago

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 28, 2025
@melvin-bot melvin-bot bot changed the title [$100] Migrate AuthScreens to useOnyx [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx Jan 28, 2025
Copy link

melvin-bot bot commented Jan 28, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.89-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2025-02-04. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jan 28, 2025

@c3024 @zanyrenney @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@deetergp
Copy link
Contributor

Melvin is incorrect as the last PR for this has been reverted, so it is not ready for payment.

@blazejkustra @WojtekBoman @c3024 Do we have a plan for re-implementing this without causing a regression?

@blazejkustra
Copy link
Contributor

I think @WojtekBoman is working on a fix, hopefully he will leave an update tomorrow 🙌

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 29, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx [HOLD for payment 2025-02-05] [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx Jan 29, 2025
Copy link

melvin-bot bot commented Jan 29, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.90-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2025-02-05. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jan 29, 2025

@c3024 @zanyrenney @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@WojtekBoman
Copy link
Contributor

WojtekBoman commented Jan 29, 2025

Melvin is incorrect as the last PR for this has been reverted, so it is not ready for payment.

@blazejkustra @WojtekBoman @c3024 Do we have a plan for re-implementing this without causing a regression?

I created a draft PR fixing this issue. It seems to work fine, but I know there are more sign in flows that should be checked thoroughly before I make it ready for review. Can we trigger builds for this PR and ask QA testers to check it?

Screen.Recording.2025-01-29.at.17.16.54.mov

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 30, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-02-05] [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx [HOLD for payment 2025-02-06] [HOLD for payment 2025-02-05] [HOLD for payment 2025-02-04] [$100] Migrate AuthScreens to useOnyx Jan 30, 2025
Copy link

melvin-bot bot commented Jan 30, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.91-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2025-02-06. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jan 30, 2025

@c3024 @zanyrenney @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@zanyrenney
Copy link
Contributor

tomorrow!

@c3024 please complete the checklist.

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2025
@c3024
Copy link
Contributor

c3024 commented Feb 6, 2025

@zanyrenney

The PR was reverted. A new PR #55966 is under works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: LOW
Development

No branches or pull requests