Revert "[BD-13] Let ModuleSystem use UserService"

cms/djangoapps/contentstore/views/preview.py

@@ -198,10 +198,12 @@ 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,
@@ -214,7 +216,7 @@ def _preview_module_system(request, descriptor, field_data):
             "field-data": field_data,
             "i18n": ModuleI18nService,
             "settings": SettingsService(),
-            "user": DjangoXBlockUserService(request.user, anonymous_user_id='student'),
+            "user": DjangoXBlockUserService(request.user),
             "partitions": StudioPartitionService(course_id=course_id),
             "teams_configuration": TeamsConfigurationService(),
         },

common/djangoapps/xblock_django/tests/test_user_service.py

@@ -2,7 +2,6 @@
 Tests for the DjangoXBlockUserService.
 """
 
-import ddt
 import pytest
 from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey
@@ -13,7 +12,6 @@
 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,
@@ -23,7 +21,6 @@
 )
 
 
-@ddt.ddt
 class UserServiceTestCase(TestCase):
     """
     Tests for the DjangoXBlockUserService.
@@ -45,7 +42,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, user_is_staff=False, anonymous_user_id=None):
+    def assert_xblock_user_matches_django(self, xb_user, dj_user):
         """
         A set of assertions for comparing a XBlockUser to a django User
         """
@@ -54,8 +51,7 @@ def assert_xblock_user_matches_django(self, xb_user, dj_user, user_is_staff=Fals
         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 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 not xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF]
         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):
@@ -67,25 +63,14 @@ def test_convert_anon_user(self):
         assert xb_user.is_current_user
         self.assert_is_anon_xb_user(xb_user)
 
-    @ddt.data(
-        (False, None),
-        (True, None),
-        (False, 'abcdef0123'),
-        (True, 'abcdef0123'),
-    )
-    @ddt.unpack
-    def test_convert_authenticate_user(self, user_is_staff, anonymous_user_id):
+    def test_convert_authenticate_user(self):
         """
         Tests for convert_django_user_to_xblock_user behavior when django user is User.
         """
-        django_user_service = DjangoXBlockUserService(
-            self.user,
-            user_is_staff=user_is_staff,
-            anonymous_user_id=anonymous_user_id,
-        )
+        django_user_service = DjangoXBlockUserService(self.user)
         xb_user = django_user_service.get_current_user()
         assert xb_user.is_current_user
-        self.assert_xblock_user_matches_django(xb_user, self.user, user_is_staff, anonymous_user_id)
+        self.assert_xblock_user_matches_django(xb_user, self.user)
 
     def test_get_anonymous_user_id_returns_none_for_non_staff_users(self):
         """

common/djangoapps/xblock_django/user_service.py

@@ -11,16 +11,11 @@
 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
 
-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,
-)
-
-
+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'
 USER_PREFERENCES_WHITE_LIST = ['pref-lang', 'time_zone']
 
 
@@ -29,18 +24,10 @@ 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):
         """
@@ -95,7 +82,6 @@ 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,11 +31,6 @@
 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
@@ -118,7 +113,7 @@ def from_json(self, value):
     to_json = from_json
 
 
-@XBlock.needs('user')
+@XBlock.wants('user')
 @XBlock.needs('i18n')
 @XBlock.wants('call_to_action')
 class ProblemBlock(
@@ -789,10 +784,9 @@ def choose_new_seed(self):
         """
         if self.rerandomize == RANDOMIZATION.NEVER:
             self.seed = 1
-        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
+        elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'):
             # see comment on randomization_bin
-            self.seed = randomization_bin(user_id, str(self.location).encode('utf-8'))
+            self.seed = randomization_bin(self.runtime.seed, str(self.location).encode('utf-8'))
         else:
             self.seed = struct.unpack('i', os.urandom(4))[0]
 
@@ -807,13 +801,9 @@ 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=anonymous_student_id,
+            anonymous_student_id=self.runtime.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,
@@ -822,7 +812,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=seed,  # Why do we do this if we have self.seed?
+            seed=self.runtime.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
@@ -1422,15 +1412,14 @@ 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 user_is_staff:
+        elif self.runtime.user_is_staff:
             # This is after the 'never' check because admins can see the answer
             # unless the problem explicitly prevents it
             return True
@@ -1470,11 +1459,10 @@ 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=user_is_staff,
+            has_staff_access=self.runtime.user_is_staff,
         )
 
     def update_score(self, data):
@@ -1789,8 +1777,7 @@ def submit_problem(self, data, override_time=False):
             # If the user is a staff member, include
             # the full exception, including traceback,
             # in the response
-            user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
-            if user_is_staff:
+            if self.runtime.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,7 +17,6 @@
 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
@@ -43,7 +42,6 @@
 
 
 @XBlock.needs("i18n")
-@XBlock.needs("user")
 class HtmlBlockMixin(  # lint-amnesty, pylint: disable=abstract-method
     XmlMixin, EditingMixin,
     XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin,
@@ -119,9 +117,8 @@ def get_html(self):
         """ Returns html required for rendering the block. """
         if self.data:
             data = self.data
-            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)
+            if getattr(self.runtime, 'anonymous_student_id', None):
+                data = data.replace("%%USER_ID%%", self.runtime.anonymous_student_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,7 +78,6 @@
 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
@@ -270,7 +269,6 @@ class LTIFields:
 
 
 @XBlock.needs("i18n")
-@XBlock.needs("user")
 class LTIBlock(
     LTIFields,
     LTI20BlockMixin,
@@ -531,10 +529,7 @@ def preview_handler(self, _, __):
         return Response(template, content_type='text/html')
 
     def get_user_id(self):
-        """
-        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)
+        user_id = self.runtime.anonymous_student_id
         assert user_id is not None
         return str(parse.quote(user_id))
 
@@ -676,8 +671,7 @@ def oauth_params(self, custom_parameters, client_key, client_secret):
         # To test functionality test in LMS
 
         if callable(self.runtime.get_real_user):
-            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)
+            real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_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,7 +33,6 @@
     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
@@ -379,7 +378,7 @@ def get_metadata(self, view=STUDENT_VIEW, context=None):
         is_hidden_after_due = False
 
         if self._required_prereq():
-            if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
+            if self.runtime.user_is_staff:
                 banner_text = _(
                     'This subsection is unlocked for learners when they meet the prerequisite requirements.'
                 )
@@ -460,7 +459,7 @@ def student_view(self, context):
         prereq_met = True
         prereq_meta_info = {}
         if self._required_prereq():
-            if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
+            if self.runtime.user_is_staff:
                 banner_text = _(
                     'This subsection is unlocked for learners when they meet the prerequisite requirements.'
                 )
@@ -554,7 +553,7 @@ def _can_user_view_content(self, course):
         """
         hidden_date = course.end if course.self_paced else self.due
         return (
-            self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) or
+            self.runtime.user_is_staff or
             self.verify_current_content_visibility(hidden_date, self.hide_after_due)
         )
 
@@ -644,9 +643,8 @@ 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, user_id
+                self.course_id, self.location, self.runtime.user_id
             )
             return fulfilled
 
@@ -694,8 +692,7 @@ 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.
         """
-        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):
+        if self.runtime.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
@@ -726,8 +723,7 @@ def _compute_is_prereq_met(self, recalc_on_unmet):
         """
         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)
-            return gating_service.compute_is_prereq_met(self.location, user_id, recalc_on_unmet)
+            return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet)
 
         return True, {}
 
@@ -919,10 +915,8 @@ def _time_limited_student_view(self):
             self.is_time_limited
         )
         if feature_enabled:
-            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'
+            user_id = self.runtime.user_id
+            user_role_in_course = 'staff' if self.runtime.user_is_staff else 'student'
             course_id = self.runtime.course_id
             content_id = self.location