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

Individual tokens #22

Merged
merged 12 commits into from
Feb 28, 2018
Merged

Individual tokens #22

merged 12 commits into from
Feb 28, 2018

Conversation

liorrozen
Copy link
Contributor

These changes allow for the stream to use a separate access_token for each entity.
Previously, only one user access token was used but after this PR, each facebook entity is passed as an object and includes it's specific access token.

@liorrozen liorrozen self-assigned this Feb 14, 2018
@liorrozen liorrozen requested a review from hadasiii February 15, 2018 14:36
@@ -141,50 +138,52 @@ FacebookInsightStream.prototype._init = function ( callback ) {
}

FacebookInsightStream.prototype._initItem = function ( item ) {
var id = typeof item === 'string' ? item : item.id
Copy link
Contributor

Choose a reason for hiding this comment

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

null will give you an object - just make sure theres no possibility for nulls in the input or take care of this end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If somehow a page object arrives in the source as NULL there is not much we can do at this stage. I don't want to start filtering out nulls that should be page objects since as soon as we do that we can't consider the job as a success. The items themselves originate from Facebook so I don't see any way to recover from being passed nulls - it will simply fail as soon as we try to access the id attribute.

* @param {String} url
* @returns {Promise}
*/
FacebookInsightStream._apiCall = function (url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we use underscore for internal functions only no?..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used as an internal function, yes - it shouldn't be used outside of the module.

Copy link
Contributor

@hadasiii hadasiii 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 :)
was hard to review because of a lot of noise from the lint. i think maybe an option is to do 2 prs. one only for lint and one for logic, that way its easier to concentrate on the important stuff :)

@liorrozen liorrozen merged commit 7daf845 into panoplyio:master Feb 28, 2018
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.

2 participants