-
Notifications
You must be signed in to change notification settings - Fork 16
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
Profiles creation #91
Conversation
a18c9d9
to
8446aab
Compare
|
||
def update_profile_from_edx(backend, user, response, is_new, *args, **kwargs): # pylint: disable=unused-argument | ||
""" | ||
Gets profile informations from EDX and saves them in the user profile |
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.
information not informations.
👍 from me after you address my comments. George is also reviewing too, it seems. |
access_token = response.get('access_token') | ||
if not access_token: | ||
# this should never happen for the edx oauth provider, but just in case... | ||
return |
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.
Can we log this case?
Functionality works great |
I think I addressed all the comments |
@@ -14,7 +14,7 @@ class EdxOrgOAuth2(BaseOAuth2): | |||
name = 'edxorg' | |||
ID_KEY = 'edx_id' | |||
REQUEST_TOKEN_URL = None | |||
EDXORG_BASE_URL = getattr(settings, 'EDXORG_BASE_URL', 'https://courses.edx.org/') | |||
EDXORG_BASE_URL = getattr(settings, 'EDXORG_BASE_URL') |
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.
Why not settings.EDXORG_BASE_URL
here? It will always exist
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.
you are right: just missed
Just one nit, feel free to squash and merge 👍 |
Profiles get created during user creation (via signal) and populated if it is the backend is EDX
4d1bd6e
to
a17bba4
Compare
What are the relevant tickets?
fixes #73
What's this PR do?
Creates a Profile for the user and populates it with the data existing in the connected EDX instance
Where should the reviewer start?
pipeline_api.py
How should this be manually tested?
Add some personal info for your user on edx and then login again on micromasters.
The user should be new on micromasters otherwise the profile will not be updated with the info from edx
Any background context you want to provide?
you need a working micromasters properly configured with a local instance of EDX
What GIF best describes this PR or how it makes you feel?