From 512db7be998cbe8d9a6e8f67cd4a69300c107229 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 9 Sep 2014 08:50:10 -0400 Subject: [PATCH] Fix embargo middleware with tests --- common/djangoapps/embargo/middleware.py | 2 +- .../embargo/tests/test_middleware.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py index e7c199cc6e..99c911c320 100644 --- a/common/djangoapps/embargo/middleware.py +++ b/common/djangoapps/embargo/middleware.py @@ -166,7 +166,7 @@ class EmbargoMiddleware(object): profile_country = cache.get(cache_key) if profile_country is None: profile = getattr(user, 'profile', None) - if profile is not None and profile.country is not None: + if profile is not None and profile.country.code is not None: profile_country = profile.country.code.upper() else: profile_country = "" diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index 5e1a908b8a..5407b8bde1 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -8,6 +8,7 @@ import unittest from django.core.urlresolvers import reverse from django.conf import settings +from django.db import connection, transaction from django.test.utils import override_settings import ddt @@ -267,6 +268,24 @@ class EmbargoMiddlewareTests(ModuleStoreTestCase): with self.assertNumQueries(12): self.client.get(self.embargoed_page) + def test_embargo_profile_country_db_null(self): + # Django country fields treat NULL values inconsistently. + # When saving a profile with country set to None, Django saves an empty string to the database. + # However, when the country field loads a NULL value from the database, it sets + # `country.code` to `None`. This caused a bug in which country values created by + # the original South schema migration -- which defaulted to NULL -- caused a runtime + # exception when the embargo middleware treated the value as a string. + # In order to simulate this behavior, we can't simply set `profile.country = None`. + # (because when we save it, it will set the database field to an empty string instead of NULL) + query = "UPDATE auth_userprofile SET country = NULL WHERE id = %s" + connection.cursor().execute(query, [str(self.user.profile.id)]) + transaction.commit_unless_managed() + + # Attempt to access an embargoed course + # Verify that the student can access the page without an error + response = self.client.get(self.embargoed_page) + self.assertEqual(response.status_code, 200) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': False}) def test_countries_embargo_off(self): # When the middleware is turned off, all requests should go through