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

Feature/allow ominiauth_success json override #914

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

thatandyrose
Copy link

Hi there!

While using devise_token_auth with Omniauth I hit onto an issue where the user json being returned didn't match the schema used on my frontend vuejs app.

I couldn't find an easy way to "hook" into the gem without forking and/or copying and pasting theomniauth_success contents, which I didn't want to do because of possible future breaking changes.

I know there is a hook in there to pass a block, but if I somehow did that in there (like providing a proxy object or something)... it just feels wrong since it assumes knowledge of the underlying gem implementation.

Anyhow, this provide a simple method that can be overriden which is just for providing some way to serialize the resource object.

Yay 👍 or nay 👎 ?

@thatandyrose thatandyrose changed the title Feature/allow omini json override Feature/allow ominiauth_success json override Jun 24, 2017
@thatandyrose
Copy link
Author

@lynndylanhurley any thoughts on this...? If it's not good we could close it, but would be great know why?

Thanks! 👍

@lynndylanhurley
Copy link
Owner

Hey @thatandyrose, sorry for the delay.

There's a method in the User concern called token_validation_response that we're using everywhere else for this:
https://github.com/lynndylanhurley/devise_token_auth/blob/master/app/models/devise_token_auth/concerns/user.rb#L231

For example, in my apps I've been doing something like this:

    # in app/models/user.rb

    def token_validation_response
      ActiveModelSerializers::SerializableResource.new(
        self,
        serializer: UserSerializer,
        key_transform: :camel_lower
      ).as_json
    end

And that way the user model is serialized the same for sign_up, registration, and token validation.

It looks like we should be using the same method for the omniauth response but we're not.

Would it be possible to just change @resource.as_json to @resource.token_validation_response on this line?

Thanks for taking the time to submit this : )

@zachfeldman
Copy link
Contributor

Hey @thatandyrose we'd love for you to finish this up if you have a moment! :) thanks again for your time.

@thatandyrose
Copy link
Author

hey @lynndylanhurley and @zachfeldman sorry for the delay guys!

Cool so shall I just make sure the ominauth controller is using that user concern method and then update the docs and we're good?

Thanks for getting back and again sorry for the delay!

cheers guys and gals!

@lynndylanhurley
Copy link
Owner

Cool so shall I just make sure the ominauth controller is using that user concern method and then update the docs and we're good?

That would be amazing. Thanks 👍

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.

3 participants