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

[FIX] Added afterUserCreated trigger after first CAS login #9022

Merged
merged 1 commit into from
Dec 8, 2017
Merged

[FIX] Added afterUserCreated trigger after first CAS login #9022

merged 1 commit into from
Dec 8, 2017

Conversation

AmShaegar13
Copy link
Contributor

@RocketChat/core

The afterUserCreated callback is not called when creating a user on first CAS login. This causes inconsistent behaviour with the 'User created' integration when using CAS.

This PR adds the missing trigger after a user is created by CAS login.

@graywolf336 graywolf336 added this to the 0.60.0 milestone Dec 6, 2017
@AmShaegar13 AmShaegar13 force-pushed the fix_cas_login_trigger_integration branch from df53048 to 286fb61 Compare December 6, 2017 16:29
@rodrigok
Copy link
Member

rodrigok commented Dec 7, 2017

@AmShaegar13 Awesome, thanks. Can you do some changes?

The best place to put that code is here inside the Accounts.insertUserDoc, you should check if the user has username and then call that callback, then we will solve that problem for every place we call the Accounts.insertUserDoc passing username and not call the callback.

The reason for the if is that callback is called only by the method setUsername, meaning you have finished your registration.

What do you think?

@AmShaegar13
Copy link
Contributor Author

@rodrigok no problem. Although I did not do it in the first place to avoid the callback being triggered for every user joining a live chat. Wouldn't that be the case?

Is that, what you want?

@rodrigok
Copy link
Member

rodrigok commented Dec 7, 2017

@AmShaegar13 Good point, that is not desired at all, we are working to remove livechat from user's collection anyways.

You can check if the user has username and type !== 'visitor', that solves the problem and keep the code in a central place. We could remove that on the next release when we finish the livechat users separation.

@rodrigok
Copy link
Member

rodrigok commented Dec 7, 2017

@AmShaegar13 Can we talk at https://open.rocket.chat/direct/rodrigo.nascimento ?

@AmShaegar13 AmShaegar13 force-pushed the fix_cas_login_trigger_integration branch from 286fb61 to a80a704 Compare December 8, 2017 14:40
@AmShaegar13 AmShaegar13 force-pushed the fix_cas_login_trigger_integration branch from a80a704 to 597eb07 Compare December 8, 2017 14:48
@AmShaegar13
Copy link
Contributor Author

@rodrigok How about this?

@biomassives
Copy link

biomassives commented Dec 8, 2017 via email

@rodrigok rodrigok changed the title [FIX] added afterUserCreated trigger after first CAS login [FIX] Added afterUserCreated trigger after first CAS login Dec 8, 2017
@rodrigok rodrigok merged commit 1501286 into RocketChat:develop Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigok rodrigok rodrigok approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@AmShaegar13 @rodrigok @biomassives @graywolf336