From fbc18460ca2011c348a7107d2f36901aa0a7d543 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Wed, 8 Apr 2020 10:41:17 -0400 Subject: [PATCH] Add support for external user ids to the xblock user service. --- cms/envs/common.py | 1 + common/djangoapps/xblock_django/api.py | 2 +- .../xblock_django/tests/test_user_service.py | 19 ++++++++++- .../djangoapps/xblock_django/user_service.py | 12 ++++++- .../core/djangoapps/external_user_ids/apps.py | 2 +- .../migrations/0004_add_lti_type.py | 32 +++++++++++++++++++ .../djangoapps/external_user_ids/models.py | 8 +++-- .../djangoapps/external_user_ids/signals.py | 1 + .../tests/test_admin_generate_id.py | 16 +++------- .../external_user_ids/tests/test_signals.py | 8 +---- openedx/tests/settings.py | 2 ++ 11 files changed, 79 insertions(+), 24 deletions(-) create mode 100644 openedx/core/djangoapps/external_user_ids/migrations/0004_add_lti_type.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 531721413e..206182b89e 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1468,6 +1468,7 @@ INSTALLED_APPS = [ 'openedx.features.discounts', 'experiments', + 'openedx.core.djangoapps.external_user_ids', # so sample_task is available to celery workers 'openedx.core.djangoapps.heartbeat', diff --git a/common/djangoapps/xblock_django/api.py b/common/djangoapps/xblock_django/api.py index b9cb2f3a53..c591ce0361 100644 --- a/common/djangoapps/xblock_django/api.py +++ b/common/djangoapps/xblock_django/api.py @@ -3,8 +3,8 @@ API methods related to xblock state. """ -from xblock_django.models import XBlockConfiguration, XBlockStudioConfiguration from openedx.core.lib.cache_utils import CacheInvalidationManager +from xblock_django.models import XBlockConfiguration, XBlockStudioConfiguration cacher = CacheInvalidationManager(model=XBlockConfiguration) diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index e894fa5e28..2a41eb86f1 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -7,6 +7,7 @@ from django.test import TestCase from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.user_api.preferences.api import set_user_preference +from openedx.core.djangoapps.external_user_ids.models import ExternalIdType from student.models import anonymous_id_for_user from student.tests.factories import AnonymousUserFactory, UserFactory from xblock_django.user_service import ( @@ -82,7 +83,10 @@ class UserServiceTestCase(TestCase): """ django_user_service = DjangoXBlockUserService(self.user, user_is_staff=False) - anonymous_user_id = django_user_service.get_anonymous_user_id(username=self.user.username, course_id='edx/toy/2012_Fall') + anonymous_user_id = django_user_service.get_anonymous_user_id( + username=self.user.username, + course_id='edx/toy/2012_Fall' + ) self.assertIsNone(anonymous_user_id) def test_get_anonymous_user_id_returns_none_for_non_existing_users(self): @@ -112,3 +116,16 @@ class UserServiceTestCase(TestCase): ) self.assertEqual(anonymous_user_id, anon_user_id) + + def test_external_id(self): + """ + Tests that external ids differ based on type. + """ + ExternalIdType.objects.create(name='test1', description='Test type 1') + ExternalIdType.objects.create(name='test2', description='Test type 2') + django_user_service = DjangoXBlockUserService(self.user, user_is_staff=True) + ext_id1 = django_user_service.get_external_user_id('test1') + ext_id2 = django_user_service.get_external_user_id('test2') + assert ext_id1 != ext_id2 + with self.assertRaises(ValueError): + django_user_service.get_external_user_id('unknown') diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 2145463eb0..f09757677a 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -7,10 +7,10 @@ from django.contrib.auth.models import User from opaque_keys.edx.keys import CourseKey from xblock.reference.user_service import UserService, XBlockUser +from openedx.core.djangoapps.external_user_ids.models import ExternalId from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences from student.models import anonymous_id_for_user, get_user_by_username_or_email - ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' ATTR_KEY_USERNAME = 'edx-platform.username' @@ -35,6 +35,16 @@ class DjangoXBlockUserService(UserService): """ return self._convert_django_user_to_xblock_user(self._django_user) + def get_external_user_id(self, type_name): + """ + Returns an external user id of the given type. + Raises ValueError if the type doesn't exist. + """ + external_id, _ = ExternalId.add_new_user_id(self._django_user, type_name) + if not external_id: + raise ValueError("External ID type: %s does not exist" % type_name) + return str(external_id.external_user_id) + def get_anonymous_user_id(self, username, course_id): """ Get the anonymous user id for a user. diff --git a/openedx/core/djangoapps/external_user_ids/apps.py b/openedx/core/djangoapps/external_user_ids/apps.py index cc29082b06..abf154abc3 100644 --- a/openedx/core/djangoapps/external_user_ids/apps.py +++ b/openedx/core/djangoapps/external_user_ids/apps.py @@ -13,4 +13,4 @@ class ExternalUserIDConfig(AppConfig): name = 'openedx.core.djangoapps.external_user_ids' def ready(self): - from . import signals # pylint: disable=unused-variable + from . import signals # pylint: disable=unused-import diff --git a/openedx/core/djangoapps/external_user_ids/migrations/0004_add_lti_type.py b/openedx/core/djangoapps/external_user_ids/migrations/0004_add_lti_type.py new file mode 100644 index 0000000000..ab8fc16751 --- /dev/null +++ b/openedx/core/djangoapps/external_user_ids/migrations/0004_add_lti_type.py @@ -0,0 +1,32 @@ +# Generated by Django 2.2.12 on 2020-04-28 15:47 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('external_user_ids', '0003_auto_20200224_1836'), + ] + + lti_type = 'lti' + + def create_lti_type(apps, schema_editor): + """ + Add a LTI type + """ + ExternalIdType = apps.get_model('external_user_ids', 'ExternalIdType') + ExternalIdType.objects.update_or_create(name=Migration.lti_type, description='LTI Xblock launches') + + def delete_lti_type(apps, schema_editor): + """ + Delete the LTI type + """ + ExternalIdType = apps.get_model('external_user_ids', 'ExternalIdType') + ExternalIdType.objects.filter( + name=Migration.lti_type + ).delete() + + operations = [ + migrations.RunPython(create_lti_type, reverse_code=delete_lti_type), + ] diff --git a/openedx/core/djangoapps/external_user_ids/models.py b/openedx/core/djangoapps/external_user_ids/models.py index 708385394d..043834930b 100644 --- a/openedx/core/djangoapps/external_user_ids/models.py +++ b/openedx/core/djangoapps/external_user_ids/models.py @@ -3,7 +3,6 @@ Models for External User Ids that are sent out of Open edX """ import uuid as uuid_tools - from logging import getLogger from django.contrib.auth.models import User @@ -22,11 +21,15 @@ class ExternalIdType(TimeStampedModel): .. no_pii: """ MICROBACHELORS_COACHING = 'mb_coaching' + LTI = 'lti' name = models.CharField(max_length=32, blank=False, unique=True, db_index=True) description = models.TextField() history = HistoricalRecords() + class Meta: + app_label = 'external_user_ids' + def __str__(self): return self.name @@ -48,6 +51,7 @@ class ExternalId(TimeStampedModel): class Meta(object): unique_together = (('user', 'external_id_type'),) + app_label = 'external_user_ids' @classmethod def user_has_external_id(cls, user, type_name): @@ -86,7 +90,7 @@ class ExternalId(TimeStampedModel): type=type_name ) ) - return None + return None, False external_id, created = cls.objects.get_or_create( user=user, external_id_type=type_obj diff --git a/openedx/core/djangoapps/external_user_ids/signals.py b/openedx/core/djangoapps/external_user_ids/signals.py index 45f7f711dc..166303d12b 100644 --- a/openedx/core/djangoapps/external_user_ids/signals.py +++ b/openedx/core/djangoapps/external_user_ids/signals.py @@ -8,6 +8,7 @@ from django.db.models.signals import post_save from django.dispatch import receiver from openedx.core.djangoapps.catalog.utils import get_programs + from .models import ExternalId, ExternalIdType LOGGER = getLogger(__name__) diff --git a/openedx/core/djangoapps/external_user_ids/tests/test_admin_generate_id.py b/openedx/core/djangoapps/external_user_ids/tests/test_admin_generate_id.py index fd48a8292b..f0d8878e8d 100644 --- a/openedx/core/djangoapps/external_user_ids/tests/test_admin_generate_id.py +++ b/openedx/core/djangoapps/external_user_ids/tests/test_admin_generate_id.py @@ -2,23 +2,17 @@ # Test the logic behind the Generate External IDs tools in Admin # """ import mock -from django.conf import settings from django.contrib.admin.sites import AdminSite from django.test import TestCase from student.tests.factories import UserFactory -from openedx.core.djangolib.testing.utils import skip_unless_lms - -# external_ids is not in CMS' INSTALLED_APPS so these imports will error during test collection -if settings.ROOT_URLCONF == 'lms.urls': - from openedx.core.djangoapps.external_user_ids.models import ( - ExternalId, - ) - from openedx.core.djangoapps.external_user_ids.tests.factories import ExternalIDTypeFactory - from openedx.core.djangoapps.external_user_ids.admin import ExternalIdAdmin +from openedx.core.djangoapps.external_user_ids.models import ( + ExternalId, +) +from openedx.core.djangoapps.external_user_ids.tests.factories import ExternalIDTypeFactory +from openedx.core.djangoapps.external_user_ids.admin import ExternalIdAdmin -@skip_unless_lms class TestGenerateExternalIds(TestCase): """ Test generating ExternalIDs for Users. diff --git a/openedx/core/djangoapps/external_user_ids/tests/test_signals.py b/openedx/core/djangoapps/external_user_ids/tests/test_signals.py index 74c1e6a6cd..33ab1a9bc1 100644 --- a/openedx/core/djangoapps/external_user_ids/tests/test_signals.py +++ b/openedx/core/djangoapps/external_user_ids/tests/test_signals.py @@ -3,7 +3,6 @@ Signal Tests for External User Ids that are sent out of Open edX """ from opaque_keys.edx.keys import CourseKey -from django.conf import settings from django.core.cache import cache from edx_django_utils.cache import RequestCache @@ -18,19 +17,14 @@ from openedx.core.djangoapps.catalog.cache import ( COURSE_PROGRAMS_CACHE_KEY_TPL, PROGRAM_CACHE_KEY_TPL, ) +from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType from student.models import CourseEnrollment from course_modes.models import CourseMode -from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -# external_ids is not in CMS' INSTALLED_APPS so these imports will error during test collection -if settings.ROOT_URLCONF == 'lms.urls': - from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType - -@skip_unless_lms class MicrobachelorsExternalIDTest(ModuleStoreTestCase, CacheIsolationTestCase): """ Test cases for Signals for External User Ids diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 7eb499b348..950150edcc 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -87,6 +87,8 @@ INSTALLED_APPS = ( 'openedx.core.djangoapps.self_paced', 'openedx.core.djangoapps.schedules.apps.SchedulesConfig', 'openedx.core.djangoapps.theming.apps.ThemingConfig', + 'openedx.core.djangoapps.external_user_ids', + 'experiments', 'openedx.features.content_type_gating', 'openedx.features.course_duration_limits',