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

[1.x] Add option to force the password to have a special character. #65

Merged
merged 8 commits into from
Sep 22, 2020
Merged

[1.x] Add option to force the password to have a special character. #65

merged 8 commits into from
Sep 22, 2020

Conversation

leo95batista
Copy link
Contributor

Hi! I have created this PR with the intention of adding in the password validation rules the option to require that the password contain or not special characters.

Regards.

@DarthShmev
Copy link
Contributor

Looks good to me.

case $this->requireUppercase
&& ! $this->requireNumeric
&& ! $this->requireSpecialCharacter:
return __('The :attribute must be at least '.$this->length.' characters and contain at least one uppercase character.');

Choose a reason for hiding this comment

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

It should probably changed to something like: return __('The :attribute must be at least :length characters and contain at least one uppercase character.', ['length' => $this->length]);

Copy link
Contributor Author

@leo95batista leo95batista Sep 21, 2020

Choose a reason for hiding this comment

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

@buismaarten Please check it now. Thanks

Choose a reason for hiding this comment

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

@leo95batista looks better!

@taylorotwell
Copy link
Member

This PR has merge conflicts. Also why switch(1) instead of switch(true)?

@leo95batista
Copy link
Contributor Author

leo95batista commented Sep 21, 2020

Hello, good morning everyone! , @taylorotwell I am reviewing, and I see that the PR #70 was approved first before this PR #65. For that reason the conflict. In PR #70, they changed the style in which the validation message was returned. When creating my PR, I can change the style, but I did not do it to maintain the style that existed initially.

Before

return __('The :attribute must be at least '.$this->length.' characters and contain at least one uppercase character.');

Now

return __('The :attribute must be at least :length characters and contain at least one uppercase character.', ['length' => $this->length]);

I think switch(1) and switch(true) mean the same in that context

@leo95batista
Copy link
Contributor Author

@taylorotwell , do you think anything else needs to be changed before doing the merge?

@taylorotwell taylorotwell merged commit d32755c into laravel:1.x Sep 22, 2020
@leo95batista leo95batista deleted the password-require-special-characters branch September 22, 2020 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants