From 913ce71812ae165a67c09292debe0cbb6a46a843 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Thu, 29 Nov 2012 15:24:46 +0100 Subject: [PATCH] [#1635] Add email notifications user preference Don't send email notifications if the user has them turned off in her preferences. --- ckan/lib/email_notifications.py | 6 ++ ckan/logic/schema.py | 1 + ckan/model/user.py | 1 + ckan/tests/lib/test_email_notifications.py | 97 ++++++++++++++++++++-- 4 files changed, 96 insertions(+), 9 deletions(-) diff --git a/ckan/lib/email_notifications.py b/ckan/lib/email_notifications.py index 8661e39d184..dae232b1980 100644 --- a/ckan/lib/email_notifications.py +++ b/ckan/lib/email_notifications.py @@ -116,6 +116,12 @@ def send_notification(user, email_dict): def get_and_send_notifications_for_user(user): + + # Don't send email if the user has her email notifications preference + # turned off. + if not user['email_notifications']: + return + # FIXME: We are accessing model from lib here but I'm not sure what else # to do unless we add a get_email_last_sent() logic function # which would only be needed by this lib. diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index b1bd1a9a833..639faf36c4e 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -344,6 +344,7 @@ def default_user_schema(): 'openid': [ignore_missing], 'apikey': [ignore], 'reset_key': [ignore], + 'email_notifications': [ignore_missing], } return schema diff --git a/ckan/model/user.py b/ckan/model/user.py index 7ef431f6b2c..a8dcee53e7e 100644 --- a/ckan/model/user.py +++ b/ckan/model/user.py @@ -23,6 +23,7 @@ Column('created', types.DateTime, default=datetime.datetime.now), Column('reset_key', types.UnicodeText), Column('about', types.UnicodeText), + Column('email_notifications', types.Boolean, default=False), ) diff --git a/ckan/tests/lib/test_email_notifications.py b/ckan/tests/lib/test_email_notifications.py index 0fa5d924fb8..9b216c7ecb6 100644 --- a/ckan/tests/lib/test_email_notifications.py +++ b/ckan/tests/lib/test_email_notifications.py @@ -73,7 +73,7 @@ def test_01_no_email_notifications_after_registration(self): # Register a new user. sara = post(self.app, 'user_create', apikey=self.joeadmin['apikey'], name='sara', email='sara@sararollins.com', password='sara', - fullname='Sara Rollins') + fullname='Sara Rollins', email_notifications=True) # Save the user for later tests to use. TestEmailNotifications.sara = sara @@ -172,13 +172,92 @@ def test_06_enable_email_notifications_sitewide(self): ''' - def test_07_no_email_notifications_when_disabled_by_user(self): - '''Users should not get email notifications when they have disabled - the feature in their user preferences.''' - def test_08_enable_email_notifications_by_user(self): - '''When a user enables email notifications in her user preferences, - she should not get emails for new activities from before email - notifications were enabled. +# It's just easier to separate these tests into their own test class. +class TestEmailNotificationsUserPreference(mock_mail_server.SmtpServerHarness, + ckan.tests.pylons_controller.PylonsTestCase): + '''Tests for the email notifications (on/off) user preference.''' - ''' + @classmethod + def setup_class(cls): + mock_mail_server.SmtpServerHarness.setup_class() + ckan.tests.pylons_controller.PylonsTestCase.setup_class() + ckan.tests.CreateTestData.create() + cls.app = paste.fixture.TestApp(pylons.test.pylonsapp) + joeadmin = model.User.get('joeadmin') + cls.joeadmin = {'id': joeadmin.id, + 'apikey': joeadmin.apikey, + } + + @classmethod + def teardown_class(self): + mock_mail_server.SmtpServerHarness.teardown_class() + ckan.tests.pylons_controller.PylonsTestCase.teardown_class() + model.repo.rebuild_db() + + def test_00_email_notifications_disabled_by_default(self): + '''Email notifications should be disabled for new users.''' + + # Register a new user. + sara = post(self.app, 'user_create', apikey=self.joeadmin['apikey'], + name='sara', email='sara@sararollins.com', password='sara', + fullname='Sara Rollins') + + # Save the user for later tests to use. + TestEmailNotificationsUserPreference.sara = sara + + # Email notifications should be disabled for the new user. + assert sara['email_notifications'] is False + assert post(self.app, 'user_show', apikey=self.sara['apikey'], + id='sara')['email_notifications'] is False + + def test_01_no_email_notifications_when_disabled(self): + '''Users with email notifications turned off should not get emails.''' + + # First make Sara follow something so she gets some new activity in + # her dashboard activity stream. + post(self.app, 'follow_dataset', apikey=self.sara['apikey'], + id='warandpeace') + + # Now make someone else update the dataset so Sara gets a new activity. + post(self.app, 'package_update', apikey=self.joeadmin['apikey'], + id='warandpeace', notes='updated') + + # Test that Sara has a new activity, just to make sure. + assert post(self.app, 'dashboard_new_activities_count', + apikey=self.sara['apikey']) > 0 + + # No email notifications should be sent to Sara. + email_notifications.get_and_send_notifications_for_all_users() + assert len(self.get_smtp_messages()) == 0 + + def test_02_enable_email_notifications(self): + '''Users should be able to turn email notifications on.''' + + self.sara['email_notifications'] = True + post(self.app, 'user_update', **self.sara) + + post(self.app, 'package_update', apikey=self.joeadmin['apikey'], + id='warandpeace', notes='updated again') + + assert post(self.app, 'dashboard_new_activities_count', + apikey=self.sara['apikey']) > 0 + + email_notifications.get_and_send_notifications_for_all_users() + assert len(self.get_smtp_messages()) == 1 + self.clear_smtp_messages() + + def test_03_disable_email_notifications(self): + '''Users should be able to turn email notifications off.''' + + self.sara['email_notifications'] = False + post(self.app, 'user_update', **self.sara) + + post(self.app, 'package_update', apikey=self.joeadmin['apikey'], + id='warandpeace', notes='updated yet again') + + assert post(self.app, 'dashboard_new_activities_count', + apikey=self.sara['apikey']) > 0 + + email_notifications.get_and_send_notifications_for_all_users() + assert len(self.get_smtp_messages()) == 0