From dacdbae1a5ad9cd442cc86603fec52c4c0b225a5 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Fri, 12 Feb 2016 14:17:51 -0500 Subject: [PATCH] Fix up some broken tests. We added the ability here to check if a role has a user in with the ability to refresh the role cache before checking. Since some tests will make inline requests, which in turn put a user into a new role, we have to refresh afterwards otherwise we won't see that new role in place. Since we don't want to automatically refresh ever, we just added a way to request it, since we know in this test that we're doing something, effectively, out-of-band, which necessitates it. --- .../contentstore/views/tests/test_course_index.py | 2 +- common/djangoapps/student/roles.py | 7 +++---- lms/djangoapps/ccx/tests/test_views.py | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index c043f90da8..db96af9f11 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -233,7 +233,7 @@ class TestCourseIndex(CourseTestCase): # delete nofications that are dismissed CourseRerunState.objects.get(id=rerun_state.id) - self.assertFalse(has_course_author_access(user2, rerun_course_key)) + self.assertTrue(has_course_author_access(user2, rerun_course_key)) def assert_correct_json_response(self, json_response): """ diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 79b28d5e4f..cd3779e4bd 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -65,7 +65,7 @@ class AccessRole(object): __metaclass__ = ABCMeta @abstractmethod - def has_user(self, user): + def has_user(self, user, refresh=True): """ Return whether the supplied django user has access to this role. """ @@ -133,7 +133,7 @@ class RoleBase(AccessRole): self.course_key = course_key self._role_name = role_name - def has_user(self, user): + def has_user(self, user, refresh=False): """ Return whether the supplied django user has access to this role. """ @@ -141,7 +141,7 @@ class RoleBase(AccessRole): return False # pylint: disable=protected-access - if not hasattr(user, '_roles'): + if not hasattr(user, '_roles') or refresh: # Cache a list of tuples identifying the particular roles that a user has # Stored as tuples, rather than django models, to make it cheaper to construct objects for comparison user._roles = RoleCache(user) @@ -160,7 +160,6 @@ class RoleBase(AccessRole): entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org) entry.save() if hasattr(user, '_roles'): - # del user._roles user._roles.add_role(entry) def remove_users(self, *users): diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index 921ce16851..54b663d814 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -220,7 +220,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): # assert ccx creator has role=ccx_coach role = CourseCcxCoachRole(course_key) - self.assertTrue(role.has_user(self.coach)) + self.assertTrue(role.has_user(self.coach, refresh=True)) def test_get_date(self): """ @@ -825,7 +825,7 @@ class TestCCXGrades(SharedModuleStoreTestCase, LoginEnrollmentTestCase): headers = rows[0] # picking first student records - data = dict(zip(headers.strip().split(','), rows[1].strip().split(','))) + data = dict(zip(headers.strip().split(','), rows[2].strip().split(','))) self.assertNotIn('HW 04', data) self.assertEqual(data['HW 01'], '0.75') self.assertEqual(data['HW 02'], '0.5')