[BD-13] Let ModuleSystem use UserService

cms/djangoapps/contentstore/views/preview.py

@@ -198,12 +198,10 @@ def _preview_module_system(request, descriptor, field_data):
         render_template=render_from_lms,
         debug=True,
         replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id),
-        user=request.user,
         can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)),
         get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)),
         mixins=settings.XBLOCK_MIXINS,
         course_id=course_id,
-        anonymous_student_id='student',
 
         # Set up functions to modify the fragment produced by student_view
         wrappers=wrappers,
@@ -216,7 +214,7 @@ def _preview_module_system(request, descriptor, field_data):
             "field-data": field_data,
             "i18n": ModuleI18nService,
             "settings": SettingsService(),
-            "user": DjangoXBlockUserService(request.user),
+            "user": DjangoXBlockUserService(request.user, anonymous_user_id='student'),
             "partitions": StudioPartitionService(course_id=course_id),
             "teams_configuration": TeamsConfigurationService(),
         },

common/djangoapps/xblock_django/constants.py

@@ -0,0 +1,11 @@
+"""
+Constants used by DjangoXBlockUserService
+"""
+
+# Optional attributes stored on the XBlockUser
+ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id'
+ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated'
+ATTR_KEY_USER_ID = 'edx-platform.user_id'
+ATTR_KEY_USERNAME = 'edx-platform.username'
+ATTR_KEY_USER_IS_STAFF = 'edx-platform.user_is_staff'
+ATTR_KEY_USER_PREFERENCES = 'edx-platform.user_preferences'

common/djangoapps/xblock_django/tests/test_user_service.py

@@ -2,6 +2,7 @@
 Tests for the DjangoXBlockUserService.
 """
 
+import ddt
 import pytest
 from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey
@@ -12,6 +13,7 @@
 from common.djangoapps.student.tests.factories import AnonymousUserFactory, UserFactory
 from common.djangoapps.xblock_django.user_service import (
     ATTR_KEY_IS_AUTHENTICATED,
+    ATTR_KEY_ANONYMOUS_USER_ID,
     ATTR_KEY_USER_ID,
     ATTR_KEY_USER_IS_STAFF,
     ATTR_KEY_USER_PREFERENCES,
@@ -21,6 +23,7 @@
 )
 
 
+@ddt.ddt
 class UserServiceTestCase(TestCase):
     """
     Tests for the DjangoXBlockUserService.
@@ -42,7 +45,7 @@ def assert_is_anon_xb_user(self, xb_user):
         assert xb_user.full_name is None
         self.assertListEqual(xb_user.emails, [])
 
-    def assert_xblock_user_matches_django(self, xb_user, dj_user):
+    def assert_xblock_user_matches_django(self, xb_user, dj_user, user_is_staff=False, anonymous_user_id=None):
         """
         A set of assertions for comparing a XBlockUser to a django User
         """
@@ -51,7 +54,8 @@ def assert_xblock_user_matches_django(self, xb_user, dj_user):
         assert xb_user.full_name == dj_user.profile.name
         assert xb_user.opt_attrs[ATTR_KEY_USERNAME] == dj_user.username
         assert xb_user.opt_attrs[ATTR_KEY_USER_ID] == dj_user.id
-        assert not xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF]
+        assert xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] == user_is_staff
+        assert xb_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] == anonymous_user_id
         assert all((pref in USER_PREFERENCES_WHITE_LIST) for pref in xb_user.opt_attrs[ATTR_KEY_USER_PREFERENCES])
 
     def test_convert_anon_user(self):
@@ -63,14 +67,25 @@ def test_convert_anon_user(self):
         assert xb_user.is_current_user
         self.assert_is_anon_xb_user(xb_user)
 
-    def test_convert_authenticate_user(self):
+    @ddt.data(
+        (False, None),
+        (True, None),
+        (False, 'abcdef0123'),
+        (True, 'abcdef0123'),
+    )
+    @ddt.unpack
+    def test_convert_authenticate_user(self, user_is_staff, anonymous_user_id):
         """
         Tests for convert_django_user_to_xblock_user behavior when django user is User.
         """
-        django_user_service = DjangoXBlockUserService(self.user)
+        django_user_service = DjangoXBlockUserService(
+            self.user,
+            user_is_staff=user_is_staff,
+            anonymous_user_id=anonymous_user_id,
+        )
         xb_user = django_user_service.get_current_user()
         assert xb_user.is_current_user
-        self.assert_xblock_user_matches_django(xb_user, self.user)
+        self.assert_xblock_user_matches_django(xb_user, self.user, user_is_staff, anonymous_user_id)
 
     def test_get_anonymous_user_id_returns_none_for_non_staff_users(self):
         """

common/djangoapps/xblock_django/user_service.py

@@ -11,11 +11,16 @@
 from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences
 from common.djangoapps.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'
-ATTR_KEY_USER_IS_STAFF = 'edx-platform.user_is_staff'
-ATTR_KEY_USER_PREFERENCES = 'edx-platform.user_preferences'
+from .constants import (
+    ATTR_KEY_ANONYMOUS_USER_ID,
+    ATTR_KEY_IS_AUTHENTICATED,
+    ATTR_KEY_USER_ID,
+    ATTR_KEY_USERNAME,
+    ATTR_KEY_USER_IS_STAFF,
+    ATTR_KEY_USER_PREFERENCES,
+)
+
+
 USER_PREFERENCES_WHITE_LIST = ['pref-lang', 'time_zone']
 
 
@@ -24,10 +29,18 @@ class DjangoXBlockUserService(UserService):
     A user service that converts Django users to XBlockUser
     """
     def __init__(self, django_user, **kwargs):
+        """
+        Constructs a DjangoXBlockUserService object.
+
+        Args:
+            user_is_staff(bool): optional - whether the user is staff in the course
+            anonymous_user_id(str): optional - anonymous_user_id for the user in the course
+        """
         super().__init__(**kwargs)
         self._django_user = django_user
         if self._django_user:
             self._django_user.user_is_staff = kwargs.get('user_is_staff', False)
+            self._django_user.anonymous_user_id = kwargs.get('anonymous_user_id', None)
 
     def get_current_user(self):
         """
@@ -82,6 +95,7 @@ def _convert_django_user_to_xblock_user(self, django_user):
                 full_name = None
             xblock_user.full_name = full_name
             xblock_user.emails = [django_user.email]
+            xblock_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] = django_user.anonymous_user_id
             xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = True
             xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id
             xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username

common/lib/xmodule/xmodule/capa_module.py

@@ -31,6 +31,11 @@
 from capa.inputtypes import Status
 from capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError
 from capa.util import convert_files_to_filenames, get_inner_html_from_xpath
+from common.djangoapps.xblock_django.constants import (
+    ATTR_KEY_ANONYMOUS_USER_ID,
+    ATTR_KEY_USER_IS_STAFF,
+    ATTR_KEY_USER_ID,
+)
 from openedx.core.djangolib.markup import HTML, Text
 from xmodule.contentstore.django import contentstore
 from xmodule.editing_module import EditingMixin
@@ -113,7 +118,7 @@ def from_json(self, value):
     to_json = from_json
 
 
-@XBlock.wants('user')
+@XBlock.needs('user')
 @XBlock.needs('i18n')
 @XBlock.wants('call_to_action')
 class ProblemBlock(
@@ -784,9 +789,10 @@ def choose_new_seed(self):
         """
         if self.rerandomize == RANDOMIZATION.NEVER:
             self.seed = 1
-        elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'):
+        elif self.rerandomize == RANDOMIZATION.PER_STUDENT:
+            user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0
             # see comment on randomization_bin
-            self.seed = randomization_bin(self.runtime.seed, str(self.location).encode('utf-8'))
+            self.seed = randomization_bin(user_id, str(self.location).encode('utf-8'))
         else:
             self.seed = struct.unpack('i', os.urandom(4))[0]
 
@@ -801,9 +807,13 @@ def new_lcp(self, state, text=None):
         if text is None:
             text = self.data
 
+        user_service = self.runtime.service(self, 'user')
+        anonymous_student_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
+        seed = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0
+
         capa_system = LoncapaSystem(
             ajax_url=self.ajax_url,
-            anonymous_student_id=self.runtime.anonymous_student_id,
+            anonymous_student_id=anonymous_student_id,
             cache=self.runtime.cache,
             can_execute_unsafe_code=self.runtime.can_execute_unsafe_code,
             get_python_lib_zip=self.runtime.get_python_lib_zip,
@@ -812,7 +822,7 @@ def new_lcp(self, state, text=None):
             i18n=self.runtime.service(self, "i18n"),
             node_path=self.runtime.node_path,
             render_template=self.runtime.render_template,
-            seed=self.runtime.seed,      # Why do we do this if we have self.seed?
+            seed=seed,  # Why do we do this if we have self.seed?
             STATIC_URL=self.runtime.STATIC_URL,
             xqueue=self.runtime.xqueue,
             matlab_api_key=self.matlab_api_key
@@ -1412,14 +1422,15 @@ def answer_available(self):
         """
         Is the user allowed to see an answer?
         """
+        user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
         if not self.correctness_available():
             # If correctness is being withheld, then don't show answers either.
             return False
         elif self.showanswer == '':
             return False
         elif self.showanswer == SHOWANSWER.NEVER:
             return False
-        elif self.runtime.user_is_staff:
+        elif user_is_staff:
             # This is after the 'never' check because admins can see the answer
             # unless the problem explicitly prevents it
             return True
@@ -1459,10 +1470,11 @@ def correctness_available(self):
 
         Limits access to the correct/incorrect flags, messages, and problem score.
         """
+        user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
         return ShowCorrectness.correctness_available(
             show_correctness=self.show_correctness,
             due_date=self.close_date,
-            has_staff_access=self.runtime.user_is_staff,
+            has_staff_access=user_is_staff,
         )
 
     def update_score(self, data):
@@ -1777,7 +1789,8 @@ def submit_problem(self, data, override_time=False):
             # If the user is a staff member, include
             # the full exception, including traceback,
             # in the response
-            if self.runtime.user_is_staff:
+            user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
+            if user_is_staff:
                 msg = f"Staff debug info: {traceback.format_exc()}"
 
             # Otherwise, display just an error message,

common/lib/xmodule/xmodule/html_module.py

@@ -17,6 +17,7 @@
 from web_fragments.fragment import Fragment
 from xblock.core import XBlock
 from xblock.fields import Boolean, List, Scope, String
+from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID
 from xmodule.contentstore.content import StaticContent
 from xmodule.editing_module import EditingMixin
 from xmodule.edxnotes_utils import edxnotes
@@ -42,6 +43,7 @@
 
 
 @XBlock.needs("i18n")
+@XBlock.needs("user")
 class HtmlBlockMixin(  # lint-amnesty, pylint: disable=abstract-method
     XmlMixin, EditingMixin,
     XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin,
@@ -117,8 +119,9 @@ def get_html(self):
         """ Returns html required for rendering the block. """
         if self.data:
             data = self.data
-            if getattr(self.runtime, 'anonymous_student_id', None):
-                data = data.replace("%%USER_ID%%", self.runtime.anonymous_student_id)
+            user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
+            if user_id:
+                data = data.replace("%%USER_ID%%", user_id)
             data = data.replace("%%COURSE_ID%%", str(self.scope_ids.usage_id.context_key))
             return data
         return self.data

common/lib/xmodule/xmodule/lti_module.py

@@ -78,6 +78,7 @@
 from openedx.core.djangolib.markup import HTML, Text
 from xmodule.editing_module import EditingMixin
 
+from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID
 from xmodule.lti_2_util import LTI20BlockMixin, LTIError
 from xmodule.raw_module import EmptyDataRawMixin
 from xmodule.util.xmodule_django import add_webpack_to_fragment
@@ -269,6 +270,7 @@ class LTIFields:
 
 
 @XBlock.needs("i18n")
+@XBlock.needs("user")
 class LTIBlock(
     LTIFields,
     LTI20BlockMixin,
@@ -529,7 +531,10 @@ def preview_handler(self, _, __):
         return Response(template, content_type='text/html')
 
     def get_user_id(self):
-        user_id = self.runtime.anonymous_student_id
+        """
+        Returns the current user ID, URL-escaped so it is safe to use as a URL component.
+        """
+        user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
         assert user_id is not None
         return str(parse.quote(user_id))
 
@@ -671,7 +676,8 @@ def oauth_params(self, custom_parameters, client_key, client_secret):
         # To test functionality test in LMS
 
         if callable(self.runtime.get_real_user):
-            real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_id)
+            user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
+            real_user_object = self.runtime.get_real_user(user_id)
             try:
                 self.user_email = real_user_object.email  # lint-amnesty, pylint: disable=attribute-defined-outside-init
             except AttributeError:

common/lib/xmodule/xmodule/seq_module.py

@@ -33,6 +33,7 @@
     XModuleToXBlockMixin,
 )
 
+from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF
 from openedx.core.djangoapps.agreements.toggles import is_integrity_signature_enabled
 
 from .exceptions import NotFoundError
@@ -378,7 +379,7 @@ def get_metadata(self, view=STUDENT_VIEW, context=None):
         is_hidden_after_due = False
 
         if self._required_prereq():
-            if self.runtime.user_is_staff:
+            if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
                 banner_text = _(
                     'This subsection is unlocked for learners when they meet the prerequisite requirements.'
                 )
@@ -459,7 +460,7 @@ def student_view(self, context):
         prereq_met = True
         prereq_meta_info = {}
         if self._required_prereq():
-            if self.runtime.user_is_staff:
+            if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
                 banner_text = _(
                     'This subsection is unlocked for learners when they meet the prerequisite requirements.'
                 )
@@ -553,7 +554,7 @@ def _can_user_view_content(self, course):
         """
         hidden_date = course.end if course.self_paced else self.due
         return (
-            self.runtime.user_is_staff or
+            self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) or
             self.verify_current_content_visibility(hidden_date, self.hide_after_due)
         )
 
@@ -643,8 +644,9 @@ def _is_gate_fulfilled(self):
         """
         gating_service = self.runtime.service(self, 'gating')
         if gating_service:
+            user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
             fulfilled = gating_service.is_gate_fulfilled(
-                self.course_id, self.location, self.runtime.user_id
+                self.course_id, self.location, user_id
             )
             return fulfilled
 
@@ -692,7 +694,8 @@ def descendants_are_gated(self, context):
             comes to determining whether a student is allowed to access this,
             with other checks being done in has_access calls.
         """
-        if self.runtime.user_is_staff or context.get('specific_masquerade', False):
+        user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
+        if user_is_staff or context.get('specific_masquerade', False):
             return False
 
         # We're not allowed to see it because of pre-reqs that haven't been
@@ -723,7 +726,8 @@ def _compute_is_prereq_met(self, recalc_on_unmet):
         """
         gating_service = self.runtime.service(self, 'gating')
         if gating_service:
-            return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet)
+            user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
+            return gating_service.compute_is_prereq_met(self.location, user_id, recalc_on_unmet)
 
         return True, {}
 
@@ -915,8 +919,10 @@ def _time_limited_student_view(self):
             self.is_time_limited
         )
         if feature_enabled:
-            user_id = self.runtime.user_id
-            user_role_in_course = 'staff' if self.runtime.user_is_staff else 'student'
+            current_user = self.runtime.service(self, 'user').get_current_user()
+            user_id = current_user.opt_attrs.get(ATTR_KEY_USER_ID)
+            user_is_staff = current_user.opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
+            user_role_in_course = 'staff' if user_is_staff else 'student'
             course_id = self.runtime.course_id
             content_id = self.location