Fix Mako templates to use latest request context

Fixed the edX middleware to lazily create request
context for Mako templates, so that it is based on
any changes made to the request object.

Verified that with this fix the "Instructor" tab is
correctly hidden when a staff member is viewing
the course as a student.

common/djangoapps/edxmako/middleware.py

@@ -7,7 +7,7 @@
 #       http://www.apache.org/licenses/LICENSE-2.0
 #
 #   Unless required by applicable law or agreed to in writing, software
-#   distribuetd under the License is distributed on an "AS IS" BASIS,
+#   distributed under the License is distributed on an "AS IS" BASIS,
 #   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
@@ -22,10 +22,24 @@ REQUEST_CONTEXT = threading.local()
 class MakoMiddleware(object):
 
     def process_request(self, request):
-        REQUEST_CONTEXT.context = RequestContext(request)
-        REQUEST_CONTEXT.context['is_secure'] = request.is_secure()
-        REQUEST_CONTEXT.context['site'] = safe_get_host(request)
+        """ Process the middleware request. """
+        REQUEST_CONTEXT.request = request
 
-    def process_response(self, request, response):
-        REQUEST_CONTEXT.context = None
+    def process_response(self, __, response):
+        """ Process the middleware response. """
+        REQUEST_CONTEXT.request = None
         return response
+
+
+def get_template_request_context():
+    """
+    Returns the template processing context to use for the current request,
+    or returns None if there is not a current request.
+    """
+    request = getattr(REQUEST_CONTEXT, "request", None)
+    if not request:
+        return None
+    context = RequestContext(request)
+    context['is_secure'] = request.is_secure()
+    context['site'] = safe_get_host(request)
+    return context

common/djangoapps/edxmako/shortcuts.py

@@ -19,7 +19,7 @@ import logging
 from microsite_configuration import microsite
 
 from edxmako import lookup_template
-import edxmako.middleware
+from edxmako.middleware import get_template_request_context
 from django.conf import settings
 from django.core.urlresolvers import reverse
 log = logging.getLogger(__name__)
@@ -114,11 +114,12 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'):
     context_instance['marketing_link'] = marketing_link
 
     # In various testing contexts, there might not be a current request context.
-    if getattr(edxmako.middleware.REQUEST_CONTEXT, "context", None):
-        for d in edxmako.middleware.REQUEST_CONTEXT.context:
-            context_dictionary.update(d)
-    for d in context_instance:
-        context_dictionary.update(d)
+    request_context = get_template_request_context()
+    if request_context:
+        for item in request_context:
+            context_dictionary.update(item)
+    for item in context_instance:
+        context_dictionary.update(item)
     if context:
         context_dictionary.update(context)
     # fetch and render template

common/djangoapps/edxmako/template.py

@@ -12,12 +12,12 @@
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
 
+import edxmako
+
 from django.conf import settings
-from mako.template import Template as MakoTemplate
+from edxmako.middleware import get_template_request_context
 from edxmako.shortcuts import marketing_link
-
-import edxmako
-import edxmako.middleware
+from mako.template import Template as MakoTemplate
 
 DJANGO_VARIABLES = ['output_encoding', 'encoding_errors']
 
@@ -48,11 +48,12 @@ class Template(MakoTemplate):
         context_dictionary = {}
 
         # In various testing contexts, there might not be a current request context.
-        if getattr(edxmako.middleware.REQUEST_CONTEXT, "context", None):
-            for d in edxmako.middleware.REQUEST_CONTEXT.context:
-                context_dictionary.update(d)
-        for d in context_instance:
-            context_dictionary.update(d)
+        request_context = get_template_request_context()
+        if request_context:
+            for item in request_context:
+                context_dictionary.update(item)
+        for item in context_instance:
+            context_dictionary.update(item)
         context_dictionary['settings'] = settings
         context_dictionary['EDX_ROOT_URL'] = settings.EDX_ROOT_URL
         context_dictionary['django_context'] = context_instance

common/djangoapps/edxmako/tests.py

@@ -10,6 +10,7 @@ from django.test.utils import override_settings
 from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
 import edxmako.middleware
+from edxmako.middleware import get_template_request_context
 from edxmako import add_lookup, LOOKUP
 from edxmako.shortcuts import (
     marketing_link,
@@ -83,11 +84,11 @@ class MakoMiddlewareTest(TestCase):
 
         self.middleware.process_request(self.request)
         # requestcontext should not be None.
-        self.assertIsNotNone(edxmako.middleware.REQUEST_CONTEXT.context)
+        self.assertIsNotNone(get_template_request_context())
 
         self.middleware.process_response(self.request, self.response)
         # requestcontext should be None.
-        self.assertIsNone(edxmako.middleware.REQUEST_CONTEXT.context)
+        self.assertIsNone(get_template_request_context())
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
     @patch("edxmako.middleware.REQUEST_CONTEXT")

common/test/acceptance/pages/lms/course_page.py

@@ -4,6 +4,7 @@ Base class for pages in courseware.
 
 from bok_choy.page_object import PageObject
 from . import BASE_URL
+from .tab_nav import TabNavPage
 
 
 class CoursePage(PageObject):
@@ -29,3 +30,11 @@ class CoursePage(PageObject):
         Construct a URL to the page within the course.
         """
         return BASE_URL + "/courses/" + self.course_id + "/" + self.url_path
+
+    def has_tab(self, tab_name):
+        """
+        Returns true if the current page is showing a tab with the given name.
+        :return:
+        """
+        tab_nav = TabNavPage(self.browser)
+        return tab_name in tab_nav.tab_names

common/test/acceptance/pages/lms/staff_view.py

@@ -2,9 +2,10 @@
 Staff view of courseware
 """
 from bok_choy.page_object import PageObject
+from .courseware import CoursewarePage
 
 
-class StaffPage(PageObject):
+class StaffPage(CoursewarePage):
     """
     View of courseware pages while logged in as course staff
     """
@@ -13,6 +14,8 @@ class StaffPage(PageObject):
     STAFF_STATUS_CSS = '#staffstatus'
 
     def is_browser_on_page(self):
+        if not super(StaffPage, self).is_browser_on_page():
+            return False
         return self.q(css=self.STAFF_STATUS_CSS).present
 
     @property

common/test/acceptance/pages/lms/tab_nav.py

@@ -48,7 +48,7 @@ class TabNavPage(PageObject):
         Return the CSS to click for `tab_name`.
         If no tabs exist for that name, return `None`.
         """
-        all_tabs = self._tab_names
+        all_tabs = self.tab_names
 
         try:
             tab_index = all_tabs.index(tab_name)
@@ -58,7 +58,7 @@ class TabNavPage(PageObject):
             return 'ol.course-tabs li:nth-of-type({0}) a'.format(tab_index + 1)
 
     @property
-    def _tab_names(self):
+    def tab_names(self):
         """
         Return the list of available tab names.  If no tab names
         are available, wait for them to load.  Raises a `BrokenPromiseError`

common/test/acceptance/tests/lms/test_lms_staff_view.py

@@ -11,15 +11,15 @@ from ...fixtures.course import CourseFixture, XBlockFixtureDesc
 from textwrap import dedent
 
 
-class StaffDebugTest(UniqueCourseTest):
+class StaffViewTest(UniqueCourseTest):
     """
-    Tests that verify the staff debug info.
+    Tests that verify the staff view.
     """
     USERNAME = "STAFF_TESTER"
     EMAIL = "johndoe@example.com"
 
     def setUp(self):
-        super(StaffDebugTest, self).setUp()
+        super(StaffViewTest, self).setUp()
 
         self.courseware_page = CoursewarePage(self.browser, self.course_id)
 
@@ -59,10 +59,31 @@ class StaffDebugTest(UniqueCourseTest):
         Open staff page with assertion
         """
         self.courseware_page.visit()
-        staff_page = StaffPage(self.browser)
+        staff_page = StaffPage(self.browser, self.course_id)
         self.assertEqual(staff_page.staff_status, 'Staff view')
         return staff_page
 
+
+class StaffViewToggleTest(StaffViewTest):
+    """
+    Tests for the staff view toggle button.
+    """
+    def test_instructor_tab_visibility(self):
+        """
+        Test that the instructor tab is hidden when viewing as a student.
+        """
+
+        course_page = self._goto_staff_page()
+        self.assertTrue(course_page.has_tab('Instructor'))
+        course_page.toggle_staff_view()
+        self.assertEqual(course_page.staff_status, 'Student view')
+        self.assertFalse(course_page.has_tab('Instructor'))
+
+
+class StaffDebugTest(StaffViewTest):
+    """
+    Tests that verify the staff debug info.
+    """
     def test_reset_attempts_empty(self):
         """
         Test that we reset even when there is no student state

common/test/acceptance/tests/studio/test_studio_container.py

@@ -678,7 +678,7 @@ class UnitPublishingTest(ContainerBase):
         """
         Verifies that the browser is on the staff page and returns a StaffPage.
         """
-        page = StaffPage(self.browser)
+        page = StaffPage(self.browser, self.course_id)
         EmptyPromise(page.is_browser_on_page, 'Browser is on staff page in LMS').fulfill()
         return page
 

common/test/acceptance/tests/studio/test_studio_outline.py

@@ -718,7 +718,7 @@ class StaffLockTest(CourseOutlineTest):
         courseware = CoursewarePage(self.browser, self.course_id)
         courseware.wait_for_page()
         self.assertEqual(courseware.num_sections, 2)
-        StaffPage(self.browser).toggle_staff_view()
+        StaffPage(self.browser, self.course_id).toggle_staff_view()
         self.assertEqual(courseware.num_sections, 1)
 
     def test_locked_subsections_do_not_appear_in_lms(self):
@@ -737,7 +737,7 @@ class StaffLockTest(CourseOutlineTest):
         courseware = CoursewarePage(self.browser, self.course_id)
         courseware.wait_for_page()
         self.assertEqual(courseware.num_subsections, 2)
-        StaffPage(self.browser).toggle_staff_view()
+        StaffPage(self.browser, self.course_id).toggle_staff_view()
         self.assertEqual(courseware.num_subsections, 1)
 
     def test_toggling_staff_lock_on_section_does_not_publish_draft_units(self):

lms/djangoapps/courseware/module_render.py

@@ -92,7 +92,7 @@ def make_track_function(request):
     return function
 
 
-def toc_for_course(user, request, course, active_chapter, active_section, field_data_cache):
+def toc_for_course(request, course, active_chapter, active_section, field_data_cache):
     '''
     Create a table of contents from the module store
 
@@ -117,7 +117,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
     '''
 
     with modulestore().bulk_operations(course.id):
-        course_module = get_module_for_descriptor(user, request, course, field_data_cache, course.id)
+        course_module = get_module_for_descriptor(request.user, request, course, field_data_cache, course.id)
         if course_module is None:
             return None
 

lms/djangoapps/courseware/tests/test_module_render.py

@@ -394,7 +394,7 @@ class TestTOC(ModuleStoreTestCase):
 
             with check_mongo_calls(toc_finds):
                 actual = render.toc_for_course(
-                    self.request.user, self.request, self.toy_course, self.chapter, None, self.field_data_cache
+                    self.request, self.toy_course, self.chapter, None, self.field_data_cache
                 )
         for toc_section in expected:
             self.assertIn(toc_section, actual)
@@ -432,7 +432,9 @@ class TestTOC(ModuleStoreTestCase):
                           'url_name': 'secret:magic', 'display_name': 'secret:magic'}])
 
             with check_mongo_calls(toc_finds):
-                actual = render.toc_for_course(self.request.user, self.request, self.toy_course, self.chapter, section, self.field_data_cache)
+                actual = render.toc_for_course(
+                    self.request, self.toy_course, self.chapter, section, self.field_data_cache
+                )
             for toc_section in expected:
                 self.assertIn(toc_section, actual)
 

lms/djangoapps/courseware/tests/test_views.py

@@ -555,7 +555,8 @@ class TestAccordionDueDate(BaseDueDateTests):
     def get_text(self, course):
         """ Returns the HTML for the accordion """
         return views.render_accordion(
-            self.request, course, course.get_children()[0].scope_ids.usage_id.to_deprecated_string(), None, None
+            self.request, course, course.get_children()[0].scope_ids.usage_id.to_deprecated_string(),
+            None, None
         )
 
 

lms/djangoapps/courseware/views.py

@@ -118,9 +118,7 @@ def render_accordion(request, course, chapter, section, field_data_cache):
     Returns the html string
     """
     # grab the table of contents
-    user = User.objects.prefetch_related("groups").get(id=request.user.id)
-    request.user = user	 # keep just one instance of User
-    toc = toc_for_course(user, request, course, chapter, section, field_data_cache)
+    toc = toc_for_course(request, course, chapter, section, field_data_cache)
 
     context = dict([
         ('toc', toc),
@@ -325,10 +323,15 @@ def index(request, course_id, chapter=None, section=None,
 
     request.user = user  # keep just one instance of User
     with modulestore().bulk_operations(course_key):
-        return _index_bulk_op(request, user, course_key, chapter, section, position)
+        return _index_bulk_op(request, course_key, chapter, section, position)
 
 
-def _index_bulk_op(request, user, course_key, chapter, section, position):
+# pylint: disable=too-many-statements
+def _index_bulk_op(request, course_key, chapter, section, position):
+    """
+    Render the index page for the specified course.
+    """
+    user = request.user
     course = get_course_with_access(user, 'load', course_key, depth=2)
 
     staff_access = has_access(user, 'staff', course)