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

Fixes #53 fixes #43 #54

Merged
merged 1 commit into from
Nov 16, 2017
Merged

Fixes #53 fixes #43 #54

merged 1 commit into from
Nov 16, 2017

Conversation

falansari
Copy link
Contributor

Due to changes to the Facebook API, the user's Facebook name and
e-mail address return as undefined undefined, and undefined respectively.
This PR addresses these issues, hopefully successfully fixing them.

Changes made:

  1. In auth.js: under Facebook, 'profileFields' was added for requesting
    permissions from the FB API.
  2. In routes.js: 'public_profile' was added to the FB permissions scope.
    Although FB provides this by default to an app that was granted access by
    a user, it is considered best practice to declare all requested permissions.
  3. In user.js: under facebook, name was changed to be before email field.
    Their order is the reason behind issue Email Id is undefined in Facebook login #43, where email id returns as undefined.

Due to changes to the Facebook API, the user's Facebook name and
e-mail address return as undefined undefined, and undefined respectively.
This PR addresses these issues, hopefully successfully fixing them.

Changes made:
1. In auth.js: under Facebook, 'profileFields' was added for requesting
permissions from the FB API.
2. In routes.js: 'public_profile' was added to the FB permissions scope.
Although FB provides this by default to an app that was granted access by
a user, it is considered best practice to declare all requested permissions.
3. In user.js: under facebook, name was changed to be before email field.
Their order is the reason behind issue scotch-io#43, where email id returns as undefined.
@chris-sev chris-sev merged commit 6d322c7 into scotch-io:master Nov 16, 2017
@chris-sev
Copy link
Member

Right on! I'll update the article as well to show these changes. Really appreciate it.

@falansari falansari deleted the fix#53#43 branch November 16, 2017 19:28
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