Reviewing Maintainer Feedback0:00
Jess has responded to our pull request, and guess what? She thinks it looks really cool, which is great news. But there are a few things she'd like to see adjusted. So, for one, she wonders if we could expose methods for each prompt directly, instead of having to call add. So, rather than saying add closure text, add closure select, add closure confirm, we would just call text, select, confirm directly on the builder itself. Some helper methods, essentially. I think that makes perfect sense, and we can absolutely do that. What's next?
I think that makes perfect sense, and we can absolutely do that. What's next? Regarding the revert behavior, I'm a bit hesitant about the complexity this feature might introduce. Oof. This one hurts a little, just because we've spent so long implementing custom revert functionality. But it's important when contributing to open source to try and see things from the maintainer's perspective. So, she says, in my mind, steps are almost like creating a form on the web, where no actions are taken until the entire form is completed. And you know what? That makes sense.
where no actions are taken until the entire form is completed. And you know what? That makes sense. If we were building a HTML form, you wouldn't process any of the inputs until you'd clicked submit. So, I guess we could remove revert logic. And it makes more sense when we take into account point three. I wondered whether the function could be named form instead of steps to continue the parallel between methods like select and select. So, renaming a few functions, parameters, and methods so that we have this analogy of a HTML form.
So, renaming a few functions and parameters and methods so that we have this analogy of a HTML form. You'll get this a lot when contributing to open source. You'll want to fight against the maintainer when they give you suggestions and ask for changes. But it's important to realize that at the end of the day, you're going to walk off and you're not going to touch this code again other than using it as an end user. But you're going to leave the maintainer with all of the work of looking after your code for the long run.
Listing Refactor Action Items1:52
But you're going to leave the maintainer with all of the work of looking after your code for the long run. So, yeah, if at all possible, see things from their perspective, go ahead and make the changes they request. Let's go ahead and distill Jess's feedback into a few action points that we can take on our code. Here we are. I've created a quick text file. So, we want to allow bypassing add to call prompt functions directly. We'll remove the revert behavior and then we'll do the renaming of steps to form, run to submit, key to name.
Renaming to Form API2:16
We'll remove the revert behavior and then we'll do the renaming of steps to form, run to submit, key to name. I'm going to reorder these so that we can tackle, for want of a better phrase, the low-hanging fruit first. So, for example, renaming, that's a very simple IDE operation. Same with removing the revert behavior. It shouldn't be too difficult to do that using the power of PhpStorm. And then finally, we're adding this new functionality at the end. Let's start with renaming. Let's go ahead and open up our steps playground.
Let's start with renaming. Let's go ahead and open up our steps playground. Jess wants us to rename steps to form. So, in PhpStorm, I can use Shift-F6, type form and hit enter. Inside form, we return stepBuilder. Let's change that to formBuilder using Shift-F6 again. formBuilder. Let's open up formBuilder. Currently, we have run. Jess would like us to rename that to submit. So, again, Shift-F6. We change to submit and hit enter.
Jess would like us to rename that to submit. So, again, Shift-F6. We change to submit and hit enter. And I still think we could call each individual component of a form a step. But perhaps to resolve the grouping of these files in the finder, we could actually call it a form step. So, again, I'm just going to click into step here, Shift-F6, and then alter it to form step. And you'll see that renames the file, which places form builder and form step together, giving us a logical grouping. Very nice.
and form step together, giving us a logical grouping. Very nice. If we take another quick look at our actions, we also want to rename key to name. So, let's go ahead and open up our steps playground. And we have an instance of that just here. So, again, I'll click on the named argument, and I'm going to use Shift-F6, type name, hit enter, and that should go ahead and update the form builder. Yeah, you can see it's changed the parameter.
and that should go ahead and update the form builder. Yeah, you can see it's changed the parameter. However, for the form step, I'm pretty sure this is still called a key. Yeah. So, again, I'll come into formStep. I'll click key, Shift-F6, and then rename this to name. Finally, we can go ahead and rename a few of our files. So, for example, steps no longer makes sense. Let's rename the playground to form.php. And then where we have stepsTest here, we can rename this to formTest.php.
Let's rename the playground to form.php. And then where we have steps test here, we can rename this to form test.php. Let's go ahead and run all the tests in this file to make sure we haven't broken anything. Everything still seems to work. And we'll have a quick look through our playground to make sure that that's been updated correctly, which is now obviously form.php instead. So, we call form. We can call add as many times as we'd like.
So, we call form. We can call add as many times as we'd like. When we pass a name, we're using name instead of key now. And then finally, at the end, we call submit to actually fire the form. So, that's our first action point complete. I'll remove it. And just before we move on to the second, let's go to the terminal, type wip to save our current progress so that we can come back to this point at any time. Right, now on to removing revert behavior.
Removing Revert Functionality5:10
so that we can come back to this point at any time. Right, now on to removing revert behavior. Let's go into our form builder. And obviously, we have this revert parameter that we currently pass. So, if I right-click in phpStorm, I can go to refactor, change signature. I'll click revert, click minus, and then refactor, and hit continue. And you'll see it completely removes that parameter and any usages of that parameter. So, if I go back to my form playground, you can see now that we never actually pass any form of revert.
So, if I go back to my form playground, you can see now that we never actually pass any form of revert. It's removed all instances from our code. Super cool. We no longer need this if statement. It's redundant. And of course, we need to also update formStep because it will no longer require a revert parameter. So, we'll come into here. Let's right-click, refactor, change signature.
So, we'll come into here. Let's right-click, refactor, change signature. Let's go ahead and remove revert, refactor, continue. And now if we come back to FormBuilder, yeah, you can see it's gone completely. Thinking about it, previousStep was actually only ever used to calculate revert. And seeing as that custom behavior is now gone, we could also remove previousStep. So, again, let's click this, go to refactor, change signature. I'll remove previous, refactor, and continue. And now looking through our code, yeah, we don't need canRevert anymore.
I'll remove previous, refactor, and continue. And now looking through our code, yeah, we don't need canRevert anymore. Let's see where that's used. Here we go. So, let's remove that. And thinking about it, we shouldn't actually allow reverting if you're on the first step because you can't revert beyond the first step. So, rather than removing this entirely, I'm going to say, well, is the index greater than zero? If that's the case, we can revert using.
I'm going to say, well, is the index greater than zero? If that's the case, we can revert using. Otherwise, we'll prevent reverting. Just a minor change there. But that means, hopefully, if I click this again, we now have no usages of canRevert, meaning we can remove it entirely from a step. The same is true of this revert function here. We don't actually use it anymore because previous and revert functionality has been removed.
We don't actually use it anymore because previous and revert functionality has been removed. Let's see where it was called. Yeah, it was called here inside our submit method. So, let's remove that. And, in fact, we can now simplify this if statement, right? We could just say, was the code reverted? If so, index minus minus. Otherwise, index plus plus. And we'll clean that up nicely, come back to form step,
Otherwise, index++. And we'll clean that up nicely, come back to formStep, and now remove revert. Very good. So, this is far simpler than it was before. There's even an argument to going back to an array tuple. But I'm going to leave it in place just in case there are other changes down the line that we want to make to the formStep. Back in our formBuilder, previousStep is now no longer used,
that we want to make to the form step. Back in our form builder, previous step is now no longer used, so we don't need to calculate that anymore. Let's take a quick look through, see if there's anything else that's out of place. No. Everything else makes sense, I think. So, let's go into our test, form_test.php, because there are several tests that no longer actually make sense in our code base.
Updating Tests and Checks8:12
because there are several tests that no longer actually make sense in our code base. Let's take a look. It can key a response by a given string. That's fine. It can prevent a step from being reverted. No, that's no longer supported. So, let's remove the test. It can revert steps. It can run custom logic
It can revert steps. It can run custom logic when reverting a step is no longer supported. So, again, we'll remove that. It can run multiple steps. Fine. It does not allow reverting normal prompts. It passes all available responses to each step. It passes all available responses to each step revert closure.
It passes all available responses to each step revert closure. Well, there is no step revert closure anymore, so, again, that can go. And then we'll run all of the tests in this file. All five are passing. I think that means that we have successfully removed the revert logic. Happy days. Back in the terminal,
Happy days. Back in the terminal, we'll run our work in progress alias again to make sure that everything is saved up to this point now that we're happy with it. And I'll tell you what, just to make sure we've not missed anything, why don't we run vendor/bin/phpstan to ensure that we haven't broken any types along the way. All good there.
to ensure that we haven't broken any types along the way. All good there. And then finally, we'll run VendorBin.php to run our test suite. And all of the tests are still passing. So, I think, I think we have a successful refactor on our hands. We can go back to our little scratch file here. We've finished this one, and we're ready to tackle our final task.
We've finished this one, and we're ready to tackle our final task.
