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.

Sign up for GitHub

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

Jump to bottom

Minor fixes #285

Merged
merged 2 commits into from
Jun 11, 2017
Merged

Minor fixes #285

merged 2 commits into from
Jun 11, 2017

Conversation

nilshoerrmann
Copy link
Contributor

This pull request fixes two minor issues:

  1. A user can either be logged in with his username or with his email address. Both should be passed to the login failure delegate.
  2. When using multiple section, Members should always fallback to one of the configured sections.

@nilshoerrmann nilshoerrmann requested a review from michael-e June 9, 2017 16:00
michael-e
michael-e reviewed Jun 9, 2017
View reviewed changes
Copy link
Member

@michael-e michael-e left a comment

Choose a reason for hiding this comment

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

The delegate improvement looks good to me, but I am not sure about the fallback for the Members section. I would like to hear @brendo's thoughts on this.

@nilshoerrmann
Copy link
Contributor Author

Without this fallback, Members throws an exception as soon as no section ID is passed in the current request.

@michael-e
Copy link
Member

I understand, and I see no risk or side effects with your commit. Nevertheless: @brendo has built the code, so he may know better.

@michael-e michael-e self-requested a review June 9, 2017 16:20
@brendo brendo merged commit 7ae22ea into symphonycms:integration Jun 11, 2017
@brendo
Copy link
Member

brendo commented Jun 11, 2017

LGTM!

@michael-e
Copy link
Member

Great!

michael-e
michael-e reviewed Jun 11, 2017
View reviewed changes
Copy link
Member

@michael-e michael-e left a comment

Choose a reason for hiding this comment

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

All fine. :-)

@nilshoerrmann
Copy link
Contributor Author

Great!

@nitriques
Copy link
Member

I just saw that this PR has never been released. Now it is in 1.6.1 Thanks y'all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michael-e michael-e michael-e left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nilshoerrmann @michael-e @brendo @nitriques