Skip to content

Commit bc7ccf1

Browse files
weaverryanweaverryan
committed
minor #799 [DX] [ResetPassword] lets stop abusing input arguments (jrushlow)
This PR was merged into the 1.0-dev branch. Discussion ---------- [DX] [ResetPassword] lets stop abusing input arguments We're currently abusing input arguments by using them as data stores internally. This PR is the first of many to refactor makers to replace input argument with proper class properties, add method return types where possible, and general housekeeping to improve code readability. The first maker to use this approach was `make:docker:database` found [here](https://github.com/symfony/maker-bundle/blob/main/src/Maker/MakeDockerDatabase.php). One decision we should make now before proceeding with refactoring additional makers is in regards to the docBlock format we should use for properties. Symfony uses the expanded format - but this can lead to reduced readability with a class that has multiple properties. As a solution to this we could break tradition and use single line docBlocks strictly for internal maker properties. ```php // Traditional expanded properties /** * @var string type of database selected by the user */ private $databaseChoice; /** * @var string Service identifier to be set in docker-compose.yaml */ private $serviceName = 'database'; /** * @var string Version set in docker-compose.yaml for the service. e.g. latest */ private $serviceVersion = 'latest'; ``` ```php // Single line /** @var string The "FROM" email address to be used in email template. */ private $fromEmailAddress; /** @var string The "FROM" name used in the email template. */ private $fromEmailName; /** @var string Redirect route to be used after a successful password reset. Route does not have to exist. */ private $controllerResetSuccessRedirect; ``` Another consideration to be made is `types` and `descriptions`. When the minimum supported PHP version for Symfony reaches `7.4`, we will be able to use typed properties. Until then I think we could omit types declared in property docBlocks unless we actually need a docBlock to begin with. Which leads me to, should we include descriptions for all properties as shown above (not included in the actual PR) or only when describing the property can not be done clearly with the property name itself. *Disclaimer - All of the above is scoped strictly to Makers and input argument replacements. It should not be taken out of context for the public API or generated code. Commits ------- 322ccfe lets stop abusing input arguments
2 parents ffb77ed + 322ccfe commit bc7ccf1

File tree

1 file changed

+39
-57
lines changed

1 file changed

+39
-57
lines changed

‎src/Maker/MakeResetPassword.php

Lines changed: 39 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator;
3030
use Symfony\Bundle\MakerBundle\Validator;
3131
use Symfony\Component\Console\Command\Command;
32-
use Symfony\Component\Console\Input\InputArgument;
3332
use Symfony\Component\Console\Input\InputInterface;
3433
use Symfony\Component\Mailer\MailerInterface;
3534
use Symfony\Component\Yaml\Yaml;
@@ -56,6 +55,14 @@ class MakeResetPassword extends AbstractMaker
5655
private $doctrineHelper;
5756
private $entityClassGenerator;
5857

58+
private $fromEmailAddress;
59+
private $fromEmailName;
60+
private $controllerResetSuccessRedirect;
61+
private $userClass;
62+
private $emailPropertyName;
63+
private $emailGetterMethodName;
64+
private $passwordSetterMethodName;
65+
5966
public function __construct(FileManager $fileManager, DoctrineHelper $doctrineHelper, EntityClassGenerator $entityClassGenerator)
6067
{
6168
$this->fileManager = $fileManager;
@@ -73,14 +80,14 @@ public static function getCommandDescription(): string
7380
return 'Create controller, entity, and repositories for use with symfonycasts/reset-password-bundle';
7481
}
7582

76-
public function configureCommand(Command $command, InputConfiguration $inputConfig)
83+
public function configureCommand(Command $command, InputConfiguration $inputConfig): void
7784
{
7885
$command
7986
->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeResetPassword.txt'))
8087
;
8188
}
8289

83-
public function configureDependencies(DependencyBuilder $dependencies)
90+
public function configureDependencies(DependencyBuilder $dependencies): void
8491
{
8592
$dependencies->addClassDependency(SymfonyCastsResetPasswordBundle::class, 'symfonycasts/reset-password-bundle');
8693
$dependencies->addClassDependency(MailerInterface::class, 'symfony/mailer');
@@ -98,21 +105,10 @@ public function configureDependencies(DependencyBuilder $dependencies)
98105
}
99106
}
100107

101-
public function interact(InputInterface $input, ConsoleStyle $io, Command $command)
108+
public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void
102109
{
103110
$io->title('Let\'s make a password reset feature!');
104111

105-
// initialize arguments & commands that are internal (i.e. meant only to be asked)
106-
$command
107-
->addArgument('from-email-address', InputArgument::REQUIRED)
108-
->addArgument('from-email-name', InputArgument::REQUIRED)
109-
->addArgument('controller-reset-success-redirect', InputArgument::REQUIRED)
110-
->addArgument('user-class')
111-
->addArgument('email-property-name')
112-
->addArgument('email-getter')
113-
->addArgument('password-setter')
114-
;
115-
116112
$interactiveSecurityHelper = new InteractiveSecurityHelper();
117113

118114
if (!$this->fileManager->fileExists($path = 'config/packages/security.yaml')) {
@@ -123,62 +119,48 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
123119
$securityData = $manipulator->getData();
124120
$providersData = $securityData['security']['providers'] ?? [];
125121

126-
$input->setArgument(
127-
'user-class',
128-
$userClass = $interactiveSecurityHelper->guessUserClass(
129-
$io,
130-
$providersData,
131-
'What is the User entity that should be used with the "forgotten password" feature? (e.g. <fg=yellow>App\\Entity\\User</>)'
132-
)
122+
$this->userClass = $interactiveSecurityHelper->guessUserClass(
123+
$io,
124+
$providersData,
125+
'What is the User entity that should be used with the "forgotten password" feature? (e.g. <fg=yellow>App\\Entity\\User</>)'
133126
);
134127

135-
$input->setArgument(
136-
'email-property-name',
137-
$interactiveSecurityHelper->guessEmailField($io, $userClass)
138-
);
139-
$input->setArgument(
140-
'email-getter',
141-
$interactiveSecurityHelper->guessEmailGetter($io, $userClass, $input->getArgument('email-property-name'))
142-
);
143-
$input->setArgument(
144-
'password-setter',
145-
$interactiveSecurityHelper->guessPasswordSetter($io, $userClass)
146-
);
128+
$this->emailPropertyName = $interactiveSecurityHelper->guessEmailField($io, $this->userClass);
129+
$this->emailGetterMethodName = $interactiveSecurityHelper->guessEmailGetter($io, $this->userClass, $this->emailPropertyName);
130+
$this->passwordSetterMethodName = $interactiveSecurityHelper->guessPasswordSetter($io, $this->userClass);
147131

148-
$io->text(sprintf('Implementing reset password for <info>%s</info>', $userClass));
132+
$io->text(sprintf('Implementing reset password for <info>%s</info>', $this->userClass));
149133

150134
$io->section('- ResetPasswordController -');
151135
$io->text('A named route is used for redirecting after a successful reset. Even a route that does not exist yet can be used here.');
152-
$input->setArgument('controller-reset-success-redirect', $io->ask(
136+
137+
$this->controllerResetSuccessRedirect = $io->ask(
153138
'What route should users be redirected to after their password has been successfully reset?',
154139
'app_home',
155140
[Validator::class, 'notBlank']
156-
)
157141
);
158142

159143
$io->section('- Email -');
160144
$emailText[] = 'These are used to generate the email code. Don\'t worry, you can change them in the code later!';
161145
$io->text($emailText);
162146

163-
$input->setArgument('from-email-address', $io->ask(
147+
$this->fromEmailAddress = $io->ask(
164148
'What email address will be used to send reset confirmations? e.g. mailer@your-domain.com',
165149
null,
166150
[Validator::class, 'validateEmailAddress']
167-
));
151+
);
168152

169-
$input->setArgument('from-email-name', $io->ask(
153+
$this->fromEmailName = $io->ask(
170154
'What "name" should be associated with that email address? e.g. "Acme Mail Bot"',
171155
null,
172156
[Validator::class, 'notBlank']
173-
)
174157
);
175158
}
176159

177-
public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator)
160+
public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void
178161
{
179-
$userClass = $input->getArgument('user-class');
180162
$userClassNameDetails = $generator->createClassNameDetails(
181-
'\\'.$userClass,
163+
'\\'.$this->userClass,
182164
'Entity\\'
183165
);
184166

@@ -217,24 +199,24 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
217199
'request_form_type_class_name' => $requestFormTypeClassNameDetails->getShortName(),
218200
'reset_form_type_full_class_name' => $changePasswordFormTypeClassNameDetails->getFullName(),
219201
'reset_form_type_class_name' => $changePasswordFormTypeClassNameDetails->getShortName(),
220-
'password_setter' => $input->getArgument('password-setter'),
221-
'success_redirect_route' => $input->getArgument('controller-reset-success-redirect'),
222-
'from_email' => $input->getArgument('from-email-address'),
223-
'from_email_name' => $input->getArgument('from-email-name'),
224-
'email_getter' => $input->getArgument('email-getter'),
225-
'email_field' => $input->getArgument('email-property-name'),
202+
'password_setter' => $this->passwordSetterMethodName,
203+
'success_redirect_route' => $this->controllerResetSuccessRedirect,
204+
'from_email' => $this->fromEmailAddress,
205+
'from_email_name' => $this->fromEmailName,
206+
'email_getter' => $this->emailGetterMethodName,
207+
'email_field' => $this->emailPropertyName,
226208
]
227209
);
228210

229-
$this->generateRequestEntity($generator, $requestClassNameDetails, $repositoryClassNameDetails, $userClass);
211+
$this->generateRequestEntity($generator, $requestClassNameDetails, $repositoryClassNameDetails);
230212

231213
$this->setBundleConfig($io, $generator, $repositoryClassNameDetails->getFullName());
232214

233215
$generator->generateClass(
234216
$requestFormTypeClassNameDetails->getFullName(),
235217
'resetPassword/ResetPasswordRequestFormType.tpl.php',
236218
[
237-
'email_field' => $input->getArgument('email-property-name'),
219+
'email_field' => $this->emailPropertyName,
238220
]
239221
);
240222

@@ -257,7 +239,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
257239
'reset_password/request.html.twig',
258240
'resetPassword/twig_request.tpl.php',
259241
[
260-
'email_field' => $input->getArgument('email-property-name'),
242+
'email_field' => $this->emailPropertyName,
261243
]
262244
);
263245

@@ -272,7 +254,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
272254
$this->successMessage($input, $io, $requestClassNameDetails->getFullName());
273255
}
274256

275-
private function setBundleConfig(ConsoleStyle $io, Generator $generator, string $repositoryClassFullName)
257+
private function setBundleConfig(ConsoleStyle $io, Generator $generator, string $repositoryClassFullName): void
276258
{
277259
$configFileExists = $this->fileManager->fileExists($path = 'config/packages/reset_password.yaml');
278260

@@ -322,7 +304,7 @@ private function setBundleConfig(ConsoleStyle $io, Generator $generator, string
322304
$generator->dumpFile($path, $manipulator->getContents());
323305
}
324306

325-
private function successMessage(InputInterface $input, ConsoleStyle $io, string $requestClassName)
307+
private function successMessage(InputInterface $input, ConsoleStyle $io, string $requestClassName): void
326308
{
327309
$closing[] = 'Next:';
328310
$closing[] = sprintf(' 1) Run <fg=yellow>"php bin/console make:migration"</> to generate a migration for the new <fg=yellow>"%s"</> entity.', $requestClassName);
@@ -337,7 +319,7 @@ private function successMessage(InputInterface $input, ConsoleStyle $io, string
337319
$io->newLine();
338320
}
339321

340-
private function generateRequestEntity(Generator $generator, ClassNameDetails $requestClassNameDetails, ClassNameDetails $repositoryClassNameDetails, string $userClass): void
322+
private function generateRequestEntity(Generator $generator, ClassNameDetails $requestClassNameDetails, ClassNameDetails $repositoryClassNameDetails): void
341323
{
342324
$requestEntityPath = $this->entityClassGenerator->generateEntityClass($requestClassNameDetails, false, false, false);
343325

@@ -365,7 +347,7 @@ private function generateRequestEntity(Generator $generator, ClassNameDetails $r
365347

366348
$manipulator->addManyToOneRelation((new RelationManyToOne())
367349
->setPropertyName('user')
368-
->setTargetClassName($userClass)
350+
->setTargetClassName($this->userClass)
369351
->setMapInverseRelation(false)
370352
->setCustomReturnType('object', false)
371353
->avoidSetter()

0 commit comments

Comments
 (0)