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 2022-12-08] [$250][Form Refactor] CompanyStep #9580

Closed
8 tasks
luacmartins opened this issue Jun 27, 2022 · 80 comments
Closed
8 tasks

[HOLD for payment 2022-12-08] [$250][Form Refactor] CompanyStep #9580

luacmartins opened this issue Jun 27, 2022 · 80 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@luacmartins
Copy link
Contributor

Coming from the New Expensify Forms design doc, we should refactor CompanyStep to use the new form component, follow the guidelines below:

Here's an example of a Form refactor: #9056

Guidelines

  1. Replace the form component with Form.js.
  2. Create a unique Onyx key in ONYXKEYS.FORM and pass it as the formID prop to Form.
  3. Pass a validate callback prop.
  4. Pass an onSubmit callback prop that calls the API via an action.
  5. Update all inputs wrapped by Form, following the guidelines in Refactor inputs.
  6. Remove any unused code.

Testing

Verify that:

  • UI looks as it did before the refactor
  • Values can be added and edited
  • Errors are highlighted correctly (input border)
  • Error messages show up correctly
  • Draft values are saved properly
  • Form alerts are displayed correctly
  • Clicking the fix the errors link focuses the first input with error
  • No duplicate submission of the form occurs (when it's already submitting)
@luacmartins luacmartins added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 labels Jun 27, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 27, 2022

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@adelekennedy
Copy link

@luacmartins is this something contributors can work on? Double checking before I open an Upwork job

@luacmartins
Copy link
Contributor Author

Yes! The Form refactor project is all open to contributors :)

@luacmartins
Copy link
Contributor Author

This issue is being put on hold due to push to page discussions, as per this comment.

@luacmartins luacmartins changed the title [Form Refactor] CompanyStep [HOLD] [Form Refactor] CompanyStep Jun 29, 2022
@adelekennedy
Copy link

Ty ty!

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2022
@adelekennedy
Copy link

on hold

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 4, 2022
@adelekennedy adelekennedy added Monthly KSv2 and removed Daily KSv2 labels Jul 6, 2022
@melvin-bot melvin-bot bot removed the Overdue label Jul 6, 2022
@adelekennedy
Copy link

on hold

@melvin-bot melvin-bot bot removed the Overdue label Jul 6, 2022
@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2022
@adelekennedy
Copy link

on hold

@melvin-bot melvin-bot bot removed the Overdue label Aug 8, 2022
@luacmartins
Copy link
Contributor Author

@adelekennedy removing hold! We can post this job on upwork and get contributors to start working on it!

@luacmartins luacmartins changed the title [HOLD] [Form Refactor] CompanyStep [Form Refactor] CompanyStep Aug 8, 2022
@luacmartins luacmartins added Weekly KSv2 Daily KSv2 and removed Monthly KSv2 Weekly KSv2 labels Aug 8, 2022
@adelekennedy
Copy link

let's gooooo

@mountiny
Copy link
Contributor

PR is almost merged here.

@JmillsExpensify
Copy link

Working through the commits, but looks like we should be through those really soon, ideally in the coming day or two.

@sketchydroide
Copy link
Contributor

this is now merged, there were some issue gettin the commits verified, but after creating a new PR this was fixed and everyine was happy

@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@sketchydroide
Copy link
Contributor

there was a deploy blocker associated, but that seems to have been fixed now

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Dec 1, 2022
@melvin-bot melvin-bot bot changed the title [$250][Form Refactor] CompanyStep [HOLD for payment 2022-12-08] [$250][Form Refactor] CompanyStep Dec 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 1, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.34-1 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 2022-12-08. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

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

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

@mananjadhav
Copy link
Collaborator

@sketchydroide @luacmartins This is supposed to be paid out today, but before that I would like to request a raise for this one. This is was held for a long time for AddressForm, and went through multiple rounds for the review. Compared to average issues, this was a bit bigger.

@luacmartins
Copy link
Contributor Author

@mananjadhav what price do you think is fair?

@adelekennedy
Copy link

pending payment on the discussion above

@mananjadhav
Copy link
Collaborator

I think 500$ should be fine here. Please note this also had a regression involved. So incase we don't want to increase then that is fine too.

@ravindra-encoresky
Copy link
Contributor

I think 500$ should be fine here. Please note this also had a regression involved. So incase we don't want to increase then that is fine too.

I would go with the first option :) , it's been long back and forth. Thanks, looking forward.

@luacmartins
Copy link
Contributor Author

$500 seems fair to me. Let's go with that.

@mananjadhav
Copy link
Collaborator

Thanks @luacmartins. @adelekennedy can you help with the payout here?

@adelekennedy
Copy link

@mananjadhav just hired you at $500

@mananjadhav
Copy link
Collaborator

Accepted @adelekennedy

@adelekennedy
Copy link

paid and paid!

@ravindra-encoresky
Copy link
Contributor

ravindra-encoresky commented Dec 13, 2022

@adelekennedy @mananjadhav I've received only $250.

@JmillsExpensify
Copy link

Re-opening until the contributor's question is answered.

@mallenexpensify
Copy link
Contributor

Sorry about that @ravindra-encoresky , not sure what happened. I just bonused you $250 so total compensation is now $500. Double checked Manan's contract and the payment was $500 there, re-closing

@ravindra-encoresky
Copy link
Contributor

Sorry about that @ravindra-encoresky , not sure what happened. I just bonused you $250 so total compensation is now $500. Double checked Manan's contract and the payment was $500 there, re-closing

Got it, thank you.

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

No branches or pull requests

10 participants