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

Use email instead of username #103

Closed
wants to merge 30 commits into from
Closed

Conversation

winzou
Copy link
Contributor

@winzou winzou commented May 24, 2013

As stated in #88

@pjedrzejewski
Copy link
Member

Nice, for sure you also have to update the scenarios. :)

@stloyd
Copy link
Contributor

stloyd commented May 24, 2013

TBH. I would prefer to give a choice for developer which type of authentication he wants:

  • username,
  • email,
  • username or email

@winzou
Copy link
Contributor Author

winzou commented May 24, 2013

@stloyd could be interesting but how do you do that?

@jjanvier
Copy link
Contributor

simply by configuring this https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Resources/doc/logging_by_username_or_email.md :)

a key in parameters.yml could do the trick

@winzou
Copy link
Contributor Author

winzou commented May 27, 2013

OK but you still have to make different registration/login forms for each cases.
I don't think we can offer that out of the box, or even if it worths it. But this is really easy for developers to implement it:

  • With this PR they got an email authentication out of the box;
  • By simply deleting the fos_user config key of this PR they can switch back to username;
  • If they want both (username and email), they can change the FOSUB provider as @jjanvier pointed out.

@winzou
Copy link
Contributor Author

winzou commented May 27, 2013

@pjedrzejewski I've updated scenarios.

@pjedrzejewski
Copy link
Member

Requires rebase against latest master. :)

@winzou
Copy link
Contributor Author

winzou commented May 29, 2013

Waw, I don't really know what I've just done with the rebase. It seems OK but github shows all the merged commits again, I don't know if it's a good thing.

@pjedrzejewski can you still merge that or not?

@pjedrzejewski
Copy link
Member

@winzou Can you pick only the right commits and fix this branch or reopen this PR? If no, I'll try to fix it! :)

@winzou
Copy link
Contributor Author

winzou commented Jun 3, 2013

I should be able to do that tonight I'll let you know.

@winzou
Copy link
Contributor Author

winzou commented Jun 6, 2013

See #144 for clean PR.

@winzou winzou closed this Jun 6, 2013
pamil pushed a commit to pamil/Sylius that referenced this pull request Mar 21, 2016
CoderMaggie pushed a commit to CoderMaggie/Sylius that referenced this pull request Jun 1, 2016
Make tax 20% and included in price
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.

None yet

7 participants