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

Add polling endpoint support #21

Merged
merged 7 commits into from
Feb 8, 2017
Merged

Add polling endpoint support #21

merged 7 commits into from
Feb 8, 2017
+963 −24

Conversation

dafortune
Copy link
Contributor

@dafortune dafortune commented Nov 25, 2016 •
edited
Loading

Adds polling provider and getState endpoints

@dafortune dafortune force-pushed the add-polling-endpoint-support branch from 72719a2 to 8517974 Compare November 30, 2016 01:10
@dafortune
Copy link
Contributor Author

Added the getState(callback) method to the transaction as suggested, so we can use it from auth0-server. getState name was chosen by me, and it seemed a good name, however I'm open to suggestions.

fiddur
fiddur approved these changes Dec 1, 2016
View reviewed changes
Copy link

@fiddur fiddur left a comment

Choose a reason for hiding this comment

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

Looks good. A couple of comments, but ship if you want :)

lib/utils/polling_client.js
/**
* @param {HttpClient} options.httpClient
*/
function pollingClient(urlBase, options) {
Copy link

Choose a reason for hiding this comment

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

This is quite a complex class for just some polling; it seems it is implementing some interface. Add some comment pointing out the interface, guessing it's socket.io interface?

lib/utils/polling_client.js
return function wrapper(payload) {
handler(object.toCamelKeys(payload));
};
}
Copy link

Choose a reason for hiding this comment

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

Wouldn't it be simpler (and also lighter for the cpu) to handle payload transformation in emitEvents, and not have to wrap and keep a store of wrapped handlers? ...or even use camel cased keys in emitEvents and just have to camelize error from remote?

Copy link
Contributor Author

@dafortune dafortune Dec 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Copy paste mistake good point. will remove this and handle that on emitEvents

@dafortune dafortune changed the title WIP: Add polling endpoint support Add polling endpoint support Dec 12, 2016
@dafortune dafortune merged commit 9c7aecf into auth0:master Feb 8, 2017
@dafortune dafortune deleted the add-polling-endpoint-support branch February 8, 2017 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fiddur fiddur fiddur approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@dafortune @fiddur