Commit 76df0f06 by Nimisha Asthagiri

Merge pull request #11057 from edx/course_api/has_access_performance

Performance enhancement in see_exists: reverse order of checks
parents 765f5285 44abf7a9
...@@ -318,7 +318,7 @@ def _has_access_course(user, action, courselike): ...@@ -318,7 +318,7 @@ def _has_access_course(user, action, courselike):
Can see if can enroll, but also if can load it: if user enrolled in a course and now Can see if can enroll, but also if can load it: if user enrolled in a course and now
it's past the enrollment period, they should still see it. it's past the enrollment period, they should still see it.
""" """
return ACCESS_GRANTED if (can_enroll() or can_load()) else ACCESS_DENIED return ACCESS_GRANTED if (can_load() or can_enroll()) else ACCESS_DENIED
def can_see_in_catalog(): def can_see_in_catalog():
""" """
......
...@@ -7,8 +7,9 @@ import ddt ...@@ -7,8 +7,9 @@ import ddt
import itertools import itertools
import pytz import pytz
from django.test import TestCase from django.contrib.auth.models import User
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.test import TestCase
from mock import Mock, patch from mock import Mock, patch
from nose.plugins.attrib import attr from nose.plugins.attrib import attr
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
...@@ -504,15 +505,9 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): ...@@ -504,15 +505,9 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
self.user_staff = UserFactory.create(is_staff=True) self.user_staff = UserFactory.create(is_staff=True)
self.user_anonymous = AnonymousUserFactory.create() self.user_anonymous = AnonymousUserFactory.create()
ENROLL_TEST_DATA = list(itertools.product( COURSE_TEST_DATA = list(itertools.product(
['user_normal', 'user_staff', 'user_anonymous'], ['user_normal', 'user_staff', 'user_anonymous'],
['enroll'], ['enroll', 'load', 'staff', 'instructor', 'see_exists', 'see_in_catalog', 'see_about_page'],
['course_default', 'course_started', 'course_not_started', 'course_staff_only'],
))
LOAD_TEST_DATA = list(itertools.product(
['user_normal', 'user_beta_tester', 'user_staff'],
['load'],
['course_default', 'course_started', 'course_not_started', 'course_staff_only'], ['course_default', 'course_started', 'course_not_started', 'course_staff_only'],
)) ))
...@@ -528,8 +523,9 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): ...@@ -528,8 +523,9 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
['course_default', 'course_with_pre_requisite', 'course_with_pre_requisites'], ['course_default', 'course_with_pre_requisite', 'course_with_pre_requisites'],
)) ))
@ddt.data(*(ENROLL_TEST_DATA + LOAD_TEST_DATA + LOAD_MOBILE_TEST_DATA + PREREQUISITES_TEST_DATA)) @ddt.data(*(COURSE_TEST_DATA + LOAD_MOBILE_TEST_DATA + PREREQUISITES_TEST_DATA))
@ddt.unpack @ddt.unpack
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_course_overview_access(self, user_attr_name, action, course_attr_name): def test_course_overview_access(self, user_attr_name, action, course_attr_name):
""" """
Check that a user's access to a course is equal to the user's access to Check that a user's access to a course is equal to the user's access to
...@@ -562,3 +558,35 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): ...@@ -562,3 +558,35 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
overview = CourseOverview.get_from_id(self.course_default.id) overview = CourseOverview.get_from_id(self.course_default.id)
with self.assertRaises(ValueError): with self.assertRaises(ValueError):
access.has_access(self.user, '_non_existent_action', overview) access.has_access(self.user, '_non_existent_action', overview)
@ddt.data(
*itertools.product(
['user_normal', 'user_staff', 'user_anonymous'],
['see_exists', 'see_in_catalog', 'see_about_page'],
['course_default', 'course_started', 'course_not_started'],
)
)
@ddt.unpack
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_course_catalog_access_num_queries(self, user_attr_name, action, course_attr_name):
course = getattr(self, course_attr_name)
# get a fresh user object that won't have any cached role information
if user_attr_name == 'user_anonymous':
user = AnonymousUserFactory()
else:
user = getattr(self, user_attr_name)
user = User.objects.get(id=user.id)
if user_attr_name == 'user_staff' and action == 'see_exists' and course_attr_name == 'course_not_started':
# checks staff role
num_queries = 1
elif user_attr_name == 'user_normal' and action == 'see_exists' and course_attr_name != 'course_started':
# checks staff role and enrollment data
num_queries = 2
else:
num_queries = 0
course_overview = CourseOverview.get_from_id(course.id)
with self.assertNumQueries(num_queries):
bool(access.has_access(user, action, course_overview, course_key=course.id))
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment