Merge pull request revsys#30 from blag/small-fixups

Small fixups

tos/apps.py

@@ -1,9 +1,8 @@
 from django.apps import AppConfig
 from django.conf import settings
-from django.core.cache import caches
 from django.db.models.signals import pre_save
-from django.dispatch import receiver
 
+from .signal_handlers import invalidate_cached_agreements
 
 MIDDLEWARES = getattr(settings, 'MIDDLEWARE_CLASSES', [])
 
@@ -14,23 +13,11 @@ class TOSConfig(AppConfig):
 
     def ready(self):
         if 'tos.middleware.UserAgreementMiddleware' in MIDDLEWARES:
-            # Force the user to create a separate cache
-            cache = caches[getattr(settings, 'TOS_CACHE_NAME', 'default')]
-
             TermsOfService = self.get_model('TermsOfService')
 
-            @receiver(pre_save, sender=TermsOfService, dispatch_uid='invalidate_cached_agreements')
-            def invalidate_cached_agreements(TermsOfService, instance, **kwargs):
-                if kwargs.get('raw', False):
-                    return
-
-                # Set the key version to 0 if it doesn't exist and leave it
-                # alone if it does
-                cache.add('django:tos:key_version', 0)
-
-                # This key will be used to version the rest of the TOS keys
-                # Incrementing it will effectively invalidate all previous keys
-                cache.incr('django:tos:key_version')
+            pre_save.connect(invalidate_cached_agreements,
+                             sender=TermsOfService,
+                             dispatch_uid='invalidate_cached_agreements')
 
             # Create the TOS key version immediately
             invalidate_cached_agreements(TermsOfService, None)

tos/signal_handlers.py

@@ -0,0 +1,19 @@
+from django.conf import settings
+
+from .compat import get_cache
+
+# Force the user to create a separate cache
+cache = get_cache(getattr(settings, 'TOS_CACHE_NAME', 'default'))
+
+
+def invalidate_cached_agreements(TermsOfService, instance, **kwargs):
+    if kwargs.get('raw', False):
+        return
+
+    # Set the key version to 0 if it doesn't exist and leave it
+    # alone if it does
+    cache.add('django:tos:key_version', 0)
+
+    # This key will be used to version the rest of the TOS keys
+    # Incrementing it will effectively invalidate all previous keys
+    cache.incr('django:tos:key_version')

tos/templates/tos/tos.html

@@ -1,3 +1,5 @@
-<h2>Terms of Service as of {{ tos.created|date:"SHORT_DATE_FORMAT" }}</h2>
+{% load i18n %}
 
-{{ tos.content|safe }}
+<h2>{% blocktrans %}Terms of Service as of {{ tos.created|date:"SHORT_DATE_FORMAT" }}{% endblocktrans %}</h2>
+
+{{ tos.content|safe }}

tos/templates/tos/tos_check.html

@@ -1,19 +1,20 @@
+{% load i18n %}
 {% load url from compat %}
 {% if note %}
     <h2>{{ note }}</note>
 {% else %}
-    <h2>Terms of Service as of {{ tos.created|date:"SHORT_DATE_FORMAT" }}</h2>
+    <h2>{% blocktrans %}Terms of Service as of {{ tos.created|date:"SHORT_DATE_FORMAT" }}{% endblocktrans %}</h2>
 {% endif %}
 
 {{ tos.content|safe }}
 
-<h2>Accept Terms of Service?</h2>
+<h2>{% trans "Accept Terms of Service?" %}</h2>
 
 <form method="post" action="{% url "tos_check_tos" %}">
     {% csrf_token %}
     {% if next %}
-        <input type="hidden" name="next" value="{{next}}">
+        <input type="hidden" name="{{ redirect_field_name }}" value="{{ next }}">
     {% endif %}
-    <input type="submit" name="accept" value="accept">
-    <input type="submit" name="accept" value="reject">
+    <button type="submit" name="reject" value="reject">{% trans "Reject" %}</button>
+    <button type="submit" name="accept" value="accept">{% trans "Accept" %}</button>
 </form>

tos/tests/test_middleware.py

@@ -4,8 +4,10 @@
 from django.test import TestCase
 from django.test.utils import override_settings
 
-from tos.compat import get_runtime_user_model
+from tos.compat import get_cache, get_runtime_user_model
+from tos.middleware import UserAgreementMiddleware
 from tos.models import TermsOfService, UserAgreement
+from tos.signal_handlers import invalidate_cached_agreements
 
 
 @override_settings(
@@ -82,3 +84,70 @@ def test_accept_after_middleware_redirects_properly(self):
         # Confirm redirects back to the index page
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url.replace('http://testserver', ''), str(reverse('index')))
+
+
+@override_settings(
+    MIDDLEWARE_CLASSES=settings.MIDDLEWARE_CLASSES + [
+        'tos.middleware.UserAgreementMiddleware',
+    ]
+)
+class BumpCoverage(TestCase):
+
+    def setUp(self):
+        # User that has agreed to TOS
+        self.user1 = get_runtime_user_model().objects.create_user('user1', 'user1@example.com', 'user1pass')
+
+        self.tos1 = TermsOfService.objects.create(
+            content="first edition of the terms of service",
+            active=True
+        )
+        self.tos2 = TermsOfService.objects.create(
+            content="second edition of the terms of service",
+            active=False
+        )
+        self.login_url = getattr(settings, 'LOGIN_URL', '/login/')
+
+        UserAgreement.objects.create(
+            terms_of_service=self.tos1,
+            user=self.user1
+        )
+
+    def test_ajax_request(self):
+        class Request(object):
+            method = 'GET'
+
+            def is_ajax(self):
+                return True
+
+        mw = UserAgreementMiddleware()
+
+        response = mw.process_request(Request())
+
+        self.assertIsNone(response)
+
+    def test_skip_for_user(self):
+        cache = get_cache(getattr(settings, 'TOS_CACHE_NAME', 'default'))
+
+        key_version = cache.get('django:tos:key_version')
+
+        cache.set('django:tos:skip_tos_check:{0}'.format(str(self.user1.id)), True, version=key_version)
+
+        self.client.login(username='user1', password='user1pass')
+        response = self.client.get(reverse('index'))
+
+        self.assertEqual(response.status_code, 200)
+
+    def test_invalidate_cached_agreements(self):
+        cache = get_cache(getattr(settings, 'TOS_CACHE_NAME', 'default'))
+
+        invalidate_cached_agreements(TermsOfService, {})
+
+        key_version = cache.get('django:tos:key_version')
+
+        invalidate_cached_agreements(TermsOfService, {})
+
+        self.assertEqual(cache.get('django:tos:key_version'), key_version+1)
+
+        invalidate_cached_agreements(TermsOfService, {}, raw=True)
+
+        self.assertEqual(cache.get('django:tos:key_version'), key_version+1)

tos/tests/test_views.py

@@ -41,6 +41,29 @@ def test_login(self):
         self.failUnless(login, 'Could not log in')
         self.assertTrue(has_user_agreed_latest_tos(self.user1))
 
+    def test_user_agrees_multiple_times(self):
+        login_response = self.client.post(reverse('login'), {
+            'username': 'user2',
+            'password': 'user2pass',
+        })
+
+        self.assertTrue(login_response)
+
+        response = self.client.post(reverse('tos_check_tos'), {'accept': 'accept'})
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(UserAgreement.objects.filter(user=self.user2).count(), 1)
+
+        response = self.client.post(reverse('tos_check_tos'), {'accept': 'accept'})
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(UserAgreement.objects.filter(user=self.user2).count(), 1)
+
+        response = self.client.post(reverse('tos_check_tos'), {'accept': 'accept'})
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(UserAgreement.objects.filter(user=self.user2).count(), 1)
+
     def test_need_agreement(self):
         """ user2 tries to login and then has to go and agree to terms"""
 

tos/views.py

@@ -57,7 +57,7 @@ def check_tos(request, template_name='tos/tos_check.html',
             user.backend = request.session['tos_backend']
 
             # Save the user agreement to the new TOS
-            UserAgreement.objects.create(terms_of_service=tos, user=user)
+            UserAgreement.objects.get_or_create(terms_of_service=tos, user=user)
 
             # Log the user in
             auth_login(request, user)
@@ -75,12 +75,14 @@ def check_tos(request, template_name='tos/tos_check.html',
     if DJANGO_VERSION >= (1, 10, 0):
         return render(request, template_name, {
             'tos': tos,
-            redirect_field_name: redirect_to,
+            'redirect_field_name': redirect_field_name,
+            'next': redirect_to,
         })
     else:
         return render_to_response(template_name, {
             'tos': tos,
-            redirect_field_name: redirect_to,
+            'redirect_field_name': redirect_field_name,
+            'next': redirect_to,
         }, RequestContext(request))