create maker for symfony casts reset password bundle #567

jrushlow · 2020-03-27T22:20:16Z

Adds a make:reset-password maker to create the classes, entities, and twig templates required by Symfony Casts Reset Password Bundle. This complements the bundle by doing the leg work to provide a true out-of-the-box solution for handling forgotten passwords in traditional Symfony applications.

test suite depends on PR explicitly set framework.router.utf8 value in test kernel #565
depends on flex recipe adds recipe for symfony casts reset password bundle recipes#748

jrushlow · 2020-03-30T01:37:11Z

src/Validator.php

@@ -167,6 +167,15 @@ public static function validateDoctrineFieldName(string $name, ManagerRegistry $
        return $name;
    }

+    public static function validateEmailAddress(string $email): string


method name could be as is or just emailAddress. As a whole, the class has functions like validate.. and notBlank. Although Validator::class naming conventions are outside the scope of this PR, may be worth considering which name would be appropriate for this method.

src/Maker/MakeResetPassword.php

weaverryan · 2020-04-03T11:05:59Z

src/Maker/MakeResetPassword.php

+            ->addArgument('password-setter')
+        ;
+
+        $io->section('- Email Templates -');


My initial instinct is that this is cool :)

hmm, off the top of my head, all of these args are "required". Do we need InputArgument::REQUIRED for the last 4 even though the interactive security guesser sort of "enforces" this..

src/Maker/MakeResetPassword.php

weaverryan · 2020-04-03T11:16:47Z

src/Maker/MakeResetPassword.php

+            throw new RuntimeCommandException(sprintf('The file "%s" does not exist. This command needs that file to accurately build the reset password config.', $path));
+        }
+
+        $manipulator = new YamlSourceManipulator($this->fileManager->getFileContents($path));


This is a "funny" situation. We sorta don't want to "update" this file (and leave the old comments). We probably want to completely override the file contents with the new ones. Specifically, we don't want our "Replace symfonycasts.reset_password.fake_request_repository with the full..." text to be in this file when we finish.

Perhaps we could check if request_password_repository: symfonycasts.reset_password.fake_request_repository is the only config in that file. And:

A) If it is, use the normal Yaml file to completely dump new contents
B) If not for some reasons, then use the manipulator to update the file more carefully

done,

if the config file doesn't exist, we assume dev is using a customized config. Remind dev to set correct repository and move on.

if the config exists with only 1 key, assume config was created by flex; create a new config file.

if config exists with more than 1 key, assume the dev modified the config before running maker. Replace repository value and preserve everything else.

src/Maker/MakeResetPassword.php

src/Resources/skeleton/resetPassword/ResetPasswordController.tpl.php

weaverryan · 2020-04-03T11:27:41Z

src/Resources/skeleton/resetPassword/twig_check_email.tpl.php

+{% block title %}Password Reset Email Sent{% endblock %}
+
+{% block body %}
+    <p>An email has been sent that contains a link you can click to reset your password. This link will expire in {{ tokenLifetime|date('g') }} hour.</p>


... a link that you can click...

Also, I guess we should say "hour(s)" on the end. I don't think we should try to introduce the translator so that we can "translate" hour versus hours. Or we could add the proper logic, though it's a little ugly:

{{ tokenLifetime|date('g') }} hour{{ tokenLifetime|date('g') != 1 ? 's' }} hour

If we did that, I might also pass tokenLifetimeHours into the template for convenience.

I went with hours(s).

From a technical stand point, using the proper logic makes sense. But, I suspect most people will be changing the format of this template to suite their needs (match UI/business themes, etc..). So perhaps we would introduce the "proper logic" at a later time if it is requested..

src/Resources/skeleton/resetPassword/twig_email.tpl.php

src/Resources/skeleton/resetPassword/ResetPasswordController.tpl.php

weaverryan · 2020-04-03T13:49:00Z

src/Maker/MakeResetPassword.php

+                'repository_class_name' => $repositoryClassNameDetails->getFullName(),
+                'user_full_class_name' => $userClassNameDetails->getFullName(),
+            ]
+        );


Let's save this for later (cause I think it will take some work), but:

It would be ideal if we could use EntityClassGenerator to generate the entity and repository classes. That's our centralized entry point for creating these classes and making sure they all look uniform. We would use that class and then use the ClassSourceManipulator to add in our custom code (that's the tricky part - writing PHP Parser code to add methods, etc take a bit of work to get right).

made a note on the board to implement in the near future..

src/Resources/skeleton/resetPassword/twig_request.tpl.php

src/Resources/skeleton/resetPassword/twig_reset.tpl.php

src/Maker/MakeResetPassword.php

tests/fixtures/MakeResetPassword/config/bundles.php

tests/fixtures/MakeResetPassword/src/Entity/User.php

weaverryan · 2020-04-03T14:09:12Z

tests/fixtures/MakeResetPassword/src/Entity/User.php

+class User implements UserInterface
+{
+
+    private $email;


No setter for this?

off the top of my head, the bundle doesn't care about the setter, just that it is able to get the email, same holds true for guesser.

weaverryan · 2020-04-03T14:10:21Z

tests/Maker/MakeResetPasswordTest.php

+                'app_home',
+            ])
+            ->addExtraDependencies('security-bundle')
+            ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeResetPassword'),


We should be a functional test inside the skeleton. For simplicity, it could just go to /reset-password and check for a 200 status code. The gold standard would be to actually fill out the form, capture the reset password link somehow, then go to THAT URL and reset the password :). We could do that "gold standard" never or later ;)

done.

I added a test to check /reset-password and grab the status. I need to add tests for the other routes as well before the merge.

Ultimately I would like to get to the "gold standard." Perhaps after the merge and before the 1.x bundle release..

jrushlow · 2020-04-03T19:59:07Z

TODO - I keep deleting tests/tmp/.gitignore locally between tests. Storm auto magically adds that change on ctrl+k, not to self: remember to put that back when all is said and done....

weaverryan · 2020-04-05T10:50:43Z

Merged! A HUGE thanks to @jrushlow for his MASSIVE work on this feature and also @romaricdrigon who kick-started this a long time ago and kept pushing on it. I'm super happy to not only have this feature in Maker, but for it to be really, really nice. ❤️

jrushlow force-pushed the feature/reset-password-bundle branch 2 times, most recently from 3649444 to bb9ffce Compare March 28, 2020 13:15

weaverryan mentioned this pull request Mar 28, 2020

remove forgotten password maker #569

Merged

jrushlow force-pushed the feature/reset-password-bundle branch from 49dd1d8 to 795bab5 Compare March 29, 2020 16:37

weaverryan mentioned this pull request Mar 29, 2020

created email validator #572

Closed

jrushlow commented Mar 30, 2020

View reviewed changes

src/Maker/MakeResetPassword.php Show resolved Hide resolved