diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 84058dfeae49..c486ec2ea447 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2516,6 +2516,9 @@ def _list_instructor_tasks(request, course_id, serialize_data=None): else: # Specifying for single problem's history tasks = task_api.get_instructor_task_history(course_id, module_state_key) + elif request.GET.get('include_canvas') is not None: + from ol_openedx_canvas_integration.task_helpers import get_filtered_instructor_tasks # pylint: disable=import-error + tasks = get_filtered_instructor_tasks(course_id, request.user) else: # If no problem or student, just get currently running tasks tasks = task_api.get_running_instructor_tasks(course_id) diff --git a/lms/djangoapps/instructor_task/views.py b/lms/djangoapps/instructor_task/views.py index 51e2812577fe..c1a31e089901 100644 --- a/lms/djangoapps/instructor_task/views.py +++ b/lms/djangoapps/instructor_task/views.py @@ -138,7 +138,9 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable= student = None problem_url = None + entrance_exam_url = None email_id = None + task_input = None try: task_input = json.loads(instructor_task.task_input) except ValueError: @@ -192,6 +194,10 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable= else: # num_succeeded < num_attempted # Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. # lint-amnesty, pylint: disable=line-too-long msg_format = _("Problem {action} for {succeeded} of {attempted} students") + elif task_input and task_input.get('course_key'): + from ol_openedx_canvas_integration.utils import get_task_output_formatted_message # pylint: disable=import-error + msg_format = get_task_output_formatted_message(task_output) + succeeded = True elif email_id is not None: # this reports on actions on bulk emails if num_attempted == 0: diff --git a/lms/djangoapps/lti_provider/migrations/0005_lticonsumer_use_lti_pii.py b/lms/djangoapps/lti_provider/migrations/0005_lticonsumer_use_lti_pii.py new file mode 100644 index 000000000000..9728dd8d60dd --- /dev/null +++ b/lms/djangoapps/lti_provider/migrations/0005_lticonsumer_use_lti_pii.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.23 on 2025-08-29 12:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('lti_provider', '0004_require_user_account'), + ] + + operations = [ + migrations.AddField( + model_name='lticonsumer', + name='use_lti_pii', + field=models.BooleanField(blank=True, default=False, help_text='When checked, the platform will automatically use any personal information provided via LTI to create an account. Otherwise an anonymous account will be created.'), + ), + ] diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py index 94582d4bf0b4..f763321748a9 100644 --- a/lms/djangoapps/lti_provider/models.py +++ b/lms/djangoapps/lti_provider/models.py @@ -40,6 +40,10 @@ class LtiConsumer(models.Model): "in this instance. This is required only for linking learner accounts with " "the LTI consumer. See the Open edX LTI Provider documentation for more details." )) + use_lti_pii = models.BooleanField(blank=True, default=False, help_text=_( + "When checked, the platform will automatically use any personal information provided " + "via LTI to create an account. Otherwise an anonymous account will be created." + )) @staticmethod def get_or_supplement(instance_guid, consumer_key): diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py index fa8274eef30e..e94caae5a688 100644 --- a/lms/djangoapps/lti_provider/tests/test_users.py +++ b/lms/djangoapps/lti_provider/tests/test_users.py @@ -2,10 +2,11 @@ Tests for the LTI user management functionality """ - +import itertools import string from unittest.mock import MagicMock, PropertyMock, patch +import ddt import pytest from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import PermissionDenied @@ -73,6 +74,7 @@ def test_random_username_generator(self): f"Username has forbidden character '{username[char]}'" +@ddt.ddt @patch('lms.djangoapps.lti_provider.users.switch_user', autospec=True) @patch('lms.djangoapps.lti_provider.users.create_lti_user', autospec=True) class AuthenticateLtiUserTest(TestCase): @@ -156,7 +158,10 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, crea create_user.assert_called_with(self.lti_user_id, self.lti_consumer) users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer) - create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, self.old_user.email) + create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, { + "email": self.old_user.email, + "full_name": "", + }) def test_auto_linking_of_users_using_lis_person_contact_email_primary_case_insensitive(self, create_user, switch_user): # pylint: disable=line-too-long request = RequestFactory().post("/", {"lis_person_contact_email_primary": self.old_user.email.upper()}) @@ -166,7 +171,10 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary_case_insen create_user.assert_called_with(self.lti_user_id, self.lti_consumer) users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer) - create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, request.user.email) + create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, { + "email": self.old_user.email, + "full_name": "", + }) def test_raise_exception_trying_to_auto_link_unauthenticate_user(self, create_user, switch_user): request = RequestFactory().post("/") @@ -190,7 +198,57 @@ def test_authenticate_unauthenticated_user_after_auto_linking_of_user_account(se assert not create_user.called switch_user.assert_called_with(self.request, lti_user, self.auto_linking_consumer) + @ddt.data( + *itertools.product( + ( + ( + { + "lis_person_contact_email_primary": "some_email@example.com", + "lis_person_name_given": "John", + "lis_person_name_family": "Doe", + }, + "some_email@example.com", + "John Doe", + ), + ( + { + "lis_person_contact_email_primary": "some_email@example.com", + "lis_person_name_full": "John Doe", + "lis_person_name_given": "Jacob", + }, + "some_email@example.com", + "John Doe", + ), + ( + {"lis_person_contact_email_primary": "some_email@example.com", "lis_person_name_full": "John Doe"}, + "some_email@example.com", + "John Doe", + ), + ({"lis_person_contact_email_primary": "some_email@example.com"}, "some_email@example.com", ""), + ({"lis_person_contact_email_primary": ""}, "", ""), + ({"lis_person_contact_email_primary": ""}, "", ""), + ({}, "", ""), + ), + [True, False], + ) + ) + @ddt.unpack + def test_create_user_when_user_account_not_required(self, params, enable_lti_pii, create_user, switch_user): + post_params, email, name = params + self.auto_linking_consumer.require_user_account = False + self.auto_linking_consumer.use_lti_pii = enable_lti_pii + self.auto_linking_consumer.save() + request = RequestFactory().post("/", post_params) + request.user = AnonymousUser() + users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer) + if enable_lti_pii: + profile = {"email": email, "full_name": name, "username": self.lti_user_id} + create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, profile) + else: + create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer) + +@ddt.ddt class CreateLtiUserTest(TestCase): """ Tests for the create_lti_user function in users.py @@ -222,22 +280,22 @@ def test_create_lti_user_creates_correct_user(self, uuid_mock, _username_mock): @patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'new_edx_id']) def test_unique_username_created(self, username_mock): User(username='edx_id').save() - users.create_lti_user('lti_user_id', self.lti_consumer) + users.create_lti_user('lti_user_id', self.lti_consumer, None) assert username_mock.call_count == 2 assert User.objects.count() == 3 user = User.objects.get(username='new_edx_id') assert user.email == 'new_edx_id@lti.example.com' def test_existing_user_is_linked(self): - lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email) + lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email}) assert lti_user.lti_consumer == self.lti_consumer assert lti_user.edx_user == self.existing_user def test_only_one_lti_user_edx_user_for_each_lti_consumer(self): - users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email) + users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email}) with pytest.raises(IntegrityError): - users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email) + users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email}) def test_create_multiple_lti_users_for_edx_user_if_lti_consumer_varies(self): lti_consumer_2 = LtiConsumer( @@ -247,11 +305,42 @@ def test_create_multiple_lti_users_for_edx_user_if_lti_consumer_varies(self): ) lti_consumer_2.save() - lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email) - lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, self.existing_user.email) + lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email}) + lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, {"email": self.existing_user.email}) assert lti_user_1.edx_user == lti_user_2.edx_user + def test_create_lti_user_with_full_profile(self): + lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, { + "email": "some.user@example.com", + "full_name": "John Doe", + "username": "john_doe", + }) + assert lti_user.edx_user.email == "some.user@example.com" + assert lti_user.edx_user.username == "john_doe" + assert lti_user.edx_user.profile.name == "John Doe" + + @patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id']) + def test_create_lti_user_with_missing_username_in_profile(self, mock): + lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, { + "email": "some.user@example.com", + "full_name": "John Doe", + }) + assert lti_user.edx_user.email == "some.user@example.com" + assert lti_user.edx_user.username == "edx_id" + assert lti_user.edx_user.profile.name == "John Doe" + + @patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'edx_id123']) + def test_create_lti_user_with_duplicate_username_in_profile(self, mock): + lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, { + "email": "some.user@example.com", + "full_name": "John Doe", + "username": self.existing_user.username, + }) + assert lti_user.edx_user.email == "some.user@example.com" + assert lti_user.edx_user.username == "edx_id" + assert lti_user.edx_user.profile.name == "John Doe" + class LtiBackendTest(TestCase): """ diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py index d1ade6ead325..168d6c4c6dc1 100644 --- a/lms/djangoapps/lti_provider/users.py +++ b/lms/djangoapps/lti_provider/users.py @@ -19,6 +19,20 @@ from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected +def get_lti_user_details(request): + """ + Returns key LTI user details from the LTI launch request. + """ + post_data = request.POST + email = post_data.get("lis_person_contact_email_primary", "").lower() + full_name = post_data.get("lis_person_name_full", "") + given_name = post_data.get("lis_person_name_given", "") + family_name = post_data.get("lis_person_name_family", "") + if not full_name and given_name: + full_name = f"{given_name} {family_name}" + return dict(email=email, full_name=full_name) + + def authenticate_lti_user(request, lti_user_id, lti_consumer): """ Determine whether the user specified by the LTI launch has an existing @@ -28,7 +42,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer): If the currently logged-in user does not match the user specified by the LTI launch, log out the old user and log in the LTI identity. """ - lis_email = request.POST.get("lis_person_contact_email_primary") + profile = get_lti_user_details(request) try: lti_user = LtiUser.objects.get( @@ -40,11 +54,14 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer): if lti_consumer.require_user_account: # Verify that the email from the LTI Launch and the logged-in user are the same # before linking the LtiUser with the edx_user. - if request.user.is_authenticated and request.user.email.lower() == lis_email.lower(): - lti_user = create_lti_user(lti_user_id, lti_consumer, request.user.email) + if request.user.is_authenticated and request.user.email.lower() == profile["email"]: + lti_user = create_lti_user(lti_user_id, lti_consumer, profile) else: # Ask the user to login before linking. raise PermissionDenied() from exc + elif lti_consumer.use_lti_pii: + profile["username"] = lti_user_id + lti_user = create_lti_user(lti_user_id, lti_consumer, profile) else: lti_user = create_lti_user(lti_user_id, lti_consumer) @@ -55,11 +72,15 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer): switch_user(request, lti_user, lti_consumer) -def create_lti_user(lti_user_id, lti_consumer, email=None): +def create_lti_user(lti_user_id, lti_consumer, profile=None): """ Generate a new user on the edX platform with a random username and password, and associates that account with the LTI identity. """ + if profile is None: + profile = {} + email = profile.get("email") + edx_user_id = profile.get("username") or generate_random_edx_username() edx_user = User.objects.filter(email=email).first() if email else None if not edx_user: @@ -67,8 +88,7 @@ def create_lti_user(lti_user_id, lti_consumer, email=None): edx_password = str(uuid.uuid4()) while not created: try: - edx_user_id = generate_random_edx_username() - edx_email = f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}" + edx_email = email if email else f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}" with transaction.atomic(): edx_user = User.objects.create_user( username=edx_user_id, @@ -78,13 +98,13 @@ def create_lti_user(lti_user_id, lti_consumer, email=None): # A profile is required if PREVENT_CONCURRENT_LOGINS flag is set. # TODO: We could populate user information from the LTI launch here, # but it's not necessary for our current uses. - edx_user_profile = UserProfile(user=edx_user) + edx_user_profile = UserProfile(user=edx_user, name=profile.get("full_name", "")) edx_user_profile.save() created = True except IntegrityError: + edx_user_id = generate_random_edx_username() # The random edx_user_id wasn't unique. Since 'created' is still # False, we will retry with a different random ID. - pass lti_user = LtiUser( lti_consumer=lti_consumer, diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py index 1106908112ed..b4da6547ea1f 100644 --- a/lms/djangoapps/lti_provider/views.py +++ b/lms/djangoapps/lti_provider/views.py @@ -32,7 +32,9 @@ OPTIONAL_PARAMETERS = [ 'context_title', 'context_label', 'lis_result_sourcedid', - 'lis_outcome_service_url', 'tool_consumer_instance_guid' + 'lis_outcome_service_url', 'tool_consumer_instance_guid', + "lis_person_name_full", "lis_person_name_given", "lis_person_name_family", + "lis_person_contact_email_primary", ] diff --git a/lms/static/js/instructor_dashboard/instructor_dashboard.js b/lms/static/js/instructor_dashboard/instructor_dashboard.js index f87e9db8e814..14f47fce6f36 100644 --- a/lms/static/js/instructor_dashboard/instructor_dashboard.js +++ b/lms/static/js/instructor_dashboard/instructor_dashboard.js @@ -176,6 +176,9 @@ such that the value can be defined later than this assignment (file load order). }, { constructor: window.InstructorDashboard.sections.ECommerce, $element: idashContent.find('.' + CSS_IDASH_SECTION + '#e-commerce') + }, { + constructor: window.InstructorDashboard.sections.CanvasIntegration, + $element: idashContent.find('.' + CSS_IDASH_SECTION + '#canvas_integration') }, { constructor: window.InstructorDashboard.sections.Membership, $element: idashContent.find('.' + CSS_IDASH_SECTION + '#membership') diff --git a/lms/static/js/instructor_dashboard/util.js b/lms/static/js/instructor_dashboard/util.js index 4e24c44c77ec..3818ba189c76 100644 --- a/lms/static/js/instructor_dashboard/util.js +++ b/lms/static/js/instructor_dashboard/util.js @@ -51,7 +51,8 @@ enableColumnReorder: false, autoHeight: true, rowHeight: 100, - forceFitColumns: true + forceFitColumns: true, + enableTextSelectionOnCells: true }; columns = [ { @@ -492,7 +493,8 @@ enableCellNavigation: true, enableColumnReorder: false, rowHeight: 30, - forceFitColumns: true + forceFitColumns: true, + enableTextSelectionOnCells: true }; columns = [ { diff --git a/lms/templates/instructor/instructor_dashboard_2/html-datatable.underscore b/lms/templates/instructor/instructor_dashboard_2/html-datatable.underscore new file mode 100644 index 000000000000..89da91b17881 --- /dev/null +++ b/lms/templates/instructor/instructor_dashboard_2/html-datatable.underscore @@ -0,0 +1,16 @@ +
+
| <%- h %> | + <%})%> +
|---|
| <%- value %> | + <%})%> +