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

flesh out server-side setup instructions in README #155

Closed
wants to merge 3 commits into from
Closed

flesh out server-side setup instructions in README #155

wants to merge 3 commits into from

Conversation

michaelrkn
Copy link
Contributor

Fixes #154.

self.resource = warden.authenticate!(auth_options)
sign_in(resource_name, resource)
yield resource if block_given?
render json: resource, status: 201
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this return the full user object? Don't thank that's great as it reveals much more data than necessary - would prefer that:

data = {
  auth_token: resource.authentication_token,
  auth_email: resource.email
}
render json: data, status: 201

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'm using ActiveModel::Serializers, which is really the "right" way to handle what's exposed, but I think it's better to have an example that's safe by default.

@michaelrkn
Copy link
Contributor Author

i updated the example sessions controller based on your feedback, as well as adding a small refactor to remove some unnecessary code that's already inherited from devise.

def create
self.resource = warden.authenticate!(auth_options)
sign_in(resource_name, resource)
yield resource if block_given?
Copy link
Member

Choose a reason for hiding this comment

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

I think that's only necessary in Devise's implementation of the #create method - you'd never get a block here.

@michaelrkn
Copy link
Contributor Author

okay, removed that unnecessary line from the example sessions controller. let me know if you have any other feedback!

@marcoow
Copy link
Member

marcoow commented May 2, 2014

I'm on vacation currently - will look at this in 2 weeks.

@marcoow marcoow self-assigned this May 5, 2014
@marcoow
Copy link
Member

marcoow commented May 12, 2014

merged: 2737025

@marcoow marcoow closed this May 12, 2014
marcoow pushed a commit that referenced this pull request Feb 8, 2018
* Enable HBS transforms for Ember 1.13.0 and below again

* Fix HBS transforms for AST difference in Ember 1.11/1.12

* Extract Ember version checks into separate module

* Reenable most positional param to hash pair tests

* Remove unused `ember-compatibility-helpers` dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Devise instructions out of date
2 participants