[#1635] Don't send emails for activities already seen on dashboard

ckan/lib/email_notifications.py

@@ -116,13 +116,13 @@ def send_notification(user, email_dict):
 
 
 def get_and_send_notifications_for_user(user):
-    # FIXME: `since` here should be the time that the last email notification
-    # was sent _or_ the time the user last viewed her dashboard, whichever is
-    # newer.
     # 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.
-    since = model.Dashboard.get(user['id']).email_last_sent
+    email_last_sent = model.Dashboard.get(user['id']).email_last_sent
+    activity_stream_last_viewed = (
+            model.Dashboard.get(user['id']).activity_stream_last_viewed)
+    since = max(email_last_sent, activity_stream_last_viewed)
 
     notifications = get_notifications(user['id'], since)
     # TODO: Handle failures from send_email_notification.

ckan/tests/lib/test_email_notifications.py

@@ -134,6 +134,33 @@ def test_04_no_repeat_email_notifications(self):
         email_notifications.get_and_send_notifications_for_all_users()
         assert len(self.get_smtp_messages()) == 0
 
+    def test_05_no_email_if_seen_on_dashboard(self):
+        '''Test that emails are not sent for activities already seen on dash.
+
+        If a user gets some new activities in her dashboard activity stream,
+        then views her dashboard activity stream, then she should not got any
+        email notifications about these new activities.
+
+        '''
+        # Make someone else update the dataset Sara's following, this should
+        # create a new activity on Sara's dashboard.
+        post(self.app, 'package_update', apikey=self.joeadmin['apikey'],
+                name='warandpeace',
+                notes='updated by test_05_no_email_if_seen_on_dashboard')
+
+        # At this point Sara should have a new activity on her dashboard.
+        num_new_activities = post(self.app, 'dashboard_new_activities_count',
+                apikey=self.sara['apikey'])
+        assert num_new_activities > 0, num_new_activities
+
+        # View Sara's dashboard.
+        post(self.app, 'dashboard_mark_activities_old',
+                apikey=self.sara['apikey'])
+
+        # No email should be sent.
+        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
         disabled site-wide by a sysadmin.'''