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 2023-08-07] [$250] Migrate WalletStatementPage.js to function component #16303

Closed
1 task
marcaaron opened this issue Mar 20, 2023 · 50 comments
Closed
1 task
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@marcaaron
Copy link
Contributor

marcaaron commented Mar 20, 2023

Class Component Migration

Filenames

Task

  • We currently have some class components in our codebase that we would like to refactor to a function component.
  • Here's a link with some general advice on how to refactor a class component to a function component: https://react.dev/reference/react/Component#alternatives
  • If you need additional guidance, please ask in #expensify-open-source
  • Test for any regressions and verify that there are no breaking changes
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01120a3696c71dc1f1
  • Upwork Job ID: 1674855096251645952
  • Last Price Increase: 2023-06-30
@marcaaron marcaaron added Engineering Improvement Item broken or needs improvement. labels Mar 20, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Mar 20, 2023
@Expensify Expensify unlocked this conversation Mar 21, 2023
@marcaaron marcaaron changed the title [HOLD] Migrate WalletStatementPage.js to function component [HOLD] [$250] Migrate WalletStatementPage.js to function component Apr 13, 2023
@MelvinBot
Copy link

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

@marcaaron
Copy link
Contributor Author

Heads up! The pricing for this issue has been adjusted based on the scope of the work and the fact that no complex bug investigations or proposal are required.

@muxriddinmuqimov77
Copy link

I'd like to work on this issue when it goes to external

@s-alves10
Copy link
Contributor

I'd love to work on this.

@Li357 Li357 self-assigned this Jun 6, 2023
@marcaaron marcaaron changed the title [HOLD] [$250] Migrate WalletStatementPage.js to function component [$250] Migrate WalletStatementPage.js to function component Jun 26, 2023
@marcaaron
Copy link
Contributor Author

Taking this one off HOLD.

@Li357 Li357 removed their assignment Jun 26, 2023
@code0406
Copy link
Contributor

I am eager to take on this task.

@dayana7204
Copy link
Contributor

I am ready to work on this ticket :)

@vdem0115
Copy link

I would like to work on this issue.

@marcaaron marcaaron added Daily KSv2 NewFeature Something to build that is a new item. External Added to denote the issue can be worked on by a contributor and removed Improvement Item broken or needs improvement. labels Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 30, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Everybody was OOO, so I assigned the only eligible person.

@melvin-bot
Copy link

melvin-bot bot commented Jun 30, 2023

Current assignee @MitchExpensify is eligible for the External assigner, not assigning anyone new.

@MitchExpensify
Copy link
Contributor

Bump @Santhosh-Sellavel 👍

@PauloGasparSv
Copy link
Contributor

Bump @Santhosh-Sellavel 👍

Hey @MitchExpensify the review is blocked because of me now I think.

Since this change is related to the Wallet @Santhosh-Sellavel and @s-alves10 can't really test this, I'm trying to figure out how they can test it though (I think it's related to enabling the wallet beta, trying to do this on my local dev environment first).

I only stopped to focus on this now but @s-alves10 also tried reaching out in the Cplus channel for an account that can test this but didn't get help there.

Will give an update here and reach out to the team if I can't find out how to test this.

@MitchExpensify
Copy link
Contributor

Sounds good, thanks @PauloGasparSv !

@PauloGasparSv
Copy link
Contributor

Hey @MitchExpensify can you please re-assign this to another engineer?

I'm OOO from now till next week (tried to review this before going OOO but couldn't, sorry for that).

To whoever takes this on and isn't sure on how to set up a wallet, please refer to this S.O. https://stackoverflowteams.com/c/expensify/questions/10083 and can you please help with this comment?

@melvin-bot
Copy link

melvin-bot bot commented Jul 18, 2023

Triggered auto assignment to @AndrewGable (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@MitchExpensify
Copy link
Contributor

Hey @MitchExpensify can you please re-assign this to another engineer?

Done!

@AndrewGable Looks like the latest on this is that we ran into problem testing it #16303 (comment)

@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 2023

@AndrewGable, @MitchExpensify, @Santhosh-Sellavel, @s-alves10 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

  • when @s-alves10 got assigned: 2023-07-03 21:15:52 Z
  • when the PR got merged: 2023-07-27 20:28:59 UTC
  • days elapsed: 17

On to the next one 🚀

@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

@AndrewGable, @MitchExpensify, @Santhosh-Sellavel, @s-alves10 Huh... This is 4 days overdue. Who can take care of this?

@MitchExpensify
Copy link
Contributor

Not overdue - PR hit staging so payment 7 days after it hit production assuming no regressions is our next step

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jul 31, 2023
@melvin-bot melvin-bot bot changed the title [$250] Migrate WalletStatementPage.js to function component [HOLD for payment 2023-08-07] [$250] Migrate WalletStatementPage.js to function component Jul 31, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.47-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 2023-08-07. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@MitchExpensify
Copy link
Contributor

MitchExpensify commented Aug 1, 2023

Reminder added to cal to pay out on 8/7 👍

Payment summary:

Reporting - None
C @s-alves10: $250
C+ @Santhosh-Sellavel : $250

Does that look right to you both? 🙇

@s-alves10
Copy link
Contributor

This migration took long because it's hard to test. Please check the review history of the PR.
Thanks

@MitchExpensify
Copy link
Contributor

This migration took long because it's hard to test. Please check the review history of the PR.

Yes I noticed that in the PR but I'm not sure we can take that into account for bonuses. For example we do not consistently factor in review delays because it is too time consuming. That said, if @AndrewGable agrees the difficulty testing should be factored in I'm happy to ignore the 50% late merge penalty!

@AndrewGable
Copy link
Contributor

There was also some internal OOO that added to this, no need to add the penalty.

@MitchExpensify
Copy link
Contributor

Ok cool! Thanks @AndrewGable & @s-alves10 - I've edited the payment summary accordingly

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 6, 2023
@MitchExpensify
Copy link
Contributor

Paid! Thanks everyone

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 Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests