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

Improve example code for fetchUserProfile stub #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jamietanna
Copy link

✏️ Changes

Improve example code for fetchUserProfile stub

  • Explicitly initialise the profile object
  • Indent code snippets with 2 spaces, not 1
  • Add a TODO to make it more obvious it's not implemented
  • Correct casing in OAuth2 and add space after API

📷 Screenshots

If there were visual changes to the application with this change, please include before and after screenshots here. If it has animation, please use screen capture software like to make a gif.

🔗 References

N/A

🎯 Testing

  1. Browse to Custom Social Connections
  2. Click + New Connection
  3. Verify that Fetch User Profile Script is set to the updated value

🚀 Deployment

This change can be merged at any time.

🎡 Rollout

In order to verify that the deployment was successful we will need to verify that the example code is created as per steps in Testing.

🔥 Rollback

We will rollback if the changes introduce breaking JavaScript example code.

📄 Procedure

This will be done via a git revert on the merge commit that introduces this changes.

🖥 Appliance

Note to reviewers: ensure that this change is compatible with the Appliance.

@fyockm
Copy link
Contributor

fyockm commented Oct 15, 2018

@jamietanna thanks for you submission! I would love to take this one step further with an example. Maybe something like this (untested)?

        fetchUserProfile: [
                          'function(accessToken, ctx, cb) {',
                          '  // TODO: call OAuth2 API with the `accessToken` and set the user\'s `profile`',
                          '  var url = 'https://oauth2provider.com/oauth/authorize; // replace with actual url',
                          '  request.get(url, {',
                          '    headers: { Authorization: \'Bearer \' + accessToken }',
                          '    json: true',
                          '  }, function(err, resp, profile) {',
                          '    if (err) return cb(err);',
                          '    if (resp.statusCode !== 200) return cb(new Error('StatusCode: ' + resp.statusCode));',
                          '    profile.user_id = profile.id;',
                          '    cb(null, profile);',
                          '   });',
                          '}'
                          ].join('\n')

@jamietanna
Copy link
Author

Great idea @fyockm, I'll try and update with a better example this weekend!

@fyockm
Copy link
Contributor

fyockm commented Feb 11, 2019

@jamietanna are you still interested in updating this PR?

- Add a `profile` object
- Indent code snippets with 2 spaces, not 1
- Add a `TODO` to make it more obvious it's not implemented
- Correct casing in `OAuth2` and add space after `API`
- Add a more complete example
@jamietanna jamietanna force-pushed the defect/fetchUserProfile branch from 152e0e1 to 8a95be8 Compare September 13, 2021 14:44
@jamietanna
Copy link
Author

jamietanna commented Sep 13, 2021

Apologies @fyockm, I've amended now 👍

@fyockm
Copy link
Contributor

fyockm commented Sep 13, 2021

👋 hey @jamietanna! great to see you come back to this PR. thanks for updating- the change LGTM.

that said, ownership of this repo has changed here at Auth0 and have alerted the new team.

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