[HOLD for payment 2022-12-02] [$1,000] [Form Refactor] RequestorStep

[luacmartins]

Coming from the New Expensify Forms design doc, we should refactor RequestorStep 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)

[melvin-bot]

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

[NicMendonca]

Job post -- https://www.upwork.com/jobs/~012c4ed44ee1b967a1

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

Triggered auto assignment to @neil-marcellini (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Murodjon000]

Hi 👋 @neil-marcellini, may i update RequsterStep, i checked the details, the steps that i will do using Form.js component inside RequestorStep component properly refactoring the code. In old one clicking the fix the errors is not working as expected, While refactoring i will stick to the instructions above.

[luacmartins]

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

[luacmartins]

@Murodjon000 thanks for the interest. However, this issue is now on hold and we won't assign anyone to it until the hold is removed.

[neil-marcellini]

On HOLD, but we said we would discuss push to page this week, have we?

[neil-marcellini]

^ Still on hold unless the push to page discussion is complete.

[neil-marcellini]

Holding

[abekkala]

@mollfpr has been hired in Upwork

@Santhosh-Sellavel I still can not find your profile in Upwork in order to hire you

[JmillsExpensify]

Special note: Please treat this issue with urgency, as it's a headline initiative in our product roadmap at the moment. Standardizing our approach here will help us get a handle on related bugs. Thanks!

[Santhosh-Sellavel]

@mollfpr We are on a clock here, if get it merged by tomorrow we are eligible for a bonus!

[mollfpr]

@Santhosh-Sellavel On it! Need to finish some of the recordings.

[mollfpr]

Pulled you guys to the PR @neil-marcellini @Santhosh-Sellavel!

I found a bug when using the StatePicker need to double-click the option to select the value, I guess it's because of the onBlur of the text input. It's not related to this refactor, so we should log that issue after this is complete.

[kevinksullivan]

FYI @abekkala I hired Santhosh. You can typically find him under "My Hires". Let's get this one knocked out!

[abekkala]

That cross post above was done by mistake - my clipboard pasted the wrong link - ugh.

[abekkala]

@mollfpr @Santhosh-Sellavel Is there an update on the PR for this?

[mollfpr]

@abekkala Yup, it was reviewed this morning. I just update the PR, and ready for another review.

[Santhosh-Sellavel]

Review in progress

[neil-marcellini]

I just merged the PR.

[Santhosh-Sellavel]

PR is already merged

[melvin-bot]

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

• Refactor requestor step form #12766

If no regressions arise, payment will be issued on 2022-12-02. 🎊

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

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mollfpr / @Santhosh-Sellavel / @neil-marcellini] The PR that introduced the bug has been identified. Link to the PR: N/A refactor PR
• [@mollfpr / @Santhosh-Sellavel / @neil-marcellini] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A refactor PR
• [@mollfpr / @Santhosh-Sellavel / @neil-marcellini] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A refactor PR
• [@abekkala] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[abekkala]

@luacmartins I'm having a hard time determining if there is a tests section that matches this in TestRail. I've looked under the Request Money and Split bills section.
Not only am I having a hard time determining a matching test, I'm not sure what the test steps would be as this GH wasn't necessarily I bug reproduction.
Does this even require a regression test?

[luacmartins]

@abekkala thanks for checking. This PR should already be covererd in the regression tests for Add/Delete Bank account so we should be good here!

[abekkala]

Thanks @luacmartins!

[abekkala]

Payments have been made to @Santhosh-Sellavel and @mollfpr
We're all set here! Closing

Thank you 🎉