diff --git a/cms/djangoapps/contentstore/views/error.py b/cms/djangoapps/contentstore/views/error.py index 2eea0bc2a7..6f58ea296d 100644 --- a/cms/djangoapps/contentstore/views/error.py +++ b/cms/djangoapps/contentstore/views/error.py @@ -39,9 +39,9 @@ def server_error(request): @jsonable_error(404, "Resource not found") def render_404(request): - return HttpResponseNotFound(render_to_string('404.html', {})) + return HttpResponseNotFound(render_to_string('404.html', {}, request=request)) @jsonable_error(500, "The Studio servers encountered an error") def render_500(request): - return HttpResponseServerError(render_to_string('500.html', {})) + return HttpResponseServerError(render_to_string('500.html', {}, request=request)) diff --git a/cms/djangoapps/course_creators/tests/test_views.py b/cms/djangoapps/course_creators/tests/test_views.py index fdca3b8d56..fca8e7faaa 100644 --- a/cms/djangoapps/course_creators/tests/test_views.py +++ b/cms/djangoapps/course_creators/tests/test_views.py @@ -5,13 +5,13 @@ Tests course_creators.views.py. from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied from django.test import TestCase, RequestFactory +from django.core.urlresolvers import reverse from course_creators.views import add_user_with_status_unrequested, add_user_with_status_granted from course_creators.views import get_course_creator_status, update_course_creator_group, user_requested_access import mock from student.roles import CourseCreatorRole from student import auth -from edxmako.tests import mako_middleware_process_request class CourseCreatorView(TestCase): @@ -73,11 +73,12 @@ class CourseCreatorView(TestCase): add_user_with_status_unrequested(self.user) self.assertEqual('unrequested', get_course_creator_status(self.user)) - request = RequestFactory().get('/') - request.user = self.user + self.client.login(username=self.user.username, password='foo') - mako_middleware_process_request(request) - user_requested_access(self.user) + # The user_requested_access function renders a template that requires + # request-specific information. Use the django TestClient to supply + # the appropriate request context. + self.client.post(reverse('request_course_creator')) self.assertEqual('pending', get_course_creator_status(self.user)) def test_user_requested_already_granted(self): diff --git a/cms/envs/common.py b/cms/envs/common.py index 2d4e6bf30e..a3204be72f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -312,6 +312,7 @@ simplefilter('ignore') ################################# Middleware ################################### MIDDLEWARE_CLASSES = ( + 'crum.CurrentRequestUserMiddleware', 'request_cache.middleware.RequestCache', 'header_control.middleware.HeaderControlMiddleware', 'django.middleware.cache.UpdateCacheMiddleware', @@ -332,7 +333,6 @@ MIDDLEWARE_CLASSES = ( 'student.middleware.UserStandingMiddleware', 'contentserver.middleware.StaticContentServer', - 'crum.CurrentRequestUserMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'track.middleware.TrackMiddleware', @@ -350,9 +350,6 @@ MIDDLEWARE_CLASSES = ( 'codejail.django_integration.ConfigureCodeJailMiddleware', - # needs to run after locale middleware (or anything that modifies the request context) - 'edxmako.middleware.MakoMiddleware', - # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500 'ratelimitbackend.middleware.RateLimitMiddleware', diff --git a/cms/urls.py b/cms/urls.py index f302d15073..21c92da9fb 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -76,7 +76,7 @@ urlpatterns += patterns( url(r'^howitworks$', 'howitworks'), url(r'^signup$', 'signup', name='signup'), url(r'^signin$', 'login_page', name='login'), - url(r'^request_course_creator$', 'request_course_creator'), + url(r'^request_course_creator$', 'request_course_creator', name='request_course_creator'), url(r'^course_team/{}(?:/(?P.+))?$'.format(COURSELIKE_KEY_PATTERN), 'course_team_handler'), url(r'^course_info/{}$'.format(settings.COURSE_KEY_PATTERN), 'course_info_handler'), diff --git a/common/djangoapps/edxmako/middleware.py b/common/djangoapps/edxmako/request_context.py similarity index 76% rename from common/djangoapps/edxmako/middleware.py rename to common/djangoapps/edxmako/request_context.py index e5268b4f55..b10b8753ae 100644 --- a/common/djangoapps/edxmako/middleware.py +++ b/common/djangoapps/edxmako/request_context.py @@ -11,29 +11,22 @@ # 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. +# +# This file has been modified by edX.org + +""" +Methods for creating RequestContext for using with Mako templates. +""" + -import threading from django.conf import settings from django.template import RequestContext from django.template.context import _builtin_context_processors from django.utils.module_loading import import_string from util.request import safe_get_host +from crum import get_current_request -from request_cache.middleware import RequestCache - -REQUEST_CONTEXT = threading.local() - - -class MakoMiddleware(object): - - def process_request(self, request): - """ Process the middleware request. """ - REQUEST_CONTEXT.request = request - - def process_response(self, __, response): - """ Process the middleware response. """ - REQUEST_CONTEXT.request = None - return response +import request_cache def get_template_context_processors(): @@ -45,21 +38,25 @@ def get_template_context_processors(): return tuple(import_string(path) for path in context_processors) -def get_template_request_context(): +def get_template_request_context(request=None): """ 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: + + if request is None: + request = get_current_request() + + if request is None: return None - request_cache_dict = RequestCache.get_request_cache().data - cache_key = "edxmako_request_context" + request_cache_dict = request_cache.get_cache('edxmako') + cache_key = "request_context" if cache_key in request_cache_dict: return request_cache_dict[cache_key] context = RequestContext(request) + context['is_secure'] = request.is_secure() context['site'] = safe_get_host(request) diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index da5ddfe3cd..94cb1cdd01 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -19,7 +19,7 @@ import logging from microsite_configuration import microsite from edxmako import lookup_template -from edxmako.middleware import get_template_request_context +from edxmako.request_context import get_template_request_context from django.conf import settings from django.core.urlresolvers import reverse log = logging.getLogger(__name__) @@ -111,7 +111,28 @@ def microsite_footer_context_processor(request): ) -def render_to_string(template_name, dictionary, context=None, namespace='main'): +def render_to_string(template_name, dictionary, context=None, namespace='main', request=None): + """ + Render a Mako template to as a string. + + The following values are available to all templates: + settings: the django settings object + EDX_ROOT_URL: settings.EDX_ROOT_URL + marketing_link: The :func:`marketing_link` function + is_any_marketing_link_set: The :func:`is_any_marketing_link_set` function + is_marketing_link_set: The :func:`is_marketing_link_set` function + + Arguments: + template_name: The name of the template to render. Will be loaded + from the template paths specified in configuration. + dictionary: A dictionary of variables to insert into the template during + rendering. + context: A :class:`~django.template.Context` with values to make + available to the template. + namespace: The Mako namespace to find the named template in. + request: The request to use to construct the RequestContext for rendering + this template. If not supplied, the current request will be used. + """ # see if there is an override template defined in the microsite template_name = microsite.get_template_path(template_name) @@ -128,7 +149,7 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'): context_instance['is_marketing_link_set'] = is_marketing_link_set # In various testing contexts, there might not be a current request context. - request_context = get_template_request_context() + request_context = get_template_request_context(request) if request_context: for item in request_context: context_dictionary.update(item) @@ -148,11 +169,11 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'): return template.render_unicode(**context_dictionary) -def render_to_response(template_name, dictionary=None, context_instance=None, namespace='main', **kwargs): +def render_to_response(template_name, dictionary=None, context_instance=None, namespace='main', request=None, **kwargs): """ Returns a HttpResponse whose content is filled with the result of calling lookup.get_template(args[0]).render with the passed arguments. """ dictionary = dictionary or {} - return HttpResponse(render_to_string(template_name, dictionary, context_instance, namespace), **kwargs) + return HttpResponse(render_to_string(template_name, dictionary, context_instance, namespace, request), **kwargs) diff --git a/common/djangoapps/edxmako/template.py b/common/djangoapps/edxmako/template.py index 8584e4ecff..47dde3fabf 100644 --- a/common/djangoapps/edxmako/template.py +++ b/common/djangoapps/edxmako/template.py @@ -15,7 +15,7 @@ import edxmako from django.conf import settings -from edxmako.middleware import get_template_request_context +from edxmako.request_context import get_template_request_context from edxmako.shortcuts import marketing_link from mako.template import Template as MakoTemplate diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index 12ccb54274..8634916d5a 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -3,14 +3,14 @@ from mock import patch, Mock import unittest import ddt +from request_cache.middleware import RequestCache from django.conf import settings from django.http import HttpResponse from django.test import TestCase 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.request_context import get_template_request_context from edxmako import add_lookup, LOOKUP from edxmako.shortcuts import ( marketing_link, @@ -81,59 +81,76 @@ class AddLookupTests(TestCase): self.assertTrue(dirs[0].endswith('management')) -class MakoMiddlewareTest(TestCase): +class MakoRequestContextTest(TestCase): """ Test MakoMiddleware. """ def setUp(self): - super(MakoMiddlewareTest, self).setUp() - self.middleware = edxmako.middleware.MakoMiddleware() + super(MakoRequestContextTest, self).setUp() self.user = UserFactory.create() self.url = "/" self.request = RequestFactory().get(self.url) self.request.user = self.user self.response = Mock(spec=HttpResponse) - def test_clear_request_context_variable(self): + self.addCleanup(RequestCache.clear_request_cache) + + def test_with_current_request(self): """ - Test the global variable requestcontext is cleared correctly - when response middleware is called. + Test that if get_current_request returns a request, then get_template_request_context + returns a RequestContext. """ - self.middleware.process_request(self.request) - # requestcontext should not be None. - self.assertIsNotNone(get_template_request_context()) + with patch('edxmako.request_context.get_current_request', return_value=self.request): + # requestcontext should not be None. + self.assertIsNotNone(get_template_request_context()) - self.middleware.process_response(self.request, self.response) - # requestcontext should be None. - self.assertIsNone(get_template_request_context()) + def test_without_current_request(self): + """ + Test that if get_current_request returns None, then get_template_request_context + returns None. + """ + with patch('edxmako.request_context.get_current_request', return_value=None): + # requestcontext should be None. + self.assertIsNone(get_template_request_context()) + + def test_request_context_caching(self): + """ + Test that the RequestContext is cached in the RequestCache. + """ + with patch('edxmako.request_context.get_current_request', return_value=None): + # requestcontext should be None, because the cache isn't filled + self.assertIsNone(get_template_request_context()) + + with patch('edxmako.request_context.get_current_request', return_value=self.request): + # requestcontext should not be None, and should fill the cache + self.assertIsNotNone(get_template_request_context()) + + mock_get_current_request = Mock() + with patch('edxmako.request_context.get_current_request', mock_get_current_request): + # requestcontext should not be None, because the cache is filled + self.assertIsNotNone(get_template_request_context()) + mock_get_current_request.assert_not_called() + + RequestCache.clear_request_cache() + + with patch('edxmako.request_context.get_current_request', return_value=None): + # requestcontext should be None, because the cache isn't filled + self.assertIsNone(get_template_request_context()) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - @patch("edxmako.middleware.REQUEST_CONTEXT") - def test_render_to_string_when_no_global_context_lms(self, context_mock): + def test_render_to_string_when_no_global_context_lms(self): """ Test render_to_string() when makomiddleware has not initialized the threadlocal REQUEST_CONTEXT.context. This is meant to run in LMS. """ - del context_mock.context self.assertIn("this module is temporarily unavailable", render_to_string("courseware/error-message.html", None)) @unittest.skipUnless(settings.ROOT_URLCONF == 'cms.urls', 'Test only valid in cms') - @patch("edxmako.middleware.REQUEST_CONTEXT") - def test_render_to_string_when_no_global_context_cms(self, context_mock): + def test_render_to_string_when_no_global_context_cms(self): """ Test render_to_string() when makomiddleware has not initialized the threadlocal REQUEST_CONTEXT.context. This is meant to run in CMS. """ - del context_mock.context self.assertIn("We're having trouble rendering your component", render_to_string("html_error.html", None)) - - -def mako_middleware_process_request(request): - """ - Initialize the global RequestContext variable - edxmako.middleware.requestcontext using the request object. - """ - mako_middleware = edxmako.middleware.MakoMiddleware() - mako_middleware.process_request(request) diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index 6d58bd16a8..3c8a85ffd7 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -14,7 +14,6 @@ from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.contrib.auth.models import AnonymousUser, User from importlib import import_module -from edxmako.tests import mako_middleware_process_request from external_auth.models import ExternalAuthMap from external_auth.views import ( shib_login, course_specific_login, course_specific_register, _flatten_to_ascii @@ -97,18 +96,11 @@ class ShibSPTest(CacheIsolationTestCase): Tests that we get the error page when there is no REMOTE_USER or Shib-Identity-Provider in request.META """ - no_remote_user_request = self.request_factory.get('/shib-login') - no_remote_user_request.META.update({'Shib-Identity-Provider': IDP}) - no_remote_user_request.user = AnonymousUser() - - mako_middleware_process_request(no_remote_user_request) - no_remote_user_response = shib_login(no_remote_user_request) + no_remote_user_response = self.client.get(reverse('shib-login'), HTTP_SHIB_IDENTITY_PROVIDER=IDP) self.assertEqual(no_remote_user_response.status_code, 403) self.assertIn("identity server did not return your ID information", no_remote_user_response.content) - no_idp_request = self.request_factory.get('/shib-login') - no_idp_request.META.update({'REMOTE_USER': REMOTE_USER}) - no_idp_response = shib_login(no_idp_request) + no_idp_response = self.client.get(reverse('shib-login'), HTTP_REMOTE_USER=REMOTE_USER) self.assertEqual(no_idp_response.status_code, 403) self.assertIn("identity server did not return your ID information", no_idp_response.content) @@ -161,22 +153,22 @@ class ShibSPTest(CacheIsolationTestCase): for idp in idps: for remote_user in remote_users: - request = self.request_factory.get('/shib-login') - request.session = import_module(settings.SESSION_ENGINE).SessionStore() # empty session - request.META.update({'Shib-Identity-Provider': idp, - 'REMOTE_USER': remote_user, - 'mail': remote_user}) - request.user = AnonymousUser() - mako_middleware_process_request(request) + self.client.logout() with patch('external_auth.views.AUDIT_LOG') as mock_audit_log: - response = shib_login(request) + response = self.client.get( + reverse('shib-login'), + **{ + 'Shib-Identity-Provider': idp, + 'mail': remote_user, + 'REMOTE_USER': remote_user, + } + ) audit_log_calls = mock_audit_log.method_calls if idp == "https://idp.stanford.edu/" and remote_user == 'withmap@stanford.edu': - self.assertIsInstance(response, HttpResponseRedirect) - self.assertEqual(request.user, user_w_map) - self.assertEqual(response['Location'], '/dashboard') + self.assertRedirects(response, '/dashboard') + self.assertEquals(int(self.client.session['_auth_user_id']), user_w_map.id) # verify logging: self.assertEquals(len(audit_log_calls), 2) self._assert_shib_login_is_logged(audit_log_calls[0], remote_user) @@ -198,9 +190,8 @@ class ShibSPTest(CacheIsolationTestCase): # self.assertEquals(remote_user, args[1]) elif idp == "https://idp.stanford.edu/" and remote_user == 'womap@stanford.edu': self.assertIsNotNone(ExternalAuthMap.objects.get(user=user_wo_map)) - self.assertIsInstance(response, HttpResponseRedirect) - self.assertEqual(request.user, user_wo_map) - self.assertEqual(response['Location'], '/dashboard') + self.assertRedirects(response, '/dashboard') + self.assertEquals(int(self.client.session['_auth_user_id']), user_wo_map.id) # verify logging: self.assertEquals(len(audit_log_calls), 2) self._assert_shib_login_is_logged(audit_log_calls[0], remote_user) @@ -313,8 +304,7 @@ class ShibSPTest(CacheIsolationTestCase): Uses django test client for its session support """ # First we pop the registration form - client = DjangoTestClient() - response1 = client.get(path='/shib-login/', data={}, follow=False, **identity) + self.client.get(path='/shib-login/', data={}, follow=False, **identity) # Then we have the user answer the registration form # These are unicode because request.POST returns unicode postvars = {'email': u'post_email@stanford.edu', @@ -323,16 +313,10 @@ class ShibSPTest(CacheIsolationTestCase): 'name': u'post_náme', 'terms_of_service': u'true', 'honor_code': u'true'} - # use RequestFactory instead of TestClient here because we want access to request.user - request2 = self.request_factory.post('/create_account', data=postvars) - request2.session = client.session - request2.user = AnonymousUser() - mako_middleware_process_request(request2) with patch('student.views.AUDIT_LOG') as mock_audit_log: - _response2 = create_account(request2) + self.client.post('/create_account', data=postvars) - user = request2.user mail = identity.get('mail') # verify logging of login happening during account creation: @@ -355,6 +339,8 @@ class ShibSPTest(CacheIsolationTestCase): self.assertEquals(u'post_username', args[1]) self.assertEquals(u'test_user@stanford.edu', args[2].external_id) + user = User.objects.get(id=self.client.session['_auth_user_id']) + # check that the created user has the right email, either taken from shib or user input if mail: self.assertEqual(user.email, mail) @@ -375,10 +361,10 @@ class ShibSPTest(CacheIsolationTestCase): if sn_empty and given_name_empty: self.assertEqual(profile.name, postvars['name']) else: - self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name) + self.assertEqual(profile.name, self.client.session['ExternalAuthMap'].external_name) self.assertNotIn(u';', profile.name) else: - self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name) + self.assertEqual(profile.name, self.client.session['ExternalAuthMap'].external_name) self.assertEqual(profile.name, identity.get('displayName').decode('utf-8')) diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index aba61434fe..635b2af8f7 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -5,6 +5,7 @@ of the external_auth app. import copy import unittest +from contextlib import contextmanager from django.conf import settings from django.contrib.auth import SESSION_KEY from django.contrib.auth.models import AnonymousUser, User @@ -13,10 +14,9 @@ from django.core.urlresolvers import reverse from django.test.client import Client from django.test.client import RequestFactory from django.test.utils import override_settings -from edxmako.middleware import MakoMiddleware from external_auth.models import ExternalAuthMap import external_auth.views -from mock import Mock +from mock import Mock, patch from student.models import CourseEnrollment from student.roles import CourseStaffRole @@ -48,6 +48,7 @@ class SSLClientTest(ModuleStoreTestCase): USER_EMAIL = 'test_user_ssl@EDX.ORG' MOCK_URL = '/' + @contextmanager def _create_ssl_request(self, url): """Creates a basic request for SSL use.""" request = self.factory.get(url) @@ -56,9 +57,11 @@ class SSLClientTest(ModuleStoreTestCase): middleware = SessionMiddleware() middleware.process_request(request) request.session.save() - MakoMiddleware().process_request(request) - return request + with patch('edxmako.request_context.get_current_request', return_value=request): + yield request + + @contextmanager def _create_normal_request(self, url): """Creates sessioned request without SSL headers""" request = self.factory.get(url) @@ -66,8 +69,9 @@ class SSLClientTest(ModuleStoreTestCase): middleware = SessionMiddleware() middleware.process_request(request) request.session.save() - MakoMiddleware().process_request(request) - return request + + with patch('edxmako.request_context.get_current_request', return_value=request): + yield request def setUp(self): """Setup test case by adding primary user.""" @@ -82,8 +86,8 @@ class SSLClientTest(ModuleStoreTestCase): Validate that an SSL login creates an eamap user and redirects them to the signup page. """ - - response = external_auth.views.ssl_login(self._create_ssl_request('/')) + with self._create_ssl_request('/') as request: + response = external_auth.views.ssl_login(request) # Response should contain template for signup form, eamap should have user, and internal # auth should not have a user @@ -122,8 +126,8 @@ class SSLClientTest(ModuleStoreTestCase): Test IMMEDIATE_SIGNUP feature flag and ensure the user account is automatically created and the user is redirected to slash. """ - - external_auth.views.ssl_login(self._create_ssl_request('/')) + with self._create_ssl_request('/') as request: + external_auth.views.ssl_login(request) # Assert our user exists in both eamap and Users, and that we are logged in try: @@ -244,7 +248,9 @@ class SSLClientTest(ModuleStoreTestCase): and this should not fail. """ # Create account, break internal password, and activate account - external_auth.views.ssl_login(self._create_ssl_request('/')) + + with self._create_ssl_request('/') as request: + external_auth.views.ssl_login(request) user = User.objects.get(email=self.USER_EMAIL) user.set_password('not autogenerated') user.is_active = True @@ -262,12 +268,13 @@ class SSLClientTest(ModuleStoreTestCase): """Make sure no external auth happens without SSL enabled""" dec_mock = external_auth.views.ssl_login_shortcut(self.mock) - request = self._create_normal_request(self.MOCK_URL) - request.user = AnonymousUser() - # Call decorated mock function to make sure it passes - # the call through without hitting the external_auth functions and - # thereby creating an external auth map object. - dec_mock(request) + + with self._create_normal_request(self.MOCK_URL) as request: + request.user = AnonymousUser() + # Call decorated mock function to make sure it passes + # the call through without hitting the external_auth functions and + # thereby creating an external auth map object. + dec_mock(request) self.assertTrue(self.mock.called) self.assertEqual(0, len(ExternalAuthMap.objects.all())) @@ -278,23 +285,23 @@ class SSLClientTest(ModuleStoreTestCase): dec_mock = external_auth.views.ssl_login_shortcut(self.mock) # Test that anonymous without cert doesn't create authmap - request = self._create_normal_request(self.MOCK_URL) - dec_mock(request) + with self._create_normal_request(self.MOCK_URL) as request: + dec_mock(request) self.assertTrue(self.mock.called) self.assertEqual(0, len(ExternalAuthMap.objects.all())) # Test valid user self.mock.reset_mock() - request = self._create_ssl_request(self.MOCK_URL) - dec_mock(request) + with self._create_ssl_request(self.MOCK_URL) as request: + dec_mock(request) self.assertFalse(self.mock.called) self.assertEqual(1, len(ExternalAuthMap.objects.all())) # Test logged in user gets called self.mock.reset_mock() - request = self._create_ssl_request(self.MOCK_URL) - request.user = UserFactory() - dec_mock(request) + with self._create_ssl_request(self.MOCK_URL) as request: + request.user = UserFactory() + dec_mock(request) self.assertTrue(self.mock.called) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -306,8 +313,9 @@ class SSLClientTest(ModuleStoreTestCase): """ dec_mock = external_auth.views.ssl_login_shortcut(self.mock) - request = self._create_ssl_request(self.MOCK_URL) - dec_mock(request) + with self._create_ssl_request(self.MOCK_URL) as request: + dec_mock(request) + # Assert our user exists in both eamap and Users try: ExternalAuthMap.objects.get(external_id=self.USER_EMAIL) @@ -334,7 +342,8 @@ class SSLClientTest(ModuleStoreTestCase): display_name='Robot Super Course' ) - external_auth.views.ssl_login(self._create_ssl_request('/')) + with self._create_ssl_request('/') as request: + external_auth.views.ssl_login(request) user = User.objects.get(email=self.USER_EMAIL) CourseEnrollment.enroll(user, course.id) course_private_url = '/courses/MITx/999/Robot_Super_Course/courseware' @@ -364,7 +373,8 @@ class SSLClientTest(ModuleStoreTestCase): display_name='Robot Super Course' ) - external_auth.views.ssl_login(self._create_ssl_request('/')) + with self._create_ssl_request('/') as request: + external_auth.views.ssl_login(request) user = User.objects.get(email=self.USER_EMAIL) CourseEnrollment.enroll(user, course.id) diff --git a/common/djangoapps/request_cache/__init__.py b/common/djangoapps/request_cache/__init__.py index d38255bc59..c662ff62a7 100644 --- a/common/djangoapps/request_cache/__init__.py +++ b/common/djangoapps/request_cache/__init__.py @@ -9,6 +9,7 @@ import logging from urlparse import urlparse from celery.signals import task_postrun +import crum from django.conf import settings from django.test.client import RequestFactory @@ -42,8 +43,10 @@ def get_cache(name): def get_request(): """ Return the current request. + + Deprecated: Please use crum to retrieve current requests. """ - return middleware.RequestCache.get_current_request() + return crum.get_current_request() def get_request_or_stub(): @@ -56,7 +59,7 @@ def get_request_or_stub(): This is useful in cases where we need to pass in a request object but don't have an active request (for example, in test cases). """ - request = get_request() + request = crum.get_current_request() if request is None: log.warning( diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index ecb85633ef..197d637683 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -1,3 +1,9 @@ +""" +An implementation of a RequestCache. This cache is reset at the beginning +and end of every request. +""" + +import crum import threading @@ -8,7 +14,6 @@ class _RequestCache(threading.local): def __init__(self): super(_RequestCache, self).__init__() self.data = {} - self.request = None REQUEST_CACHE = _RequestCache() @@ -30,7 +35,7 @@ class RequestCache(object): """ This method is deprecated. Please use :func:`request_cache.get_request`. """ - return REQUEST_CACHE.request + return crum.get_current_request() @classmethod def clear_request_cache(cls): @@ -38,13 +43,18 @@ class RequestCache(object): Empty the request cache. """ REQUEST_CACHE.data = {} - REQUEST_CACHE.request = None def process_request(self, request): self.clear_request_cache() - REQUEST_CACHE.request = request return None def process_response(self, request, response): self.clear_request_cache() return response + + def process_exception(self, request, exception): # pylint: disable=unused-argument + """ + Clear the RequestCache after a failed request. + """ + self.clear_request_cache() + return None diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index fec922ab44..a712274e9e 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -16,7 +16,6 @@ import mock from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from lang_pref import LANGUAGE_KEY from notification_prefs import NOTIFICATION_PREF_KEY -from edxmako.tests import mako_middleware_process_request from external_auth.models import ExternalAuthMap import student from student.models import UserAttribute @@ -231,9 +230,9 @@ class TestCreateAccount(TestCase): request.session['ExternalAuthMap'] = extauth request.user = AnonymousUser() - mako_middleware_process_request(request) - with mock.patch('django.contrib.auth.models.User.email_user') as mock_send_mail: - student.views.create_account(request) + with mock.patch('edxmako.request_context.get_current_request', return_value=request): + with mock.patch('django.contrib.auth.models.User.email_user') as mock_send_mail: + student.views.create_account(request) # check that send_mail is called if bypass_activation_email: diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index c1edad9271..3c44111608 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -18,7 +18,6 @@ from mock import Mock, patch from django.http import HttpResponse from django.conf import settings from edxmako.shortcuts import render_to_string -from edxmako.tests import mako_middleware_process_request from util.request import safe_get_host from util.testing import EventTestMixin from openedx.core.djangoapps.theming.test_util import with_is_edx_domain @@ -173,9 +172,9 @@ class ReactivationEmailTests(EmailTestMixin, TestCase): request.META['HTTP_HOST'] = "aGenericValidHostName" self.append_allowed_hosts("aGenericValidHostName") - mako_middleware_process_request(request) - body = render_to_string('emails/activation_email.txt', context) - host = safe_get_host(request) + with patch('edxmako.request_context.get_current_request', return_value=request): + body = render_to_string('emails/activation_email.txt', context) + host = safe_get_host(request) self.assertIn(host, body) @@ -368,9 +367,9 @@ class EmailChangeConfirmationTests(EmailTestMixin, TransactionTestCase): request.META['HTTP_HOST'] = "aGenericValidHostName" self.append_allowed_hosts("aGenericValidHostName") - mako_middleware_process_request(request) - body = render_to_string('emails/confirm_email_change.txt', context) - url = safe_get_host(request) + with patch('edxmako.request_context.get_current_request', return_value=request): + body = render_to_string('emails/confirm_email_change.txt', context) + url = safe_get_host(request) self.assertIn(url, body) diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index c606478b25..c7c7d8a30f 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -11,7 +11,6 @@ from importlib import import_module from django.test.utils import override_settings from django.conf import settings from mock import patch -from edxmako.tests import mako_middleware_process_request from external_auth.models import ExternalAuthMap from student.views import create_account @@ -262,8 +261,8 @@ class TestPasswordPolicy(TestCase): request.session['ExternalAuthMap'] = extauth request.user = AnonymousUser() - mako_middleware_process_request(request) - response = create_account(request) + with patch('edxmako.request_context.get_current_request', return_value=request): + response = create_account(request) self.assertEqual(response.status_code, 200) obj = json.loads(response.content) self.assertTrue(obj['success']) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index ee879390b4..fa8fa991f0 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -5,6 +5,7 @@ import unittest import json import mock +from contextlib import contextmanager from django import test from django.contrib import auth from django.contrib.auth import models as auth_models @@ -17,7 +18,6 @@ from social import actions, exceptions from social.apps.django_app import utils as social_utils from social.apps.django_app import views as social_views -from edxmako.tests import mako_middleware_process_request from lms.djangoapps.commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY from student import models as student_models from student import views as student_views @@ -526,6 +526,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase): return request, strategy + @contextmanager + def _patch_edxmako_current_request(self, request): + """Make ``request`` be the current request for edxmako template rendering.""" + + with mock.patch('edxmako.request_context.get_current_request', return_value=request): + yield + def get_user_by_email(self, strategy, email): """Gets a user by email, using the given strategy.""" return strategy.storage.user.user_model().objects.get(email=email) @@ -579,7 +586,6 @@ class IntegrationTest(testutil.TestCase, test.TestCase): pipeline.get_login_url(self.provider.provider_id, pipeline.AUTH_ENTRY_LOGIN)) actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access - mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access @@ -627,10 +633,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): pipeline.get_login_url(self.provider.provider_id, pipeline.AUTH_ENTRY_LOGIN)) actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access - mako_middleware_process_request(strategy.request) - student_views.signin_user(strategy.request) - student_views.login_user(strategy.request) - actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access + with self._patch_edxmako_current_request(strategy.request): + student_views.signin_user(strategy.request) + student_views.login_user(strategy.request) + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access # First we expect that we're in the linked state, with a backend entry. self.assert_account_settings_context_looks_correct(account_settings_context(request), user, linked=True) @@ -686,10 +692,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.client.get(pipeline.get_login_url(self.provider.provider_id, pipeline.AUTH_ENTRY_LOGIN)) actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access - mako_middleware_process_request(strategy.request) - student_views.signin_user(strategy.request) - student_views.login_user(strategy.request) - actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access + with self._patch_edxmako_current_request(strategy.request): + student_views.signin_user(strategy.request) + student_views.login_user(strategy.request) + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access # Monkey-patch storage for messaging; pylint: disable=protected-access request._messages = fallback.FallbackStorage(request) @@ -727,10 +733,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # pylint: disable=protected-access self.assert_redirect_to_login_looks_correct(actions.do_complete(request.backend, social_views._do_login)) - mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we # fire off the view that displays the login form and posts it via JS. - self.assert_login_response_in_pipeline_looks_correct(student_views.signin_user(strategy.request)) + with self._patch_edxmako_current_request(strategy.request): + self.assert_login_response_in_pipeline_looks_correct(student_views.signin_user(strategy.request)) # Next, we invoke the view that handles the POST, and expect it # redirects to /auth/complete. In the browser ajax handlers will @@ -760,8 +766,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): user.is_active = False user.save() - mako_middleware_process_request(strategy.request) - self.assert_json_failure_response_is_inactive_account(student_views.login_user(strategy.request)) + with self._patch_edxmako_current_request(strategy.request): + self.assert_json_failure_response_is_inactive_account(student_views.login_user(strategy.request)) def test_signin_fails_if_no_account_associated(self): _, strategy = self.get_request_and_strategy( @@ -806,11 +812,11 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # pylint: disable=protected-access self.assert_redirect_to_register_looks_correct(actions.do_complete(request.backend, social_views._do_login)) - mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we # fire off the view that displays the registration form. - self.assert_register_response_in_pipeline_looks_correct( - student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) + with self._patch_edxmako_current_request(request): + self.assert_register_response_in_pipeline_looks_correct( + student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) # Next, we invoke the view that handles the POST. Not all providers # supply email. Manually add it as the user would have to; this @@ -828,7 +834,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # ...but when we invoke create_account the existing edX view will make # it, but not social auths. The pipeline creates those later. - self.assert_json_success_response_looks_correct(student_views.create_account(strategy.request)) + with self._patch_edxmako_current_request(strategy.request): + self.assert_json_success_response_looks_correct(student_views.create_account(strategy.request)) # We've overridden the user's password, so authenticate() with the old # value won't work: created_user = self.get_user_by_email(strategy, email) @@ -876,12 +883,14 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # pylint: disable=protected-access self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) - mako_middleware_process_request(strategy.request) - self.assert_register_response_in_pipeline_looks_correct( - student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) - strategy.request.POST = self.get_registration_post_vars() - # Create twice: once successfully, and once causing a collision. - student_views.create_account(strategy.request) + with self._patch_edxmako_current_request(request): + self.assert_register_response_in_pipeline_looks_correct( + student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) + + with self._patch_edxmako_current_request(strategy.request): + strategy.request.POST = self.get_registration_post_vars() + # Create twice: once successfully, and once causing a collision. + student_views.create_account(strategy.request) self.assert_json_failure_response_is_username_collision(student_views.create_account(strategy.request)) def test_pipeline_raises_auth_entry_error_if_auth_entry_invalid(self): diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index af789bf88e..f99fd22de5 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -16,7 +16,6 @@ from nose.plugins.attrib import attr from edxmako.shortcuts import render_to_response from branding.views import index -from edxmako.tests import mako_middleware_process_request import student.views from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -63,11 +62,9 @@ class AnonymousIndexPageTest(ModuleStoreTestCase): anonymous and start dates are being checked. It replaces a previous test as it solves the issue in a different way """ - request = self.factory.get('/') - request.user = AnonymousUser() - - mako_middleware_process_request(request) - student.views.index(request) + self.client.logout() + response = self.client.get(reverse('root')) + self.assertEqual(response.status_code, 200) @override_settings(FEATURES=FEATURES_WITH_STARTDATE) def test_anon_user_with_startdate_index(self): diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 82f8f3467f..11cceae8f0 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -14,7 +14,6 @@ from django.conf import settings from django.core.cache import caches from django.test.client import RequestFactory from django.test.utils import override_settings -from edxmako.middleware import MakoMiddleware from nose.plugins.attrib import attr from pytz import UTC from request_cache.middleware import RequestCache @@ -64,11 +63,13 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, self.student = UserFactory.create() self.request = self.request_factory.get("foo") self.request.user = self.student + + patcher = mock.patch('edxmako.request_context.get_current_request', return_value=self.request) + patcher.start() + self.addCleanup(patcher.stop) self.course = None self.ccx = None - MakoMiddleware().process_request(self.request) - def setup_course(self, size, enable_ccx, view_as_ccx): """ Build a gradable course where each node has `size` children. diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 5bb061f831..b55188c6fa 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -30,7 +30,6 @@ from courseware.tests.factories import ( ) import courseware.views.views as views from courseware.tests.helpers import LoginEnrollmentTestCase -from edxmako.tests import mako_middleware_process_request from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment from student.roles import CourseCcxCoachRole @@ -137,21 +136,13 @@ class CoachAccessTestCaseCCX(SharedModuleStoreTestCase, LoginEnrollmentTestCase) CourseEnrollment.enroll(student, ccx_locator) # Test for access of a coach - request = self.request_factory.get(reverse('about_course', args=[unicode(ccx_locator)])) - request.user = self.coach - mako_middleware_process_request(request) - resp = views.progress(request, course_id=unicode(ccx_locator), student_id=student.id) + resp = self.client.get(reverse('student_progress', args=[unicode(ccx_locator), student.id])) self.assertEqual(resp.status_code, 200) # Assert access of a student - request = self.request_factory.get(reverse('about_course', args=[unicode(ccx_locator)])) - request.user = student - mako_middleware_process_request(request) - - with self.assertRaises(Http404) as context: - views.progress(request, course_id=unicode(ccx_locator), student_id=self.coach.id) - - self.assertIsNotNone(context.exception) + self.client.login(username=student.username, password='test') + resp = self.client.get(reverse('student_progress', args=[unicode(ccx_locator), self.coach.id])) + self.assertEqual(resp.status_code, 404) @attr('shard_1') diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index b944ea9ee2..b1a279d770 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -43,7 +43,6 @@ from courseware.tests.factories import StudentModuleFactory, GlobalStaffFactory from courseware.url_helpers import get_redirect_url from courseware.user_state_client import DjangoXBlockUserStateClient from courseware.views.index import render_accordion, CoursewareIndex -from edxmako.tests import mako_middleware_process_request from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration @@ -242,15 +241,13 @@ class ViewsTestCase(ModuleStoreTestCase): self.enrollment = CourseEnrollment.enroll(self.user, self.course_key) self.enrollment.created = self.date self.enrollment.save() - self.request_factory = RequestFactory() chapter = 'Overview' self.chapter_url = '%s/%s/%s' % ('/courses', self.course_key, chapter) self.org = u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" self.org_html = "

'+Stark/Industries+'

" - self.request = self.request_factory.get("foo") - self.request.user = self.user + self.assertTrue(self.client.login(username=self.user.username, password=self.password)) # refresh the course from the modulestore so that it has children self.course = modulestore().get_course(self.course.id) @@ -290,7 +287,6 @@ class ViewsTestCase(ModuleStoreTestCase): Verifies the response when the courseware index page is accessed with the given chapter and section names. """ - self.client.login(username=self.user.username, password=self.password) url = reverse( 'courseware_section', kwargs={ @@ -304,7 +300,6 @@ class ViewsTestCase(ModuleStoreTestCase): return response def test_index_no_visible_section_in_chapter(self): - self.client.login(username=self.user.username, password=self.password) # reload the chapter from the store so its children information is updated self.chapter = self.store.get_item(self.chapter.location) @@ -328,7 +323,7 @@ class ViewsTestCase(ModuleStoreTestCase): Create global staff user and log them in """ self.global_staff = GlobalStaffFactory.create() # pylint: disable=attribute-defined-outside-init - self.client.login(username=self.global_staff.username, password='test') + self.assertTrue(self.client.login(username=self.global_staff.username, password='test')) def _create_url_for_enroll_staff(self): """ @@ -428,25 +423,22 @@ class ViewsTestCase(ModuleStoreTestCase): in_cart_span = '' # don't mock this course due to shopping cart existence checking course = CourseFactory.create(org="new", number="unenrolled", display_name="course") - request = self.request_factory.get(reverse('about_course', args=[unicode(course.id)])) - request.user = AnonymousUser() - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) - response = views.course_about(request, unicode(course.id)) + self.client.logout() + response = self.client.get(reverse('about_course', args=[unicode(course.id)])) self.assertEqual(response.status_code, 200) self.assertNotIn(in_cart_span, response.content) # authenticated user with nothing in cart - request.user = self.user - response = views.course_about(request, unicode(course.id)) + self.assertTrue(self.client.login(username=self.user.username, password=self.password)) + response = self.client.get(reverse('about_course', args=[unicode(course.id)])) self.assertEqual(response.status_code, 200) self.assertNotIn(in_cart_span, response.content) # now add the course to the cart cart = shoppingcart.models.Order.get_cart_for_user(self.user) shoppingcart.models.PaidCourseRegistration.add_to_order(cart, course.id) - response = views.course_about(request, unicode(course.id)) + response = self.client.get(reverse('about_course', args=[unicode(course.id)])) self.assertEqual(response.status_code, 200) self.assertIn(in_cart_span, response.content) @@ -468,19 +460,18 @@ class ViewsTestCase(ModuleStoreTestCase): course = CourseFactory.create() CourseModeFactory(mode_slug=CourseMode.PROFESSIONAL, course_id=course.id, sku=sku, min_price=1) - request = self.request_factory.get(reverse('about_course', args=[unicode(course.id)])) - request.user = AnonymousUser() if is_anonymous else self.user - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) - - # Generate the course about page content - response = views.course_about(request, unicode(course.id)) + if is_anonymous: + self.client.logout() + else: + self.assertTrue(self.client.login(username=self.user.username, password=self.password)) # Construct the link according the following scenarios and verify its presence in the response: # (1) shopping cart is enabled and the user is not logged in # (2) shopping cart is enabled and the user is logged in href = ''.format(uri_stem=checkout_page, sku=sku) + + # Generate the course about page content + response = self.client.get(reverse('about_course', args=[unicode(course.id)])) self.assertEqual(response.status_code, 200) self.assertIn(href, response.content) @@ -489,7 +480,6 @@ class ViewsTestCase(ModuleStoreTestCase): if not is_anonymous: self.assert_enrollment_link_present(is_anonymous=is_anonymous) else: - request = self.request_factory.get("foo") self.assertEqual(EcommerceService().is_enabled(AnonymousUser()), False) @ddt.data(True, False) @@ -504,7 +494,6 @@ class ViewsTestCase(ModuleStoreTestCase): if not is_anonymous: self.assert_enrollment_link_present(is_anonymous=is_anonymous) else: - request = self.request_factory.get("foo") self.assertEqual(EcommerceService().is_enabled(AnonymousUser()), False) def test_user_groups(self): @@ -553,7 +542,7 @@ class ViewsTestCase(ModuleStoreTestCase): self.section.location.name, 'f' ]) - self.client.login(username=self.user.username, password=self.password) + self.assertTrue(self.client.login(username=self.user.username, password=self.password)) response = self.client.get(request_url) self.assertEqual(response.status_code, 404) @@ -566,7 +555,7 @@ class ViewsTestCase(ModuleStoreTestCase): self.section.location.name, '1' ] - self.client.login(username=self.user.username, password=self.password) + self.assertTrue(self.client.login(username=self.user.username, password=self.password)) for idx, val in enumerate(url_parts): url_parts_copy = url_parts[:] url_parts_copy[idx] = val + u'χ' @@ -595,9 +584,8 @@ class ViewsTestCase(ModuleStoreTestCase): def test_jump_to_invalid(self): # TODO add a test for invalid location # TODO add a test for no data * - request = self.request_factory.get(self.chapter_url) - self.assertRaisesRegexp(Http404, 'Invalid course_key or usage_key', views.jump_to, - request, 'bar', ()) + response = self.client.get(reverse('jump_to', args=['foo/bar/baz', 'baz'])) + self.assertEquals(response.status_code, 404) @unittest.skip def test_no_end_on_about_page(self): @@ -623,13 +611,7 @@ class ViewsTestCase(ModuleStoreTestCase): If `expected_end_text` is None, verifies that the about page *does not* contain the text "Classes End". """ - request = self.request_factory.get("foo") - request.user = self.user - - # TODO: Remove the dependency on MakoMiddleware (by making the views explicitly supply a RequestContext) - mako_middleware_process_request(request) - - result = views.course_about(request, course_id) + result = self.client.get(reverse('about_course', args=[unicode(course_id)])) if expected_end_text is not None: self.assertContains(result, "Classes End") self.assertContains(result, expected_end_text) @@ -640,7 +622,7 @@ class ViewsTestCase(ModuleStoreTestCase): # log into a staff account admin = AdminFactory() - self.client.login(username=admin.username, password='test') + self.assertTrue(self.client.login(username=admin.username, password='test')) url = reverse('submission_history', kwargs={ 'course_id': unicode(self.course_key), @@ -655,7 +637,7 @@ class ViewsTestCase(ModuleStoreTestCase): # log into a staff account admin = AdminFactory() - self.client.login(username=admin.username, password='test') + self.assertTrue(self.client.login(username=admin.username, password='test')) # try it with an existing user and a malicious location url = reverse('submission_history', kwargs={ @@ -679,7 +661,7 @@ class ViewsTestCase(ModuleStoreTestCase): # log into a staff account admin = AdminFactory.create() - self.client.login(username=admin.username, password='test') + self.assertTrue(self.client.login(username=admin.username, password='test')) usage_key = self.course_key.make_usage_key('problem', 'test-history') state_client = DjangoXBlockUserStateClient(admin) @@ -733,7 +715,7 @@ class ViewsTestCase(ModuleStoreTestCase): course_key = course.id client = Client() admin = AdminFactory.create() - client.login(username=admin.username, password='test') + self.assertTrue(client.login(username=admin.username, password='test')) state_client = DjangoXBlockUserStateClient(admin) usage_key = course_key.make_usage_key('problem', 'test-history') state_client.set( @@ -766,7 +748,6 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertNotContains(response, checkbox_html, html=True) def test_financial_assistance_page(self): - self.client.login(username=self.user.username, password=self.password) url = reverse('financial_assistance') response = self.client.get(url) # This is a static page, so just assert that it is returned correctly @@ -796,7 +777,6 @@ class ViewsTestCase(ModuleStoreTestCase): ) CourseEnrollmentFactory(course_id=course, user=self.user, mode=mode) - self.client.login(username=self.user.username, password=self.password) url = reverse('financial_assistance_form') response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -815,7 +795,6 @@ class ViewsTestCase(ModuleStoreTestCase): def _submit_financial_assistance_form(self, data): """Submit a financial assistance request.""" - self.client.login(username=self.user.username, password=self.password) url = reverse('submit_financial_assistance_request') return self.client.post(url, json.dumps(data), content_type='application/json') @@ -898,53 +877,38 @@ class ViewsTestCase(ModuleStoreTestCase): reverse('financial_assistance_form'), reverse('submit_financial_assistance_request') ): + self.client.logout() response = self.client.get(url) self.assertRedirects(response, reverse('signin_user') + '?next=' + url) def test_bypass_course_info(self): course_id = unicode(self.course_key) - request = self.request_factory.get( - reverse('info', args=[course_id]) - ) - - # Middleware is not supported by the request factory. Simulate a - # logged-in user by setting request.user manually. - request.user = self.user - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) self.assertFalse(self.course.bypass_home) - self.assertIsNone(request.META.get('HTTP_REFERER')) # pylint: disable=no-member - response = views.course_info(request, course_id) + response = self.client.get(reverse('info', args=[course_id])) self.assertEqual(response.status_code, 200) - request.META['HTTP_REFERER'] = reverse('dashboard') # pylint: disable=no-member - response = views.course_info(request, course_id) + response = self.client.get(reverse('info', args=[course_id]), HTTP_REFERER=reverse('dashboard')) self.assertEqual(response.status_code, 200) self.course.bypass_home = True self.store.update_item(self.course, self.user.id) # pylint: disable=no-member self.assertTrue(self.course.bypass_home) - response = views.course_info(request, course_id) + response = self.client.get(reverse('info', args=[course_id]), HTTP_REFERER=reverse('dashboard')) - # assertRedirects would be great here, but it forces redirections to be absolute URLs. - self.assertEqual(response.status_code, 302) - self.assertEqual( - response.url, - reverse('courseware', args=[course_id]) - ) + self.assertRedirects(response, reverse('courseware', args=[course_id]), fetch_redirect_response=False) - request.META['HTTP_REFERER'] = 'foo' # pylint: disable=no-member - response = views.course_info(request, course_id) + response = self.client.get(reverse('info', args=[course_id]), HTTP_REFERER='foo') self.assertEqual(response.status_code, 200) def test_accordion(self): + request = RequestFactory().get('foo') + request.user = self.user table_of_contents = toc_for_course( - self.request.user, - self.request, + request.user, + request, self.course, unicode(self.course.get_children()[0].scope_ids.usage_id), None, @@ -952,7 +916,7 @@ class ViewsTestCase(ModuleStoreTestCase): ) # removes newlines and whitespace from the returned view string - view = ''.join(render_accordion(self.request, self.course, table_of_contents['chapters']).split()) + view = ''.join(render_accordion(request, self.course, table_of_contents['chapters']).split()) # the course id unicode is re-encoded here because the quote function does not accept unicode course_id = quote(unicode(self.course.id).encode("utf-8")) @@ -978,7 +942,7 @@ class BaseDueDateTests(ModuleStoreTestCase): """ __test__ = False - def get_text(self, course): + def get_response(self, course): """Return the rendered text for the page to be verified""" raise NotImplementedError @@ -1005,10 +969,8 @@ class BaseDueDateTests(ModuleStoreTestCase): def setUp(self): super(BaseDueDateTests, self).setUp() - self.request_factory = RequestFactory() self.user = UserFactory.create() - self.request = self.request_factory.get("foo") - self.request.user = self.user + self.assertTrue(self.client.login(username=self.user.username, password='test')) self.time_with_tz = "due Sep 18, 2013 at 11:30 UTC" self.time_without_tz = "due Sep 18, 2013 at 11:30" @@ -1019,50 +981,50 @@ class BaseDueDateTests(ModuleStoreTestCase): # in course_module's init method, the date_display_format will be set accordingly to # remove the timezone. course = self.set_up_course(due_date_display_format=None, show_timezone=False) - text = self.get_text(course) - self.assertIn(self.time_without_tz, text) - self.assertNotIn(self.time_with_tz, text) + response = self.get_response(course) + self.assertContains(response, self.time_without_tz) + self.assertNotContains(response, self.time_with_tz) # Test that show_timezone has been cleared (which means you get the default value of True). self.assertTrue(course.show_timezone) def test_defaults(self): course = self.set_up_course() - text = self.get_text(course) - self.assertIn(self.time_with_tz, text) + response = self.get_response(course) + self.assertContains(response, self.time_with_tz) def test_format_none(self): # Same for setting the due date to None course = self.set_up_course(due_date_display_format=None) - text = self.get_text(course) - self.assertIn(self.time_with_tz, text) + response = self.get_response(course) + self.assertContains(response, self.time_with_tz) def test_format_plain_text(self): # plain text due date course = self.set_up_course(due_date_display_format="foobar") - text = self.get_text(course) - self.assertNotIn(self.time_with_tz, text) - self.assertIn("due foobar", text) + response = self.get_response(course) + self.assertNotContains(response, self.time_with_tz) + self.assertContains(response, "due foobar") def test_format_date(self): # due date with no time course = self.set_up_course(due_date_display_format=u"%b %d %y") - text = self.get_text(course) - self.assertNotIn(self.time_with_tz, text) - self.assertIn("due Sep 18 13", text) + response = self.get_response(course) + self.assertNotContains(response, self.time_with_tz) + self.assertContains(response, "due Sep 18 13") def test_format_hidden(self): # hide due date completely course = self.set_up_course(due_date_display_format=u"") - text = self.get_text(course) - self.assertNotIn("due ", text) + response = self.get_response(course) + self.assertNotContains(response, "due ") def test_format_invalid(self): # improperly formatted due_date_display_format falls through to default # (value of show_timezone does not matter-- setting to False to make that clear). course = self.set_up_course(due_date_display_format=u"%%%", show_timezone=False) - text = self.get_text(course) - self.assertNotIn("%%%", text) - self.assertIn(self.time_with_tz, text) + response = self.get_response(course) + self.assertNotContains(response, "%%%") + self.assertContains(response, self.time_with_tz) class TestProgressDueDate(BaseDueDateTests): @@ -1071,12 +1033,9 @@ class TestProgressDueDate(BaseDueDateTests): """ __test__ = True - def get_text(self, course): + def get_response(self, course): """ Returns the HTML for the progress page """ - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(self.request) - return views.progress(self.request, course_id=unicode(course.id), student_id=self.user.id).content + return self.client.get(reverse('progress', args=[unicode(course.id)])) class TestAccordionDueDate(BaseDueDateTests): @@ -1085,12 +1044,9 @@ class TestAccordionDueDate(BaseDueDateTests): """ __test__ = True - def get_text(self, course): + def get_response(self, course): """ Returns the HTML for the accordion """ - table_of_contents = toc_for_course( - self.request.user, self.request, course, unicode(course.get_children()[0].scope_ids.usage_id), None, None - ) - return render_accordion(self.request, course, table_of_contents['chapters']) + return self.client.get(reverse('courseware', args=[unicode(course.id)]), follow=True) @attr('shard_1') @@ -1102,13 +1058,7 @@ class StartDateTests(ModuleStoreTestCase): def setUp(self): super(StartDateTests, self).setUp() - self.request_factory = RequestFactory() self.user = UserFactory.create() - self.request = self.request_factory.get("foo") - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(self.request) - self.request.user = self.user def set_up_course(self): """ @@ -1120,12 +1070,11 @@ class StartDateTests(ModuleStoreTestCase): course = modulestore().get_course(course.id) return course - def get_about_text(self, course_key): + def get_about_response(self, course_key): """ Get the text of the /about page for the course. """ - text = views.course_about(self.request, unicode(course_key)).content - return text + return self.client.get(reverse('about_course', args=[unicode(course_key)])) @patch('util.date_utils.pgettext', fake_pgettext(translations={ ("abbreviated month name", "Sep"): "SEPTEMBER", @@ -1135,9 +1084,9 @@ class StartDateTests(ModuleStoreTestCase): })) def test_format_localized_in_studio_course(self): course = self.set_up_course() - text = self.get_about_text(course.id) + response = self.get_about_response(course.id) # The start date is set in the set_up_course function above. - self.assertIn("2013-SEPTEMBER-16", text) + self.assertContains(response, "2013-SEPTEMBER-16") @patch('util.date_utils.pgettext', fake_pgettext(translations={ ("abbreviated month name", "Jul"): "JULY", @@ -1147,9 +1096,9 @@ class StartDateTests(ModuleStoreTestCase): })) @unittest.skip def test_format_localized_in_xml_course(self): - text = self.get_about_text(SlashSeparatedCourseKey('edX', 'toy', 'TT_2012_Fall')) + response = self.get_about_response(SlashSeparatedCourseKey('edX', 'toy', 'TT_2012_Fall')) # The start date is set in common/test/data/two_toys/policies/TT_2012_Fall/policy.json - self.assertIn("2015-JULY-17", text) + self.assertContains(response, "2015-JULY-17") @attr('shard_1') @@ -1163,13 +1112,8 @@ class ProgressPageTests(ModuleStoreTestCase): def setUp(self): super(ProgressPageTests, self).setUp() - self.request_factory = RequestFactory() self.user = UserFactory.create() - self.request = self.request_factory.get("foo") - self.request.user = self.user - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(self.request) + self.assertTrue(self.client.login(username=self.user.username, password='test')) self.setup_course() @@ -1193,7 +1137,9 @@ class ProgressPageTests(ModuleStoreTestCase): """ Test that XSS attack is prevented """ - resp = views.progress(self.request, course_id=unicode(self.course.id), student_id=self.user.id) + resp = self.client.get( + reverse('student_progress', args=[unicode(self.course.id), self.user.id]) + ) self.assertEqual(resp.status_code, 200) # Test that malicious code does not appear in html self.assertNotIn(malicious_code, resp.content) @@ -1201,7 +1147,9 @@ class ProgressPageTests(ModuleStoreTestCase): def test_pure_ungraded_xblock(self): ItemFactory.create(category='acid', parent_location=self.vertical.location) - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertEqual(resp.status_code, 200) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) @@ -1212,7 +1160,9 @@ class ProgressPageTests(ModuleStoreTestCase): """ # Create new course with respect to 'default_store' + # Enroll student into course self.course = CourseFactory.create(default_store=default_store) + CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=CourseMode.HONOR) # Invalid Student Ids (Integer and Non-int) invalid_student_ids = [ @@ -1220,16 +1170,15 @@ class ProgressPageTests(ModuleStoreTestCase): 'azU3N_8$', ] for invalid_id in invalid_student_ids: - self.assertRaises( - Http404, views.progress, - self.request, - course_id=unicode(self.course.id), - student_id=invalid_id - ) - # Enroll student into course - CourseEnrollment.enroll(self.user, self.course.id) - resp = views.progress(self.request, course_id=unicode(self.course.id), student_id=self.user.id) + resp = self.client.get( + reverse('student_progress', args=[unicode(self.course.id), invalid_id]) + ) + self.assertEquals(resp.status_code, 404) + + resp = self.client.get( + reverse('student_progress', args=[unicode(self.course.id), self.user.id]) + ) # Assert that valid 'student_id' returns 200 status self.assertEqual(resp.status_code, 200) @@ -1244,7 +1193,8 @@ class ProgressPageTests(ModuleStoreTestCase): # Create a new course, a user which will not be enrolled in course, admin user for staff access course = CourseFactory.create(default_store=default_store) not_enrolled_user = UserFactory.create() - self.request.user = AdminFactory.create() + admin = AdminFactory.create() + self.assertTrue(self.client.login(username=admin.username, password='test')) # Create and enable Credit course CreditCourse.objects.create(course_key=course.id, enabled=True) @@ -1265,25 +1215,38 @@ class ProgressPageTests(ModuleStoreTestCase): # Add a single credit requirement (final grade) set_credit_requirements(course.id, requirements) - resp = views.progress(self.request, course_id=unicode(course.id), student_id=not_enrolled_user.id) + resp = self.client.get( + reverse('student_progress', args=[unicode(course.id), not_enrolled_user.id]) + ) self.assertEqual(resp.status_code, 200) def test_non_ascii_grade_cutoffs(self): - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertEqual(resp.status_code, 200) def test_generate_cert_config(self): - resp = views.progress(self.request, course_id=unicode(self.course.id)) + + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertNotContains(resp, 'Request Certificate') # Enable the feature, but do not enable it for this course CertificateGenerationConfiguration(enabled=True).save() - resp = views.progress(self.request, course_id=unicode(self.course.id)) + + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertNotContains(resp, 'Request Certificate') # Enable certificate generation for this course certs_api.set_cert_generation_enabled(self.course.id, True) - resp = views.progress(self.request, course_id=unicode(self.course.id)) + + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertNotContains(resp, 'Request Certificate') @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) @@ -1326,7 +1289,9 @@ class ProgressPageTests(ModuleStoreTestCase): self.course.save() self.store.update_item(self.course, self.user.id) - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertContains(resp, u"View Certificate") self.assertContains(resp, u"You can keep working for a higher grade") @@ -1337,7 +1302,9 @@ class ProgressPageTests(ModuleStoreTestCase): certificates[0]['is_active'] = False self.store.update_item(self.course, self.user.id) - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertNotContains(resp, u"View Your Certificate") self.assertNotContains(resp, u"You can now view your certificate") self.assertContains(resp, u"We're creating your certificate.") @@ -1364,11 +1331,13 @@ class ProgressPageTests(ModuleStoreTestCase): # Enable certificate generation for this course certs_api.set_cert_generation_enabled(self.course.id, True) - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertContains(resp, u"Download Your Certificate") @ddt.data( - *itertools.product(((41, 4, True), (41, 4, False)), (True, False)) + *itertools.product(((55, 4, True), (55, 4, False)), (True, False)) ) @ddt.unpack def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled): @@ -1376,7 +1345,9 @@ class ProgressPageTests(ModuleStoreTestCase): SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) with self.assertNumQueries(sql_calls), check_mongo_calls(mongo_calls): - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) self.assertEqual(resp.status_code, 200) @patch('courseware.grades.grade', Mock(return_value={ @@ -1405,7 +1376,9 @@ class ProgressPageTests(ModuleStoreTestCase): 'lms.djangoapps.verify_student.models.SoftwareSecurePhotoVerification.user_is_verified' ) as user_verify: user_verify.return_value = user_verified - resp = views.progress(self.request, course_id=unicode(self.course.id)) + resp = self.client.get( + reverse('progress', args=[unicode(self.course.id)]) + ) cert_button_hidden = course_mode is CourseMode.AUDIT or \ course_mode in CourseMode.VERIFIED_MODES and not user_verified @@ -1503,8 +1476,7 @@ class GenerateUserCertTests(ModuleStoreTestCase): grade_cutoffs={'cutoff': 0.75, 'Pass': 0.5} ) self.enrollment = CourseEnrollment.enroll(self.student, self.course.id, mode='honor') - self.request = RequestFactory() - self.client.login(username=self.student, password='123456') + self.assertTrue(self.client.login(username=self.student, password='123456')) self.url = reverse('generate_user_cert', kwargs={'course_id': unicode(self.course.id)}) def test_user_with_out_passing_grades(self): @@ -1671,7 +1643,8 @@ class TestIndexView(ModuleStoreTestCase): CourseEnrollmentFactory(user=user, course_id=course.id) - request = RequestFactory().get( + self.assertTrue(self.client.login(username=user.username, password='test')) + response = self.client.get( reverse( 'courseware_section', kwargs={ @@ -1681,15 +1654,8 @@ class TestIndexView(ModuleStoreTestCase): } ) ) - request.user = user - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) # Trigger the assertions embedded in the ViewCheckerBlocks - response = CoursewareIndex.as_view()( - request, unicode(course.id), chapter=chapter.url_name, section=section.url_name - ) self.assertEquals(response.content.count("ViewCheckerPassed"), 3) @XBlock.register_temp_plugin(ActivateIDCheckerBlock, 'id_checker') @@ -1704,7 +1670,8 @@ class TestIndexView(ModuleStoreTestCase): CourseEnrollmentFactory(user=user, course_id=course.id) - request = RequestFactory().get( + self.assertTrue(self.client.login(username=user.username, password='test')) + response = self.client.get( reverse( 'courseware_section', kwargs={ @@ -1714,14 +1681,6 @@ class TestIndexView(ModuleStoreTestCase): } ) + '?activate_block_id=test_block_id' ) - request.user = user - - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) - - response = CoursewareIndex.as_view()( - request, unicode(course.id), chapter=chapter.url_name, section=section.url_name - ) self.assertIn("Activate Block ID: test_block_id", response.content) @@ -1818,7 +1777,8 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): """ Test index view with a gated sequential raises Http404 """ - request = RequestFactory().get( + self.assertTrue(self.client.login(username=self.user.username, password='test')) + response = self.client.get( reverse( 'courseware_section', kwargs={ @@ -1828,18 +1788,8 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): } ) ) - request.user = self.user - # Set up the edxmako middleware for this request to create the RequestContext - mako_middleware_process_request(request) - - with self.assertRaises(Http404): - CoursewareIndex.as_view()( - request, - unicode(self.course.id), - chapter=self.chapter.url_name, - section=self.gated_seq.url_name - ) + self.assertEquals(response.status_code, 404) class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 5a78c268e1..eef21b84f7 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -658,7 +658,6 @@ def course_about(request, course_id): @ensure_valid_course_key def progress(request, course_id, student_id=None): """ Display the progress page. """ - course_key = CourseKey.from_string(course_id) with modulestore().bulk_operations(course_key): @@ -673,6 +672,14 @@ def _progress(request, course_key, student_id): Course staff are allowed to see the progress of students in their class. """ + + if student_id is not None: + try: + student_id = int(student_id) + # Check for ValueError if 'student_id' cannot be converted to integer. + except ValueError: + raise Http404 + course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True) # check to see if there is a required survey that must be taken before @@ -697,8 +704,7 @@ def _progress(request, course_key, student_id): raise Http404 try: student = User.objects.get(id=student_id) - # Check for ValueError if 'student_id' cannot be converted to integer. - except (ValueError, User.DoesNotExist): + except User.DoesNotExist: raise Http404 # NOTE: To make sure impersonation by instructor works, use diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 13d9c4fd79..dfeb5d9e4c 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -8,7 +8,6 @@ from django.test.client import Client, RequestFactory from django.test.utils import override_settings from django.utils import translation from lms.lib.comment_client.utils import CommentClientPaginatedResult -from edxmako.tests import mako_middleware_process_request from django_comment_common.utils import ThreadContext from django_comment_client.forum import views @@ -441,14 +440,13 @@ class SingleCohortedThreadTestCase(CohortedTestCase): def test_html(self, mock_request): self._create_mock_cohorted_thread(mock_request) - request = RequestFactory().get("dummy_url") - request.user = self.student - mako_middleware_process_request(request) - response = views.single_thread( - request, - self.course.id.to_deprecated_string(), - "cohorted_topic", - self.mock_thread_id + self.client.login(username=self.student.username, password='test') + response = self.client.get( + reverse('single_thread', kwargs={ + 'course_id': unicode(self.course.id), + 'discussion_id': "cohorted_topic", + 'thread_id': self.mock_thread_id, + }) ) self.assertEquals(response.status_code, 200) @@ -561,19 +559,14 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi headers = {} if is_ajax: headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest" - request = RequestFactory().get( - "dummy_url", + + self.client.login(username=user.username, password='test') + + return self.client.get( + reverse('single_thread', args=[unicode(self.course.id), commentable_id, "dummy_thread_id"]), data=request_data, **headers ) - request.user = user - mako_middleware_process_request(request) - return views.single_thread( - request, - self.course.id.to_deprecated_string(), - commentable_id, - "dummy_thread_id" - ) def test_group_info_in_html_response(self, mock_request): response = self.call_view( @@ -599,30 +592,28 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi @patch('requests.request', autospec=True) -class SingleThreadContentGroupTestCase(ContentGroupTestCase): +class SingleThreadContentGroupTestCase(UrlResetMixin, ContentGroupTestCase): + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(SingleThreadContentGroupTestCase, self).setUp() + def assert_can_access(self, user, discussion_id, thread_id, should_have_access): """ Verify that a user has access to a thread within a given discussion_id when should_have_access is True, otherwise verify that the user does not have access to that thread. """ - request = RequestFactory().get("dummy_url") - request.user = user - mako_middleware_process_request(request) - def call_single_thread(): - return views.single_thread( - request, - unicode(self.course.id), - discussion_id, - thread_id + self.client.login(username=user.username, password='test') + return self.client.get( + reverse('single_thread', args=[unicode(self.course.id), discussion_id, thread_id]) ) if should_have_access: self.assertEqual(call_single_thread().status_code, 200) else: - with self.assertRaises(Http404): - call_single_thread() + self.assertEqual(call_single_thread().status_code, 404) def test_staff_user(self, mock_request): """ @@ -813,17 +804,13 @@ class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdT headers = {} if is_ajax: headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest" - request = RequestFactory().get( - "dummy_url", + + self.client.login(username=user.username, password='test') + return self.client.get( + reverse(views.forum_form_discussion, args=[unicode(self.course.id)]), data=request_data, **headers ) - request.user = user - mako_middleware_process_request(request) - return views.forum_form_discussion( - request, - self.course.id.to_deprecated_string() - ) def test_group_info_in_html_response(self, mock_request): response = self.call_view( @@ -869,18 +856,13 @@ class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupI headers = {} if is_ajax: headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest" - request = RequestFactory().get( - "dummy_url", + + self.client.login(username=requesting_user.username, password='test') + return self.client.get( + reverse('user_profile', args=[unicode(self.course.id), profiled_user.id]), data=request_data, **headers ) - request.user = requesting_user - mako_middleware_process_request(request) - return views.user_profile( - request, - self.course.id.to_deprecated_string(), - profiled_user.id - ) def call_view(self, mock_request, _commentable_id, user, group_id, pass_group_id=True, is_ajax=False): return self.call_view_for_profiled_user( @@ -1109,11 +1091,12 @@ class InlineDiscussionTestCase(ModuleStoreTestCase): @patch('requests.request', autospec=True) -class UserProfileTestCase(ModuleStoreTestCase): +class UserProfileTestCase(UrlResetMixin, ModuleStoreTestCase): TEST_THREAD_TEXT = 'userprofile-test-text' TEST_THREAD_ID = 'userprofile-test-thread-id' + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): super(UserProfileTestCase, self).setUp() @@ -1126,14 +1109,15 @@ class UserProfileTestCase(ModuleStoreTestCase): mock_request.side_effect = make_mock_request_impl( course=self.course, text=self.TEST_THREAD_TEXT, thread_id=self.TEST_THREAD_ID ) - request = RequestFactory().get("dummy_url", data=params, **headers) - request.user = self.student + self.client.login(username=self.student.username, password='test') - mako_middleware_process_request(request) - response = views.user_profile( - request, - self.course.id.to_deprecated_string(), - self.profiled_user.id + response = self.client.get( + reverse('user_profile', kwargs={ + 'course_id': unicode(self.course.id), + 'user_id': self.profiled_user.id, + }), + data=params, + **headers ) mock_request.assert_any_call( "get", diff --git a/lms/djangoapps/django_comment_client/tests/utils.py b/lms/djangoapps/django_comment_client/tests/utils.py index e9c0ca966b..eda735dd14 100644 --- a/lms/djangoapps/django_comment_client/tests/utils.py +++ b/lms/djangoapps/django_comment_client/tests/utils.py @@ -7,11 +7,12 @@ from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from django_comment_common.models import Role from django_comment_common.utils import seed_permissions_roles from student.tests.factories import CourseEnrollmentFactory, UserFactory +from util.testing import UrlResetMixin from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -class CohortedTestCase(SharedModuleStoreTestCase): +class CohortedTestCase(UrlResetMixin, SharedModuleStoreTestCase): """ Sets up a course with a student, a moderator and their cohorts. """ diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py index 19e556a660..e67d4e91ab 100644 --- a/lms/djangoapps/notification_prefs/tests.py +++ b/lms/djangoapps/notification_prefs/tests.py @@ -2,6 +2,7 @@ import json from django.contrib.auth.models import AnonymousUser from django.core.exceptions import PermissionDenied +from django.core.urlresolvers import reverse from django.http import Http404 from django.test import TestCase from django.test.client import RequestFactory @@ -11,7 +12,6 @@ from mock import Mock, patch from notification_prefs import NOTIFICATION_PREF_KEY from notification_prefs.views import ajax_enable, ajax_disable, ajax_status, set_subscription, UsernameCipher from student.tests.factories import UserFactory -from edxmako.tests import mako_middleware_process_request from openedx.core.djangoapps.user_api.models import UserPreference from util.testing import UrlResetMixin @@ -214,11 +214,9 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): self.create_prefs() def test_user(user): - request = self.request_factory.get("dummy") - request.user = AnonymousUser() + url = reverse('unsubscribe_forum_update', args=[self.tokens[user]]) - mako_middleware_process_request(request) - response = set_subscription(request, self.tokens[user], subscribe=False) + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertNotPrefExists(user) @@ -227,12 +225,10 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): def test_unsubscribe_twice(self): self.create_prefs() - request = self.request_factory.get("dummy") - request.user = AnonymousUser() - mako_middleware_process_request(request) - set_subscription(request, self.tokens[self.user], False) - response = set_subscription(request, self.tokens[self.user], subscribe=False) + url = reverse('unsubscribe_forum_update', args=[self.tokens[self.user]]) + self.client.get(url) + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertNotPrefExists(self.user) @@ -240,11 +236,8 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): def test_user(user): # start without a pref key self.assertFalse(UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY)) - request = self.request_factory.get("dummy") - request.user = AnonymousUser() - - mako_middleware_process_request(request) - response = set_subscription(request, self.tokens[user], subscribe=True) + url = reverse('resubscribe_forum_update', args=[self.tokens[user]]) + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertPrefValid(user) diff --git a/lms/djangoapps/static_template_view/views.py b/lms/djangoapps/static_template_view/views.py index 3873a64d93..e22eca1dc9 100644 --- a/lms/djangoapps/static_template_view/views.py +++ b/lms/djangoapps/static_template_view/views.py @@ -71,8 +71,8 @@ def render_press_release(request, slug): def render_404(request): - return HttpResponseNotFound(render_to_string('static_templates/404.html', {})) + return HttpResponseNotFound(render_to_string('static_templates/404.html', {}, request=request)) def render_500(request): - return HttpResponseServerError(render_to_string('static_templates/server-error.html', {})) + return HttpResponseServerError(render_to_string('static_templates/server-error.html', {}, request=request)) diff --git a/lms/envs/common.py b/lms/envs/common.py index 3f7c85e75c..fa5c061556 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1087,7 +1087,10 @@ simplefilter('ignore') ################################# Middleware ################################### MIDDLEWARE_CLASSES = ( + 'crum.CurrentRequestUserMiddleware', + 'request_cache.middleware.RequestCache', + 'mobile_api.middleware.AppVersionUpgrade', 'header_control.middleware.HeaderControlMiddleware', 'microsite_configuration.middleware.MicrositeMiddleware', @@ -1111,7 +1114,6 @@ MIDDLEWARE_CLASSES = ( 'student.middleware.UserStandingMiddleware', 'contentserver.middleware.StaticContentServer', - 'crum.CurrentRequestUserMiddleware', # Adds user tags to tracking events # Must go before TrackMiddleware, to get the context set up @@ -1151,8 +1153,6 @@ MIDDLEWARE_CLASSES = ( # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500 'ratelimitbackend.middleware.RateLimitMiddleware', - # needs to run after locale middleware (or anything that modifies the request context) - 'edxmako.middleware.MakoMiddleware', # for expiring inactive sessions 'session_inactivity_timeout.middleware.SessionInactivityTimeout',