-
Notifications
You must be signed in to change notification settings - Fork 1
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/login with google #8
base: develop
Are you sure you want to change the base?
Changes from 4 commits
8463450
a1e8214
24ba353
8040ac1
6d8a592
32b7c6e
ebc40d1
4869e76
24680fc
a5b8f99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,14 +21,16 @@ Package.onUse(function(api) { | |
'check', | ||
'accounts-base', | ||
'accounts-password', | ||
'accounts-google', | ||
'service-configuration', | ||
'space:[email protected]', | ||
'space:[email protected]', | ||
'space:[email protected]' | ||
]); | ||
|
||
// MODULES | ||
api.addFiles(['source/server/module.coffee'], 'server'); | ||
api.addFiles(['source/client/module.coffee'], 'client'); | ||
api.addFiles(['source/server/module.js'], 'server'); | ||
api.addFiles(['source/client/module.js'], 'client'); | ||
|
||
// SHARED | ||
api.addFiles([ | ||
|
@@ -37,7 +39,7 @@ Package.onUse(function(api) { | |
|
||
// CLIENT | ||
api.addFiles([ | ||
'source/client/events.coffee', | ||
'source/client/events.js', | ||
// Stores | ||
'source/client/stores/users-store.js', | ||
'source/client/stores/signups-store.js', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,54 +1,70 @@ | ||
Space.messaging.define Space.messaging.Event, 'Space.accountsUi', { | ||
Space.messaging.define(Space.messaging.Event, 'Space.accountsUi', { | ||
|
||
# SIGNUP | ||
// SIGNUP | ||
|
||
SignupRequested: { | ||
username: Match.OneOf(Username, null), | ||
email: Match.OneOf(EmailAddress, null), | ||
password: Match.OneOf(Password, null) | ||
} | ||
}, | ||
|
||
SignupInitiated: { | ||
signupId: Guid | ||
} | ||
}, | ||
|
||
SignupRetried: { | ||
signupId: Guid, | ||
} | ||
}, | ||
|
||
SignupFailed: { | ||
signupId: Guid, | ||
error: String | ||
} | ||
}, | ||
|
||
SignupCompleted: { | ||
signupId: Guid | ||
} | ||
}, | ||
|
||
# LOGIN | ||
// LOGIN | ||
|
||
LoginRequested: { | ||
user: String | ||
user: String, | ||
password: Password | ||
} | ||
}, | ||
|
||
LoginInitiated: { | ||
user: String | ||
} | ||
}, | ||
|
||
LoginFailed: { | ||
user: String | ||
user: String, | ||
error: Meteor.Error | ||
} | ||
}, | ||
|
||
LoginSucceeded: { | ||
user: String | ||
} | ||
}, | ||
|
||
// LOGIN WITH GOOGLE | ||
LoginWithGoogleRequested: { | ||
|
||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if there are not properties, just keep it compact: |
||
|
||
# LOGOUT | ||
LoginWithGoogleInitiated: { | ||
}, | ||
|
||
LogoutRequested: {} | ||
LoginWithGoogleFailed: { | ||
error: Match.OneOf(Meteor.Error, ServiceConfiguration.ConfigError) | ||
}, | ||
|
||
LoginWithGoogleSucceeded: { | ||
}, | ||
|
||
// LOGOUT | ||
|
||
LogoutRequested: {}, | ||
|
||
LoggedOut: {} | ||
|
||
} | ||
}); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
Space.accountsUi = Space.Module.define('Space.accountsUi', { | ||
|
||
requiredModules: [ | ||
'Space.flux' | ||
], | ||
|
||
stores: [ | ||
'Space.accountsUi.SignupsStore', | ||
'Space.accountsUi.LoginStore', | ||
'Space.accountsUi.UsersStore' | ||
], | ||
|
||
controllers: [ | ||
'Space.accountsUi.LoginController' | ||
], | ||
|
||
singletons: [ | ||
'Space.accountsUi.SignupsTracker' | ||
], | ||
|
||
onInitialize() { | ||
this.injector.map('SHA256').to(SHA256); | ||
this.injector.map('Meteor.user').to(Meteor.user); | ||
this.injector.map('Meteor.users').to(Meteor.users); | ||
this.injector.map('Space.accountsUi.Signups').asStaticValue(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation! |
||
} | ||
|
||
}); |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
Space.accountsUi = Space.Module.define('Space.accountsUi', { | ||
|
||
requiredModules: [ | ||
'Space.accounts' | ||
], | ||
|
||
singletons: [ | ||
'Space.accountsUi.SignupsPublication' | ||
], | ||
|
||
onInitialize() { | ||
this.injector.map('SHA256').to(SHA256); | ||
this.injector.map('Meteor.user').to(Meteor.user); | ||
this.injector.map('Meteor.users').to(Meteor.users); | ||
this.injector.map('Space.accountsUi.Signups').asStaticValue(); | ||
} | ||
|
||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather keep a single
LoginRequested
event and use something like aservice
property to distinguish between the various strategies. Same for the failed / succeeded events!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two concerns with that. Login and account creation are one if the user is logging in with external OAUTH service, and events have different properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But there should be one LoginWithServiceRequested event for all services or the solution with one event as you are suggesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok then only unify the failure / success events? My concern here is that subscribers need too much detail about the login process otherwise. Imagine you allow login with all different service providers. Then you would need 6 different event handlers although the basic flow (showing errors / login successful) is always the same 😉