From 5f46ea52cd26a0db3b517ff2803d8a54da19a6ee Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 2 Jun 2023 17:16:20 +0200 Subject: [PATCH] feat: remove field data binding from the runtime --- lms/djangoapps/courseware/block_render.py | 14 ++----------- .../courseware/tests/test_block_render.py | 20 +++++++++---------- lms/djangoapps/courseware/tests/test_views.py | 2 +- .../schedules/tests/test_resolvers.py | 2 +- .../tests/views/test_course_updates.py | 2 +- xmodule/services.py | 6 ++---- 6 files changed, 17 insertions(+), 29 deletions(-) diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index e7675893f5..5c8d4bbc5b 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -433,9 +433,6 @@ def prepare_runtime_for_user( Arguments: see arguments for get_block() request_token (str): A token unique to the request use by xblock initialization - - Returns: - KvsFieldData: student_data bound to, primarily, the user and block """ def inner_get_block(block): @@ -524,14 +521,11 @@ def prepare_runtime_for_user( if staff_access: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) - field_data = DateLookupFieldData(block._field_data, course_id, user) # pylint: disable=protected-access - field_data = LmsFieldData(field_data, student_data) - store = modulestore() services = { 'fs': FSService(), - 'field-data': field_data, + 'field-data': student_data, 'mako': mako_service, 'user': DjangoXBlockUserService( user, @@ -601,8 +595,6 @@ def prepare_runtime_for_user( block.runtime.set('position', position) - return field_data - # TODO: Find all the places that this method is called and figure out how to # get a loaded course passed into it @@ -619,7 +611,7 @@ def get_block_for_descriptor_internal(user, block, student_data, course_id, trac request_token (str): A unique token for this request, used to isolate xblock rendering """ - student_data = prepare_runtime_for_user( + prepare_runtime_for_user( user=user, student_data=student_data, # These have implicit user bindings, the rest of args are considered not to block=block, @@ -645,8 +637,6 @@ def get_block_for_descriptor_internal(user, block, student_data, course_id, trac ], ) - block.scope_ids = block.scope_ids._replace(user_id=user.id) - # Do not check access when it's a noauth request. # Not that the access check needs to happen after the block is bound # for the student, since there may be field override data for the student diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 33ed581c68..bd1e744191 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -2283,7 +2283,7 @@ class LMSXBlockServiceMixin(SharedModuleStoreTestCase): """ Instantiate the runtem. """ - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2654,7 +2654,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.track_function = Mock() self.request_token = Mock() self.contentstore = contentstore() - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2682,7 +2682,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): if is_staff: self.user = StaffFactory(course_key=self.course.id) - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2704,7 +2704,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): if is_global_staff: self.user = GlobalStaffFactory.create() - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2724,7 +2724,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): if is_beta_tester: self.user = BetaTesterFactory(course_key=self.course.id) - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2745,7 +2745,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): if is_instructor: self.user = InstructorFactory(course_key=self.course.id) - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2774,7 +2774,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): anonymous_student_id value. """ - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.problem_block, @@ -2788,7 +2788,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID ) == anonymous_id_for_user(self.user, None) - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, @@ -2808,7 +2808,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): ) == anonymous_id_for_user(self.user, None) def test_user_service_with_anonymous_user(self): - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( AnonymousUser(), self.student_data, self.block, @@ -2837,7 +2837,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): Newer code should use the user service, which gets tested in test_user_service.py """ - _ = render.prepare_runtime_for_user( + render.prepare_runtime_for_user( self.user, self.student_data, self.block, diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 02419c7f94..c7d7ce7b17 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -354,7 +354,7 @@ class IndexQueryTestCase(ModuleStoreTestCase): self.client.login(username=self.user.username, password=self.user_password) CourseEnrollment.enroll(self.user, course.id) - with self.assertNumQueries(203, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(177, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = reverse( 'courseware_section', diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 8bc7f9b4e7..5881861b22 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -274,7 +274,7 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor def test_schedule_context(self): resolver = self.create_resolver() # using this to make sure the select_related stays intact - with self.assertNumQueries(40): + with self.assertNumQueries(30): sc = resolver.get_schedules() schedules = list(sc) apple_logo_url = 'http://email-media.s3.amazonaws.com/edX/2021/store_apple_229x78.jpg' diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 851d4d86e6..379be52ed4 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -49,7 +49,7 @@ class TestCourseUpdatesPage(BaseCourseUpdatesTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = course_updates_url(self.course) self.client.get(url) diff --git a/xmodule/services.py b/xmodule/services.py index 595c8082fb..88cbc183dc 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -201,7 +201,7 @@ class RebindUserService(Service): with modulestore().bulk_operations(self.course_id): course = modulestore().get_course(course_key=self.course_id) - inner_student_data = self._ref["prepare_runtime_for_user"]( + self._ref["prepare_runtime_for_user"]( user=real_user, student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to block=block, @@ -215,12 +215,10 @@ class RebindUserService(Service): [ partial(DateLookupFieldData, course_id=self.course_id, user=self.user), partial(OverrideFieldData.wrap, real_user, course), - partial(LmsFieldData, student_data=inner_student_data), + partial(LmsFieldData, student_data=student_data_real_user), ], ) - block.scope_ids = block.scope_ids._replace(user_id=real_user.id) - class EventPublishingService(Service): """