Skip to content

Add course flag to send external_user_id as user_id in LTI 1.1 XBlock launches - #307

Merged
michaelroytman merged 1 commit into
masterfrom
mroytman/MST-1717-external-id-LTI-1.1-launches
Nov 21, 2022
Merged

Add course flag to send external_user_id as user_id in LTI 1.1 XBlock launches#307
michaelroytman merged 1 commit into
masterfrom
mroytman/MST-1717-external-id-LTI-1.1-launches

Conversation

@michaelroytman

@michaelroytman michaelroytman commented Nov 18, 2022

Copy link
Copy Markdown
Contributor

Description:

JIRA: MST-1717

This commit introduces a new CourseWaffleFlag lti_consumer.enable_external_user_id_1p1_launches. When this flag is enabled for a course, LTI 1.1 XBlock launches in that course will send the user's external_user_id as the user_id attribute of the launch. external_user_id is the user's external user ID as defined, created, and stored by the external_user_ids Djangoapp in the edx-platform. When this waffle is not enabled for a course - the default case - LTI 1.1 XBlock launches in that course will continue to send the user's anonymous_user_id as the user_id attribute of the launch, as before.

This provides an opt-in opportunity for courses to send a consistent, static, and opaque user identifier in an LTI 1.1 XBlock launch. This may be necessary for integration with LTI tools that require such an identifier.

Please be aware that toggling this flag in a running course carries the risk of breaking the LTI integrations in the course. This flag should also only be enabled for new courses in which no LTI attempts have been made.

Verification:

This change was manually tested using the testing instructions in the README via an integration with the saLTIre testing tool. Please see the screenshots below. They show two courses to demonstrate that, after the change, the user_id is the same.

Before Change, Course 1:

before-course-1

Before Change, Course 2:

before-course-2

After Change, Course 1:

after-course-1

After Change, Course 2:

after-course-2

Testing:

In order to test these changes, follow these steps.

  1. Set up an LTI 1.1 integration in Studio by following the steps in the README: https://github.com/openedx/xblock-lti-consumer#lti-11.
  2. In the Django admin, add a CourseWaffleFlag in django-waffle > Flags.
    Name: lti_consumer.enable_external_user_id_1p1_launches
    Leave the remaining fields as the default.
  3. In the Django admin, add a CourseWaffleFlag override in Waffle_Utils › Waffle flag course overrides.
    Waffle flag: lti_consumer.enable_external_user_id_1p1_launches
    Course id: <course_id of the course you are testing in>
    Override choice: Force On
    Enabled:
  4. As a non-superuser, load the LTI component in the LMS and observe the user_id attribute in the Message Parameters section.

@michaelroytman
michaelroytman force-pushed the mroytman/MST-1717-external-id-LTI-1.1-launches branch from bffc465 to 0b6fbea Compare November 18, 2022 21:14
…XBlock launches

This commit introduces a new CourseWaffleFlag lti_consumer.enable_external_user_id_1p1_launches. When this flag is enabled for a course, LTI 1.1 XBlock launches in that course will send the user's external_user_id as the user_id attribute of the launch. external_user_id is the user's external user ID as defined, created, and stored by the external_user_ids Djangoapp in the edx-platform. When this waffle is not enabled for a course - the default case - LTI 1.1 XBlock launches in that course will continue to send the user's anonymous_user_id as the user_id attribute of the launch, as before.

This provides an opt-in opportunity for courses to send a consistent, static, and opaque user identifier in an LTI 1.1 XBlock launch. This may be necessary for integration with LTI tools that require such an identifier.

Please be aware that toggling this flag in a running course carries the risk of breaking the LTI integrations in the course. This flag should also only be enabled for new courses in which no LTI attempts have been made.
@michaelroytman
michaelroytman force-pushed the mroytman/MST-1717-external-id-LTI-1.1-launches branch from 0b6fbea to cff7744 Compare November 18, 2022 21:16
@codecov

codecov Bot commented Nov 18, 2022

Copy link
Copy Markdown

Codecov Report

Base: 97.79% // Head: 97.78% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (cff7744) compared to base (9c04004).
Patch coverage: 93.54% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
- Coverage   97.79%   97.78%   -0.01%     
==========================================
  Files          72       72              
  Lines        5843     5870      +27     
==========================================
+ Hits         5714     5740      +26     
- Misses        129      130       +1     
Flag Coverage Δ
unittests 97.78% <93.54%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lti_consumer/plugin/compat.py 74.07% <50.00%> (-4.19%) ⬇️
lti_consumer/lti_xblock.py 95.64% <100.00%> (+0.27%) ⬆️
lti_consumer/tests/unit/test_lti_xblock.py 99.12% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ashultz0 ashultz0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good!

@zacharis278 zacharis278 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@michaelroytman
michaelroytman merged commit bfd9c08 into master Nov 21, 2022
@michaelroytman
michaelroytman deleted the mroytman/MST-1717-external-id-LTI-1.1-launches branch November 21, 2022 14:53
michaelroytman added a commit that referenced this pull request Nov 29, 2022
… support external user IDs

In #307, we added the ability to send a stable, static user identifier (i.e. external user ID) to fix failed launches with the QwikLabs tool. This is because the QwikLabs tool did not work with the course-anonymized user IDs we used to send (i.e. anonymous user IDs). Inadvertently, this change broke the LTI 1.1 Basic Outcomes Service and the LTI 2.0 Result Service for courses that use the external user ID (i.e. they have the lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag enabled). The Basic Outcomes Service and Result Service handle grade pass backs. Because we now have two ways to identify a user in LTI 1.1/2.0, we must update the Basic Outcomes Service and Result Service to support both. This commit fixes this bug.
michaelroytman added a commit that referenced this pull request Nov 29, 2022
… support external user IDs

In #307, we added the ability to send a stable, static user identifier (i.e. external user ID) to fix failed launches with the QwikLabs tool. This is because the QwikLabs tool did not work with the course-anonymized user IDs we used to send (i.e. anonymous user IDs). Inadvertently, this change broke the LTI 1.1 Basic Outcomes Service and the LTI 2.0 Result Service for courses that use the external user ID (i.e. they have the lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag enabled). The Basic Outcomes Service and Result Service handle grade pass backs. Because we now have two ways to identify a user in LTI 1.1/2.0, we must update the Basic Outcomes Service and Result Service to support both. This commit fixes this bug.
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.

3 participants