-
Notifications
You must be signed in to change notification settings - Fork 281
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
preSignUpHook isn't called for social sign up #669
Comments
The problem is that the social login code is called for signup and signin. Not sure how we can easily add the hook into social signup. |
This is a really critical issue to fix for purposes of accurate analytics tracking on user signups (among other things). Especially if you are trying to track multiple funnels into your app. If someone can point me in the right direction I would attempt to fix the issue. |
I agree. |
Great workaround - thanks elie! |
A better solution would be to edit (or as a cleaner hack use underscore's _.wrap()) whatever function the social buttons are calling when they need to create a user from the client side, and call the pre/post sign up hooks from there. I just can't seem to locate where the heck the social buttons call to create a user (on the client side) if they try to do an OAuth login and can't match it to an existing account. |
This might be of help: https://github.com/meteor-useraccounts/core/blob/master/lib/server_methods.js#L7 But you may need to have a look at the styling packages. E.g. useraccounts-bootstrap, to see where the client makes the calls to the server. I would be interesting to see how this works out. |
I had come across this before, but we need to find where it is called on the client side. I will look again now. |
Finally got around to tracking this down/fixing it today. Pull request to fix this issue here: meteor/meteor#7411 In the meantime, you can just copy/redefine the Accounts.updateOrCreateUserFromExternalService on the server side of your Meteor app to contain my updated version of the function as seen here. https://github.com/oohaysmlm/meteor/blob/7419a0d577d2cd09942fc6a96677da51d064f035/packages/accounts-base/accounts_server.js#L1326 This still doesn't solve the need to pass variables from the client to the server via an OAuth login - it just ensures the postSignUpHook will fire. |
@Bandit I'm not really seeing the answer the question I asked:
Can we open a issue against |
In the meantime i have a workaround by checking if the field "roles" is undefined after every login using Accounts.onLogin and calling a method that sets the field |
This seems to be a bug to me, unless it was intended for some reason.
This code:
https://github.com/meteor-useraccounts/core/blob/master/lib/templates_helpers/at_pwd_form.js#L203
Needs to be added somewhere in this file:
https://github.com/meteor-useraccounts/core/blob/master/lib/templates_helpers/at_social.js
The text was updated successfully, but these errors were encountered: