feat: Add support for using LTI data to populate user profile#37307
Conversation
|
Thanks for the pull request, @xitij2000! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
43606bb to
f6013a1
Compare
b637199 to
8f193ec
Compare
|
Hi @xitij2000 thanks for creating this PR. I'll let @farhaanbukhsh do the PR review but I just wanted to run this scenario by you: Assuming that the LTI configuration used in open edX has the |
62439ee to
fe4da75
Compare
The new feature here the "Use LTI PII" flag which if enabled will make sure that if they are logged into canvas, and access Open edX via LTI, they get automatically logged in to their openedx account if it does exist, and if not, it will create a new user that uses the username, name, email etc from the LTI launch info to create a new account. |
Great, do you think this use case will fail on this line when they don't have an account/active session in edx because the request will not be authenticated? |
32e3e16 to
f9b6fd4
Compare
To activate this new functionality, you'd have to turn off "Require Account" and turn on |
farhaanbukhsh
left a comment
There was a problem hiding this comment.
I have added minor comments overall looks good
|
@farhaanbukhsh I've added the changes you requested to a fixup comment. |
|
@xitij2000 can you please fix the failing tests? |
farhaanbukhsh
left a comment
There was a problem hiding this comment.
@xitij2000 I can test this and it's working as expected lets fix the test cases, and then it is good to merge. 🚀 🐰
Currently the LTI provider implementation auto-creates a random user when logging in, however, the LTI launch can include relevant user details such as their email, full name and even a username. This change makes the LTI code use the provided details if the "Use lti pii" setting is set in the Django admin.
ff71b16 to
7ca8ea9
Compare
farhaanbukhsh
left a comment
There was a problem hiding this comment.
👍
- ✅ I tested this on tutor devstack master branch
- ✅ I read through the code
- ❌ I checked for accessibility issues
- ✔️ Includes documentation
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
Currently the LTI provider implementation auto-creates a random user when logging in, however, the LTI launch can include relevant user details such as their email, full name and even a username. This change makes the LTI code use the provided details if the "Use lti pii" setting is set int eh Django admin.
Supporting information
Testing instructions
Other information