Merge pull request #156 from pmclanahan/legacy-private-subscriptions-…

…1228052

LEGACY: Fix bug 1228052: Add private newsletters.

.travis.yml

@@ -1,7 +1,7 @@
 sudo: false
 language: python
 python:
-  - "2.6"
+  - "2.7"
 env:
   - PIP_DOWNLOAD_CACHE="pip_cache"
 cache:

news/admin.py

@@ -48,13 +48,13 @@ class SubscriberAdmin(admin.ModelAdmin):
 class NewsletterAdmin(admin.ModelAdmin):
     fields = ('title', 'slug', 'vendor_id', 'welcome', 'confirm_message',
               'description', 'languages', 'show', 'order', 'active',
-              'requires_double_optin')
+              'requires_double_optin', 'private')
     list_display = ('order', 'title', 'slug', 'vendor_id', 'welcome',
                     'confirm_message', 'languages', 'show', 'active',
-                    'requires_double_optin')
+                    'requires_double_optin', 'private')
     list_display_links = ('title', 'slug')
-    list_editable = ('order', 'show', 'active', 'requires_double_optin')
-    list_filter = ('show', 'active', 'requires_double_optin')
+    list_editable = ('order', 'show', 'active', 'requires_double_optin', 'private')
+    list_filter = ('show', 'active', 'requires_double_optin', 'private')
     prepopulated_fields = {"slug": ("title",)}
     search_fields = ('title', 'slug', 'description', 'vendor_id')
 

news/migrations/0015_auto__add_field_newsletter_private.py

@@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'Newsletter.private'
+        db.add_column(u'news_newsletter', 'private',
+                      self.gf('django.db.models.fields.BooleanField')(default=False),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'Newsletter.private'
+        db.delete_column(u'news_newsletter', 'private')
+
+
+    models = {
+        u'news.apiuser': {
+            'Meta': {'object_name': 'APIUser'},
+            'api_key': ('django.db.models.fields.CharField', [], {'default': "'ea007af5-4292-4fcd-bc69-01f45c8a09ed'", 'max_length': '40', 'db_index': 'True'}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '256'})
+        },
+        u'news.blockedemail': {
+            'Meta': {'object_name': 'BlockedEmail'},
+            'email_domain': ('django.db.models.fields.CharField', [], {'max_length': '50'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        },
+        u'news.failedtask': {
+            'Meta': {'object_name': 'FailedTask'},
+            'args': ('jsonfield.fields.JSONField', [], {'default': '[]'}),
+            'einfo': ('django.db.models.fields.TextField', [], {'default': 'None', 'null': 'True'}),
+            'exc': ('django.db.models.fields.TextField', [], {'default': 'None', 'null': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'kwargs': ('jsonfield.fields.JSONField', [], {'default': '{}'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'task_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
+            'when': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'})
+        },
+        u'news.interest': {
+            'Meta': {'object_name': 'Interest'},
+            '_welcome_id': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}),
+            'default_steward_emails': ('news.fields.CommaSeparatedEmailField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'interest_id': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'})
+        },
+        u'news.localestewards': {
+            'Meta': {'unique_together': "(('interest', 'locale'),)", 'object_name': 'LocaleStewards'},
+            'emails': ('news.fields.CommaSeparatedEmailField', [], {}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'interest': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['news.Interest']"}),
+            'locale': ('news.fields.LocaleField', [], {'max_length': '32'})
+        },
+        u'news.newsletter': {
+            'Meta': {'ordering': "['order']", 'object_name': 'Newsletter'},
+            'active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'confirm_message': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}),
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '256', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'languages': ('django.db.models.fields.CharField', [], {'max_length': '200'}),
+            'order': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'private': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'requires_double_optin': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'show': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'slug': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'vendor_id': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'welcome': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'})
+        },
+        u'news.newslettergroup': {
+            'Meta': {'object_name': 'NewsletterGroup'},
+            'active': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '256', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'newsletters': ('django.db.models.fields.related.ManyToManyField', [], {'related_name': "'newsletter_groups'", 'symmetrical': 'False', 'to': u"orm['news.Newsletter']"}),
+            'show': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'slug': ('django.db.models.fields.SlugField', [], {'unique': 'True', 'max_length': '50'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '128'})
+        },
+        u'news.smsmessage': {
+            'Meta': {'object_name': 'SMSMessage'},
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '200', 'blank': 'True'}),
+            'message_id': ('django.db.models.fields.SlugField', [], {'max_length': '50', 'primary_key': 'True'}),
+            'vendor_id': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        u'news.subscriber': {
+            'Meta': {'object_name': 'Subscriber'},
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'primary_key': 'True'}),
+            'fxa_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '100', 'null': 'True', 'blank': 'True'}),
+            'token': ('django.db.models.fields.CharField', [], {'default': "'f92b5b31-dc5f-44f3-8d3e-e91fe94fb05a'", 'max_length': '40', 'db_index': 'True'})
+        }
+    }
+
+    complete_apps = ['news']

news/models.py

@@ -74,6 +74,11 @@ class Newsletter(models.Model):
                   "are only shown to those who are already subscribed, and "
                   "might have other differences in behavior.",
     )
+    private = models.BooleanField(
+        default=False,
+        help_text="Whether this newsletter is private. Private newsletters "
+                  "require the subscribe requests to use an API key.",
+    )
     # Note: use .welcome_id property to get this field or the default
     welcome = models.CharField(
         max_length=64,

news/newsletters.py

@@ -136,6 +136,11 @@ def newsletter_and_group_slugs():
     return list(set(newsletter_slugs()) | set(newsletter_group_slugs()))
 
 
+def newsletter_private_slugs():
+    """Return a list of private newsletter ids"""
+    return [nl.slug for nl in _newsletters()['by_name'].values() if nl.private]
+
+
 def slug_to_vendor_id(slug):
     """Given a newsletter's slug, return its vendor_id"""
     return _newsletters()['by_name'][slug].vendor_id

news/tests/test__utils.py

@@ -10,8 +10,8 @@
 
 from news import tasks
 from news.models import BlockedEmail
-from news.tasks import SUBSCRIBE
 from news.utils import (
+    SUBSCRIBE, UNSUBSCRIBE, SET,
     email_block_list_cache,
     email_is_blocked,
     EmailValidationError,
@@ -233,6 +233,114 @@ def test_success_with_sync_no_subscriber(self):
         self.assert_response_ok(response, token='mytoken', created=True)
         self.update_user.delay.assert_called_with(data, 'a@example.com', 'mytoken', SUBSCRIBE, True)
 
+    @patch('news.utils.newsletter_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    def test_success_with_unsubscribe_private_newsletter(self, mock_private, mock_slugs):
+        """
+        Should be able to unsubscribe from a private newsletter regardless.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        request = self.factory.post('/')
+        data = {'token': 'mytoken', 'email': 'dude@example.com', 'newsletters': 'private'}
+        response = update_user_task(request, UNSUBSCRIBE, data)
+
+        self.assert_response_ok(response)
+        self.update_user.delay.assert_called_with(data, 'dude@example.com', None,
+                                                  UNSUBSCRIBE, True)
+
+    @patch('news.utils.newsletter_and_group_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    def test_subscribe_private_newsletter_ssl_required(self, mock_private, mock_slugs):
+        """
+        If subscribing to a private newsletter and the request isn't HTTPS, return a 401.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        data = {'newsletters': 'private', 'email': 'dude@example.com'}
+        request = self.factory.post('/', data)
+        request.is_secure = lambda: False
+        response = update_user_task(request, SUBSCRIBE, data)
+        self.assert_response_error(response, 401, errors.BASKET_SSL_REQUIRED)
+
+    @patch('news.utils.newsletter_and_group_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    @patch('news.utils.has_valid_api_key')
+    def test_subscribe_private_newsletter_invalid_api_key(self, mock_api_key, mock_private, mock_slugs):
+        """
+        If subscribing to a private newsletter and the request has an invalid API key,
+        return a 401.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        data = {'newsletters': 'private', 'email': 'dude@example.com'}
+        request = self.factory.post('/', data)
+        request.is_secure = lambda: True
+        mock_api_key.return_value = False
+
+        response = update_user_task(request, SUBSCRIBE, data)
+        self.assert_response_error(response, 401, errors.BASKET_AUTH_ERROR)
+        mock_api_key.assert_called_with(request)
+
+    @patch('news.utils.newsletter_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    def test_set_private_newsletter_ssl_required(self, mock_private, mock_slugs):
+        """
+        If subscribing to a private newsletter and the request isn't HTTPS, return a 401.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        data = {'newsletters': 'private', 'email': 'dude@example.com'}
+        request = self.factory.post('/', data)
+        request.is_secure = lambda: False
+        response = update_user_task(request, SET, data)
+        self.assert_response_error(response, 401, errors.BASKET_SSL_REQUIRED)
+
+    @patch('news.utils.newsletter_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    @patch('news.utils.has_valid_api_key')
+    def test_set_private_newsletter_invalid_api_key(self, mock_api_key, mock_private, mock_slugs):
+        """
+        If subscribing to a private newsletter and the request has an invalid API key,
+        return a 401.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        data = {'newsletters': 'private', 'email': 'dude@example.com'}
+        request = self.factory.post('/', data)
+        request.is_secure = lambda: True
+        mock_api_key.return_value = False
+
+        response = update_user_task(request, SET, data)
+        self.assert_response_error(response, 401, errors.BASKET_AUTH_ERROR)
+        mock_api_key.assert_called_with(request)
+
+    @patch('news.utils.newsletter_slugs')
+    @patch('news.utils.newsletter_and_group_slugs')
+    @patch('news.utils.newsletter_private_slugs')
+    @patch('news.utils.has_valid_api_key')
+    def test_private_newsletter_success(self, mock_api_key, mock_private, mock_group_slugs,
+                                        mock_slugs):
+        """
+        If subscribing to a private newsletter and the request has an invalid API key,
+        return a 401.
+        """
+        mock_private.return_value = ['private']
+        mock_slugs.return_value = ['private', 'other']
+        mock_group_slugs.return_value = ['private', 'other']
+        data = {'newsletters': 'private', 'email': 'dude@example.com'}
+        request = self.factory.post('/', data)
+        request.is_secure = lambda: True
+        mock_api_key.return_value = True
+
+        response = update_user_task(request, SUBSCRIBE, data)
+        self.assert_response_ok(response)
+        mock_api_key.assert_called_with(request)
+
+        response = update_user_task(request, SET, data)
+        self.assert_response_ok(response)
+        mock_api_key.assert_called_with(request)
+
 
 class TestGetAcceptLanguages(TestCase):
     # mostly stolen from bedrock

news/tests/test_newsletters.py

@@ -46,6 +46,12 @@ def setUp(self):
                 title='Beginning Nihilism',
                 vendor_id='EXTORTING',
                 languages='en'),
+            Newsletter.objects.create(
+                slug='papers',
+                title='Just papers, personal papers',
+                vendor_id='CREEDENCE',
+                languages='en',
+                private=True),
         ]
         self.groupies = [
             NewsletterGroup.objects.create(
@@ -63,27 +69,30 @@ def setUp(self):
         ]
         self.groupies[0].newsletters.add(self.newsies[1], self.newsies[2])
 
+    def test_newseltter_private_slugs(self):
+        self.assertEqual(newsletters.newsletter_private_slugs(), ['papers'])
+
     def test_newsletter_slugs(self):
         self.assertEqual(set(newsletters.newsletter_slugs()),
-                         set(['bowling', 'surfing', 'extorting']))
+                         {'bowling', 'surfing', 'extorting', 'papers'})
 
     def test_newsletter_group_slugs(self):
         self.assertEqual(set(newsletters.newsletter_group_slugs()),
-                         set(['bowling', 'abiding']))
+                         {'bowling', 'abiding'})
 
     def test_newsletter_and_group_slugs(self):
         self.assertEqual(set(newsletters.newsletter_and_group_slugs()),
-                         set(['bowling', 'abiding', 'surfing', 'extorting']))
+                         {'bowling', 'abiding', 'surfing', 'extorting', 'papers'})
 
     def test_newsletter_group_newsletter_slugs(self):
         self.assertEqual(set(newsletters.newsletter_group_newsletter_slugs('bowling')),
-                         set(['extorting', 'surfing']))
+                         {'extorting', 'surfing'})
 
     def test_parse_newsletters_for_groups(self):
         """If newsletter slug is a group for SUBSCRIBE, expand to group's newsletters."""
         record = {}
         to_sub, to_unsub = utils.parse_newsletters(record, utils.SUBSCRIBE, ['bowling'], set())
-        self.assertEqual(set(to_sub), set(['surfing', 'extorting']))
+        self.assertEqual(set(to_sub), {'surfing', 'extorting'})
         self.assertEqual(record['SURFING_FLG'], 'Y')
         self.assertEqual(record['EXTORTING_FLG'], 'Y')
 
@@ -98,6 +107,6 @@ def test_parse_newsletters_not_groups_unsubscribe(self):
         """If newsletter slug is a group for SET mode, don't expand to group's newsletters."""
         record = {}
         to_sub, to_unsub = utils.parse_newsletters(record, utils.UNSUBSCRIBE, ['bowling'],
-                                                   set(['bowling', 'surfing', 'extorting']))
+                                                   {'bowling', 'surfing', 'extorting'})
         self.assertEqual(to_unsub, ['bowling'])
         self.assertEqual(record['BOWLING_FLG'], 'N')

news/tests/test_views.py

@@ -638,15 +638,17 @@ def test_newsletters_view(self):
         )
 
         models.Newsletter.objects.create(slug='slug2', vendor_id='VENDOR2')
+        models.Newsletter.objects.create(slug='slug3', vendor_id='VENDOR3', private=True)
 
         req = self.rf.get(self.url)
         resp = views.newsletters(req)
         data = json.loads(resp.content)
         newsletters = data['newsletters']
-        self.assertEqual(2, len(newsletters))
+        self.assertEqual(3, len(newsletters))
         # Find the 'slug' newsletter in the response
         obj = newsletters['slug']
 
+        self.assertTrue(newsletters['slug3']['private'])
         self.assertEqual(nl1.title, obj['title'])
         self.assertEqual(nl1.active, obj['active'])
         for lang in ['en-US', 'fr']: