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

[make:security:form-login] no interaction for make:security:form-login #1595

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eltharin
Copy link
Contributor

allow command make:security:form-login with name in argument to no interaction

as help says : make:security:form-login [<controllerName>]

@eltharin eltharin force-pushed the NonInteractive_formlogin branch 2 times, most recently from c7c8f6a to 9529eda Compare September 17, 2024 09:24
@eltharin eltharin changed the title no interaction for make:security:form-login [make:security:form-login] no interaction for make:security:form-login Sep 17, 2024
Copy link
Collaborator

@jrushlow jrushlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments and we can get this moving...

Comment on lines -60 to -64
private string $controllerName;
private string $firewallToUpdate;
private string $userClass;
private string $userNameField;
private bool $willLogout;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep both of these properties. MakerBundle is getting away from "abusing" setArgument and getArgument for the purposes of passing user input from one method to the next within a maker command. e.g. separation of concerns.

tl;dr interact() configures and sets any values onto their respective properties that will later be needed in generate()

interact() should get, validate, and set input.
generate() should only use values that are set via class properties when possible.

Comment on lines -127 to -131
$securityHelper = new InteractiveSecurityHelper();
$this->firewallToUpdate = $securityHelper->guessFirewallName($io, $securityData);
$this->userClass = $securityHelper->guessUserClass($io, $securityData['security']['providers']);
$this->userNameField = $securityHelper->guessUserNameField($io, $this->userClass, $securityData['security']['providers']);
$this->willLogout = $io->confirm('Do you want to generate a \'/logout\' URL?');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following on from the above, these all need to stay in interact. They should not be set in the generate method.

@jrushlow jrushlow added Feature New Feature Status: Needs Work Additional work is needed labels Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Status: Needs Work Additional work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants