From e7078046702810cd25bbac6a742da4ab689ccc93 Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Thu, 30 Aug 2018 14:52:59 -0400 Subject: [PATCH] Revert "EDUCATOR-3374 | Hacky query to fix inline discussions performance." This reverts commit 4a1caf6c030b9a27137f35e00e585923f179a025. --- .../djangoapps/django_comment_common/models.py | 18 +++--------------- .../courseware/tests/test_discussion_xblock.py | 8 +++----- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index a0b48c4c2f..0d27aa9a07 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -145,25 +145,13 @@ def all_permissions_for_user_in_course(user, course_id): # pylint: disable=inva if course is None: raise ItemNotFoundError(course_id) - all_roles = {role for role in Role.objects.filter(users=user, course_id=course_id)} - role_names = {role.name for role in all_roles} - - # TODO: EDUCATOR-3374 - # The `roles__users__roles__in=all_roles` part of this filter is a hack to get the new - # Aurora MySql query planner to properly use the unique index on the roles/users join table. - # Without this hack, the planner uses a unique index on (role_id, user_id) to do lookup - # by user_id only, which is a completely inefficient way to use that index. - permission_queryset = Permission.objects.filter( - roles__users=user, - roles__course_id=course_id, - roles__users__roles__in=all_roles - ).distinct() + all_roles = {role.name for role in Role.objects.filter(users=user, course_id=course_id)} permissions = { permission.name for permission - in permission_queryset - if not permission_blacked_out(course, role_names, permission.name) + in Permission.objects.filter(roles__users=user, roles__course_id=course_id) + if not permission_blacked_out(course, all_roles, permission.name) } return permissions diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index d57267d4cc..10b8bbacd5 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -402,13 +402,11 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase): discussion_target='Target Discussion', )) - # 2 queries are required to do first discussion xblock render: + # 3 queries are required to do first discussion xblock render: # * django_comment_client_role + # * django_comment_client_permission # * lms_xblock_xblockasidesconfig - # If the query for roles returned a non-empty result set, there would be - # an additional query against django_comment_client_permission, but there - # are no roles associated with this test. - num_queries = 2 + num_queries = 3 for discussion in discussions: discussion_xblock = get_module_for_descriptor_internal( user=user,