From 2cd265292fbfb635d34ca199fd7f424a8eb4af89 Mon Sep 17 00:00:00 2001 From: Awais Date: Wed, 15 Apr 2015 18:21:39 +0500 Subject: [PATCH] ECOM-1421 implementing the functionality --- common/djangoapps/embargo/api.py | 9 ++++-- common/djangoapps/embargo/models.py | 29 +++++++++++++++++-- common/djangoapps/embargo/test_utils.py | 3 +- .../embargo/tests/test_middleware.py | 10 +++++-- .../djangoapps/embargo/tests/test_models.py | 10 ++++++- 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/common/djangoapps/embargo/api.py b/common/djangoapps/embargo/api.py index 25e85a8050..1142db9116 100644 --- a/common/djangoapps/embargo/api.py +++ b/common/djangoapps/embargo/api.py @@ -18,7 +18,8 @@ log = logging.getLogger(__name__) def redirect_if_blocked(course_key, access_point='enrollment', **kwargs): - """Redirect if the user does not have access to the course. + """Redirect if the user does not have access to the course. In case of blocked if access_point + is not enrollment and course has enabled is_disabled_access_check then user can view that course. Arguments: course_key (CourseKey): Location of the course the user is trying to access. @@ -30,7 +31,11 @@ def redirect_if_blocked(course_key, access_point='enrollment', **kwargs): if settings.FEATURES.get('EMBARGO'): is_blocked = not check_course_access(course_key, **kwargs) if is_blocked: - return message_url_path(course_key, access_point) + if access_point == "courseware": + if not RestrictedCourse.is_disabled_access_check(course_key): + return message_url_path(course_key, access_point) + else: + return message_url_path(course_key, access_point) def check_course_access(course_key, user=None, ip_address=None, url=None): diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py index 82c09c30fe..5c8192fdc5 100644 --- a/common/djangoapps/embargo/models.py +++ b/common/djangoapps/embargo/models.py @@ -162,14 +162,39 @@ class RestrictedCourse(models.Model): """ return unicode(course_id) in cls._get_restricted_courses_from_cache() + @classmethod + def is_disabled_access_check(cls, course_id): + """ + Check if the course is in restricted list has disabled_access_check + + Args: + course_id (str): course_id to look for + + Returns: + Boolean + disabled_access_check attribute of restricted course + """ + + # checking is_restricted_course method also here to make sure course exists in the list otherwise in case of + # no course found it will throw the key not found error on 'disable_access_check' + return ( + cls.is_restricted_course(unicode(course_id)) + and cls._get_restricted_courses_from_cache().get(unicode(course_id))["disable_access_check"] + ) + @classmethod def _get_restricted_courses_from_cache(cls): """ - Cache all restricted courses and returns the list of course_keys that are restricted + Cache all restricted courses and returns the dict of course_keys and disable_access_check that are restricted """ restricted_courses = cache.get(cls.COURSE_LIST_CACHE_KEY) if restricted_courses is None: - restricted_courses = list(RestrictedCourse.objects.values_list('course_key', flat=True)) + restricted_courses = { + unicode(course.course_key): { + 'disable_access_check': course.disable_access_check + } + for course in RestrictedCourse.objects.all() + } cache.set(cls.COURSE_LIST_CACHE_KEY, restricted_courses) return restricted_courses diff --git a/common/djangoapps/embargo/test_utils.py b/common/djangoapps/embargo/test_utils.py index a278e5d49d..dec25cf758 100644 --- a/common/djangoapps/embargo/test_utils.py +++ b/common/djangoapps/embargo/test_utils.py @@ -10,7 +10,7 @@ from embargo.models import Country, CountryAccessRule, RestrictedCourse @contextlib.contextmanager -def restrict_course(course_key, access_point="enrollment"): +def restrict_course(course_key, access_point="enrollment", disable_access_check=False): """Simulate that a course is restricted. This does two things: @@ -58,6 +58,7 @@ def restrict_course(course_key, access_point="enrollment"): restricted_course, __ = RestrictedCourse.objects.get_or_create(course_key=course_key) restricted_course.enroll_msg_key = 'default' restricted_course.access_msg_key = 'default' + restricted_course.disable_access_check = disable_access_check restricted_course.save() # Ensure that there is a blacklist rule for the country diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index 3e26b9bf4c..b3c16fa668 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -51,10 +51,14 @@ class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase): config_cache.clear() @patch.dict(settings.FEATURES, {'EMBARGO': True}) - def test_blocked(self): - with restrict_course(self.course.id, access_point='courseware') as redirect_url: + @ddt.data(True, False) + def test_blocked(self, disable_access_check): + with restrict_course(self.course.id, access_point='courseware', disable_access_check=disable_access_check) as redirect_url: # pylint: disable=line-too-long response = self.client.get(self.courseware_url) - self.assertRedirects(response, redirect_url) + if disable_access_check: + self.assertEqual(response.status_code, 200) + else: + self.assertRedirects(response, redirect_url) @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_allowed(self): diff --git a/common/djangoapps/embargo/tests/test_models.py b/common/djangoapps/embargo/tests/test_models.py index caf8422c70..79d378ca1b 100644 --- a/common/djangoapps/embargo/tests/test_models.py +++ b/common/djangoapps/embargo/tests/test_models.py @@ -119,20 +119,28 @@ class RestrictedCourseTest(TestCase): # Warm the cache with self.assertNumQueries(1): RestrictedCourse.is_restricted_course(course_id) + RestrictedCourse.is_disabled_access_check(course_id) # it should come from cache with self.assertNumQueries(0): RestrictedCourse.is_restricted_course(course_id) + RestrictedCourse.is_disabled_access_check(course_id) + + self.assertFalse(RestrictedCourse.is_disabled_access_check(course_id)) # add new the course so the cache must get delete and again hit the db new_course_id = CourseLocator('def', '123', 'doremi') - RestrictedCourse.objects.create(course_key=new_course_id) + RestrictedCourse.objects.create(course_key=new_course_id, disable_access_check=True) with self.assertNumQueries(1): RestrictedCourse.is_restricted_course(new_course_id) + RestrictedCourse.is_disabled_access_check(new_course_id) # it should come from cache with self.assertNumQueries(0): RestrictedCourse.is_restricted_course(new_course_id) + RestrictedCourse.is_disabled_access_check(new_course_id) + + self.assertTrue(RestrictedCourse.is_disabled_access_check(new_course_id)) # deleting an object will delete cache also.and hit db on # get the is_restricted course