Properly handle already emailed courses.

lms/djangoapps/linkedin/management/commands/mailusers.py

@@ -3,7 +3,7 @@ Send emails to users inviting them to add their course certificates to their
 LinkedIn profiles.
 """
 
-from itertools import imap
+import json
 
 from django.core.management.base import BaseCommand
 from optparse import make_option
@@ -37,16 +37,23 @@ class Command(BaseCommand):
     def handle(self, *args, **options):
         grandfather = options.get('grandfather', False)
         accounts = LinkedIn.objects.filter(has_linkedin_account=True)
-        for user in imap(lambda account: account.user, accounts):   # lazy
+        for account in accounts:
+            emailed = json.loads(account.emailed_courses)
+            user = account.user
             certificates = GeneratedCertificate.objects.filter(user=user)
             certificates = certificates.filter(status='downloadable')
+            certificates = [cert for cert in certificates
+                            if cert.course_id not in emailed]
             if not certificates:
                 continue
             if grandfather:
                 send_grandfather_email(user, certificates)
+                emailed.extend([cert.course_id for cert in certificates])
             else:
                 for certificate in certificates:
                     send_email(user, certificate)
+                    emailed.append(certificate.course_id)
+            account.emailed_courses = json.dumps(emailed)
 
 
 def send_grandfather_email(user, certificates):

lms/djangoapps/linkedin/management/commands/tests/test_mailusers.py

@@ -1,6 +1,7 @@
 """
 Test email scripts.
 """
+import json
 import mock
 import unittest
 
@@ -17,11 +18,18 @@ class MailusersTests(unittest.TestCase):
     @mock.patch('linkedin.management.commands.mailusers.LinkedIn')
     def test_mail_users(self, linkedin, certificates, send_email):
         """
-        Test "happy path" for emailing users.
+        Test emailing users.
         """
         fut = mailusers.Command().handle
-        fred = mock.Mock(user=mock.Mock(certificates=[1, 2]))
-        barney = mock.Mock(user=mock.Mock(certificates=[3]))
+        cert1 = mock.Mock(course_id=1)
+        cert2 = mock.Mock(course_id=2)
+        cert3 = mock.Mock(course_id=3)
+        fred = mock.Mock(
+            emailed_courses="[]",
+            user=mock.Mock(certificates=[cert1, cert2]))
+        barney = mock.Mock(
+            emailed_courses="[]",
+            user=mock.Mock(certificates=[cert3]))
         linkedin.objects.filter.return_value = [fred, barney]
 
         def filter_user(user):
@@ -34,20 +42,29 @@ class MailusersTests(unittest.TestCase):
         fut()
         self.assertEqual(
             send_email.call_args_list,
-            [((fred.user, 1),),
-             ((fred.user, 2),),
-             ((barney.user, 3),)])
+            [((fred.user, cert1),),
+             ((fred.user, cert2),),
+             ((barney.user, cert3),)])
+        self.assertEqual(json.loads(fred.emailed_courses), [1, 2])
+        self.assertEqual(json.loads(barney.emailed_courses), [3])
 
     @mock.patch('linkedin.management.commands.mailusers.send_grandfather_email')
     @mock.patch('linkedin.management.commands.mailusers.GeneratedCertificate')
     @mock.patch('linkedin.management.commands.mailusers.LinkedIn')
     def test_mail_users_grandfather(self, linkedin, certificates, send_email):
         """
-        Test "happy path" for sending grandfather emails.
+        Test sending grandfather emails.
         """
         fut = mailusers.Command().handle
-        fred = mock.Mock(user=mock.Mock(certificates=[1, 2]))
-        barney = mock.Mock(user=mock.Mock(certificates=[3]))
+        cert1 = mock.Mock(course_id=1)
+        cert2 = mock.Mock(course_id=2)
+        cert3 = mock.Mock(course_id=3)
+        fred = mock.Mock(
+            emailed_courses="[]",
+            user=mock.Mock(certificates=[cert1, cert2]))
+        barney = mock.Mock(
+            emailed_courses="[]",
+            user=mock.Mock(certificates=[cert3]))
         linkedin.objects.filter.return_value = [fred, barney]
 
         def filter_user(user):
@@ -60,5 +77,43 @@ class MailusersTests(unittest.TestCase):
         fut(grandfather=True)
         self.assertEqual(
             send_email.call_args_list,
-            [((fred.user, [1, 2]),),
-             ((barney.user, [3]),)])
+            [((fred.user, [cert1, cert2]),),
+             ((barney.user, [cert3]),)])
+        self.assertEqual(json.loads(fred.emailed_courses), [1, 2])
+        self.assertEqual(json.loads(barney.emailed_courses), [3])
+
+    @mock.patch('linkedin.management.commands.mailusers.send_email')
+    @mock.patch('linkedin.management.commands.mailusers.GeneratedCertificate')
+    @mock.patch('linkedin.management.commands.mailusers.LinkedIn')
+    def test_mail_users_only_new_courses(self, linkedin, certificates,
+                                         send_email):
+        """
+        Test emailing users, making sure they are only emailed about new
+        certificates.
+        """
+        fut = mailusers.Command().handle
+        cert1 = mock.Mock(course_id=1)
+        cert2 = mock.Mock(course_id=2)
+        cert3 = mock.Mock(course_id=3)
+        fred = mock.Mock(
+            emailed_courses="[1]",
+            user=mock.Mock(certificates=[cert1, cert2]))
+        barney = mock.Mock(
+            emailed_courses="[]",
+            user=mock.Mock(certificates=[cert3]))
+        linkedin.objects.filter.return_value = [fred, barney]
+
+        def filter_user(user):
+            "Mock querying the database."
+            queryset = mock.Mock()
+            queryset.filter.return_value = user.certificates
+            return queryset
+
+        certificates.objects.filter = filter_user
+        fut()
+        self.assertEqual(
+            send_email.call_args_list,
+            [((fred.user, cert2),),
+             ((barney.user, cert3),)])
+        self.assertEqual(json.loads(fred.emailed_courses), [1, 2])
+        self.assertEqual(json.loads(barney.emailed_courses), [3])