Repository: python/pythondotorg
Issue: Submissions emails should only be sent for new submissions

While #656 fixed the number of emails we're getting, the problem of getting approval emails for every single change of a record still isn't fixed.

We should really only be getting emails with the subject "Job Submitted for Approval" for new submissions.

It may make sense to also get emails for changes to existing records, but those should then be using a different subject, e.g. "Job Edited", but I'm not sure whether that's really needed.


Changed files:
jobs/tests/test_views.py, jobs/tests/test_models.py, jobs/listeners.py

diff --git a/jobs/listeners.py b/jobs/listeners.py
index 8fdbce0..ed4cc11 100644
--- a/jobs/listeners.py
+++ b/jobs/listeners.py
@@ -96,7 +96,7 @@ def on_job_was_submitted(sender, instance, **kwargs):
 
     """
     Job = models.get_model('jobs', 'Job')
-    if instance.status != Job.STATUS_REVIEW:
+    if instance.status != Job.STATUS_REVIEW or not kwargs.get('created'):
         return
 
     subject_template = loader.get_template('jobs/email/job_was_submitted_subject.txt')
diff --git a/jobs/tests/test_models.py b/jobs/tests/test_models.py
index 6aebc9c..13080c3 100644
--- a/jobs/tests/test_models.py
+++ b/jobs/tests/test_models.py
@@ -9,12 +9,6 @@ from ..models import Job, JobType, JobCategory
 
 
 class JobsModelsTests(TestCase):
-    def setUp(self):
-        self.job = factories.JobFactory(
-            city='Memphis',
-            region='TN',
-            country='USA',
-        )
 
     def create_job(self, **kwargs):
         job_kwargs = {
@@ -28,15 +22,17 @@ class JobsModelsTests(TestCase):
         return job
 
     def test_is_new(self):
-        self.assertTrue(self.job.is_new)
+        job = self.create_job()
+        self.assertTrue(job.is_new)
 
         threshold = Job.NEW_THRESHOLD
 
-        self.job.created = timezone.now() - (threshold * 2)
-        self.assertFalse(self.job.is_new)
+        job.created = timezone.now() - (threshold * 2)
+        self.assertFalse(job.is_new)
 
     def test_location_slug(self):
-        self.assertEqual(self.job.location_slug, 'memphis-tn-usa')
+        job = self.create_job()
+        self.assertEqual(job.location_slug, 'memphis-tn-usa')
 
     def test_approved_manager(self):
         self.assertEqual(Job.objects.approved().count(), 0)
@@ -69,9 +65,9 @@ class JobsModelsTests(TestCase):
         self.assertEqual(Job.objects.removed().count(), 1)
 
     def test_review_manager(self):
-        self.assertEqual(Job.objects.review().count(), 1)
+        self.assertEqual(Job.objects.review().count(), 0)
         factories.ReviewJobFactory()
-        self.assertEqual(Job.objects.review().count(), 2)
+        self.assertEqual(Job.objects.review().count(), 1)
 
     def test_visible_manager(self):
         j1 = factories.ApprovedJobFactory()
@@ -137,16 +133,3 @@ class JobsModelsTests(TestCase):
 
         job2 = self.create_job(region='')
         self.assertEqual(job2.display_location, 'Memphis, USA')
-
-    def test_email_on_submit(self):
-        job = self.create_job()
-        mail.outbox = []
-        self.assertEqual(0, len(mail.outbox))
-        job.status = Job.STATUS_REVIEW
-        job.save()
-        self.assertEqual(1, len(mail.outbox))
-        job.status = Job.STATUS_DRAFT
-        job.save()
-        self.assertEqual(1, len(mail.outbox))
-        expected_subject = "Job Submitted for Approval: {}".format(job.display_name)
-        self.assertEqual(mail.outbox[0].subject, expected_subject)
diff --git a/jobs/tests/test_views.py b/jobs/tests/test_views.py
index 1ab9b06..edf81b9 100644
--- a/jobs/tests/test_views.py
+++ b/jobs/tests/test_views.py
@@ -168,6 +168,7 @@ class JobsViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
 
     def test_job_create(self):
+        mail.outbox = []
         url = reverse('jobs:job_create')
         response = self.client.get(url)
 
@@ -190,6 +191,7 @@ class JobsViewTests(TestCase):
         # First test job from anonymous non-logged in user
         response = self.client.post(url, post_data)
         self.assertEqual(response.status_code, 302)
+        self.assertEqual(len(mail.outbox), 1)
 
         jobs = Job.objects.filter(company_name='Some Company')
         self.assertEqual(len(jobs), 1)
@@ -197,8 +199,11 @@ class JobsViewTests(TestCase):
         job = jobs[0]
         self.assertNotEqual(job.created, None)
         self.assertNotEqual(job.updated, None)
-
         self.assertEqual(job.status, 'review')
+        self.assertEqual(
+            mail.outbox[0].subject,
+            "Job Submitted for Approval: {}".format(job.display_name)
+        )
 
         post_data['company_name'] = 'Other Studio'
 
@@ -211,6 +216,7 @@ class JobsViewTests(TestCase):
         self.client.login(username=creator.username, password='secret')
         response = self.client.post(url, post_data)
         self.assertEqual(response.status_code, 302)
+        self.assertEqual(len(mail.outbox), 2)
 
         jobs = Job.objects.filter(company_name='Other Studio')
         self.assertEqual(len(jobs), 1)
@@ -220,6 +226,10 @@ class JobsViewTests(TestCase):
         self.assertNotEqual(job.updated, None)
         self.assertEqual(job.creator, creator)
         self.assertEqual(job.status, 'review')
+        self.assertEqual(
+            mail.outbox[1].subject,
+            "Job Submitted for Approval: {}".format(job.display_name)
+        )
 
     def test_job_create_prepopulate_email(self):
         create_url = reverse('jobs:job_create')