diff --git a/ckan/lib/email_notifications.py b/ckan/lib/email_notifications.py index ca153631b94..4a1ab4d838d 100644 --- a/ckan/lib/email_notifications.py +++ b/ckan/lib/email_notifications.py @@ -105,6 +105,11 @@ def send_notification(user, email_dict): try: ckan.lib.mailer.mail_recipient(user['display_name'], user['email'], email_dict['subject'], email_dict['body']) + context = {'model': model, 'session': model.Session, + 'user': user['name']} + response = logic.get_action('dashboard_update_email_notification_last_sent')( + context, {}) + # TODO: Do something with response? except ckan.lib.mailer.MailerException: raise @@ -113,7 +118,10 @@ def get_and_send_notifications_for_user(user): # FIXME: Get the time that the last email notification was sent to the # user and the time that they last viewed their dashboard, set `since` to # whichever is more recent. - since = datetime.datetime.min + context = {'model': model, 'session': model.Session, 'user': user['name']} + since = logic.get_action('dashboard_email_notification_last_sent')( + context, {}) + notifications = get_notifications(user['id'], since) # TODO: Handle failures from send_email_notification. for notification in notifications: diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 5d51721e4d8..98d469ea7cb 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2300,9 +2300,9 @@ def dashboard_mark_activities_as_read(context, data_dict): def dashboard_email_notification_last_sent(context, data_dict): model = context['model'] - user = model.User.get(context['user']) # The authorized user. + user = model.User.get(context['user']) # The authorized user. last_viewed = model.Dashboard.get_activity_stream_last_viewed(user.id) - return last_viewed.timetuple() + return last_viewed def get_email_notifications(context, data_dict): diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 5dfb365ba7a..ea23b910190 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -937,3 +937,9 @@ def user_role_bulk_update(context, data_dict): 'domain_object': data_dict['domain_object']} user_role_update(context, uro_data_dict) return _get_action('roles_show')(context, data_dict) + + +def dashboard_update_email_notification_last_sent(context, data_dict): + model = context['model'] + user = model.User.get(context['user']) # The authorized user. + model.Dashboard.update_activity_stream_last_viewed(user.id) diff --git a/ckan/tests/lib/test_email_notifications.py b/ckan/tests/lib/test_email_notifications.py index 25022ce3e69..3e36218147a 100644 --- a/ckan/tests/lib/test_email_notifications.py +++ b/ckan/tests/lib/test_email_notifications.py @@ -41,6 +41,16 @@ def mime_encode(self, msg, recipient_name): body.encode('utf-8'), 'plain', 'utf-8').get_payload().strip() return encoded_body + def check_email(self, email, address, name, subject, body): + assert email[1] == 'info@test.ckan.net' + assert email[2] == [address] + encoded_subject = 'Subject: =?utf-8?q?{subject}'.format( + subject=subject.replace(' ', '_')) + assert encoded_subject in email[3] + encoded_body = self.mime_encode(body, name) + assert encoded_body in email[3] + # TODO: Check that body contains link to dashboard and email prefs. + def test_01_no_email_notifications_after_registration(self): '''A new user who isn't following anything shouldn't get any emails.''' @@ -89,24 +99,44 @@ def test_02_one_new_activity(self): email_notifications.get_and_send_notifications_for_all_users() assert len(self.get_smtp_messages()) == 1 email = self.get_smtp_messages()[0] - assert email[1] == 'info@test.ckan.net' - assert email[2] == ['sara@sararollins.com'] - assert 'Subject: =?utf-8?q?You_have_new_activity' in email[3] - encoded_body = self.mime_encode( - "You have new activity", "Sara Rollins") - assert encoded_body in email[3] - # TODO: Check that body contains link to dashboard and email prefs. + self.check_email(email, 'sara@sararollins.com', 'Sara Rollins', + 'You have new activity', 'You have new activity') + + self.clear_smtp_messages() def test_03_multiple_new_activities(self): '''Test that a user with multiple new activities gets just one email. ''' + # Make someone else update the dataset Sara's following three times, + # this should create three new activities on Sara's dashboard. + for i in range(1, 4): + params = {'name': 'warandpeace', + 'notes': 'updated {0} times'.format(i)} + extra_environ = {'Authorization': str(self.joeadmin['apikey'])} + response = self.app.post('/api/action/package_update', + params=json.dumps(params), extra_environ=extra_environ).json + assert response['success'] is True + + # Run the email notifier job, it should send one notification email + # to Sara. + email_notifications.get_and_send_notifications_for_all_users() + assert len(self.get_smtp_messages()) == 1 + email = self.get_smtp_messages()[0] + self.check_email(email, 'sara@sararollins.com', 'Sara Rollins', + 'You have new activity', 'You have new activity') + + self.clear_smtp_messages() def test_04_no_repeat_email_notifications(self): '''Test that a user does not get a second email notification for the same new activity. ''' + # TODO: Assert that Sara has some new activities and has already had + # an email about them. + email_notifications.get_and_send_notifications_for_all_users() + assert len(self.get_smtp_messages()) == 0 def test_05_no_email_notifications_when_disabled_site_wide(self): '''Users should not get email notifications when the feature is