Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixes bug 814647 - Added a test to verify an email cannot be sent twi…

…ce, fixed a few issues.
  • Loading branch information...
commit 47f870b6e0ea57a9d6997371ab32637b55b8911a 1 parent bb7a862
@AdrianGaudebert AdrianGaudebert authored
View
30 socorro/cron/jobs/automatic_emails.py
@@ -8,6 +8,11 @@
from socorro.cron.base import PostgresBackfillCronApp
from socorro.external.exacttarget import exacttarget
+from socorro.lib.datetimeutil import utc_now
+
+
+def string_to_list(input_str):
+ return [x.strip() for x in input_str.split(',') if x.strip()]
SQL_REPORTS = """
@@ -17,8 +22,9 @@
WHERE r.date_processed > %(start_date)s
AND r.date_processed < %(end_date)s
AND r.email IS NOT NULL
- AND e.last_sending < %(delayed_date)s
+ AND (e.last_sending < %(delayed_date)s OR e.last_sending IS NULL)
AND r.product IN %(products)s
+ ORDER BY r.email
"""
@@ -34,6 +40,13 @@
"""
+SQL_INSERT = """
+ INSERT INTO emails
+ (email, last_sending)
+ VALUES (%(email)s, %(last_sending)s)
+"""
+
+
class AutomaticEmailsCronApp(PostgresBackfillCronApp):
"""Send an email to every user that crashes and gives us his or her email
address. """
@@ -51,7 +64,8 @@ class AutomaticEmailsCronApp(PostgresBackfillCronApp):
required_config.add_option(
'restrict_products',
default=['Firefox'],
- doc='List of products for which to send an email. '
+ doc='List of products for which to send an email. ',
+ from_string_converter=string_to_list
)
required_config.add_option(
'exacttarget_user',
@@ -65,7 +79,7 @@ class AutomaticEmailsCronApp(PostgresBackfillCronApp):
)
required_config.add_option(
'email_template',
- default='socorro_dev_test',
+ default='',
doc='Name of the email template to use in ExactTarget. '
)
required_config.add_option(
@@ -103,7 +117,7 @@ def run(self, connection, run_datetime):
for row in cursor.fetchall():
report = dict(zip(SQL_FIELDS, row))
self.send_email(report)
- self.update_user(report, run_datetime, connection)
+ self.update_user(report, utc_now(), connection)
def send_email(self, report):
list_service = self.email_service.list()
@@ -115,9 +129,9 @@ def send_email(self, report):
subscriber = list_service.get_subscriber(
report['email'],
None,
- ['SubscriberKey']
+ ['token']
)
- subscriber_key = subscriber.SubscriberKey
+ subscriber_key = subscriber['token']
except exacttarget.NewsletterException:
# subscriber does not exist, let's give it an ID
subscriber_key = report['email']
@@ -136,3 +150,7 @@ def update_user(self, report, sending_datetime, connection):
'last_sending': sending_datetime
}
cursor.execute(SQL_UPDATE, sql_params)
+ if cursor.rowcount == 0:
+ # This email address is not known yet, insert it
+ cursor.execute(SQL_INSERT, sql_params)
+ connection.commit()
View
23 socorro/unittest/cron/base.py
@@ -2,6 +2,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+import datetime
import os
import shutil
import tempfile
@@ -62,6 +63,28 @@ def _setup_config_manager(self, jobs_string, extra_value_source=None):
)
return config_manager, json_file
+ def _wind_clock(self, json_file, days=0, hours=0, seconds=0):
+ # note that 'hours' and 'seconds' can be negative numbers
+ if days:
+ hours += days * 24
+ if hours:
+ seconds += hours * 60 * 60
+
+ # modify ALL last_run and next_run to pretend time has changed
+ db = crontabber.JSONJobDatabase()
+ db.load(json_file)
+
+ def _wind(data):
+ for key, value in data.items():
+ if isinstance(value, dict):
+ _wind(value)
+ else:
+ if isinstance(value, datetime.datetime):
+ data[key] = value - datetime.timedelta(seconds=seconds)
+
+ _wind(db)
+ db.save(json_file)
+
class IntegrationTestCaseBase(TestCaseBase):
"""Useful class for running integration tests related to crontabber apps
View
180 socorro/unittest/cron/jobs/test_automatic_emails.py
@@ -39,7 +39,7 @@ def setUp(self):
'%(now)s'
), (
'2',
- 'someoneelse@example.com',
+ 'quidam@example.com',
'WaterWolf',
'20.0',
'Release',
@@ -51,6 +51,41 @@ def setUp(self):
'20.0',
'Release',
'%(now)s'
+ ), (
+ '4',
+ 'a@example.org',
+ 'NightlyTrain',
+ '1.0',
+ 'Nightly',
+ '%(now)s'
+ ), (
+ '5',
+ 'b@example.org',
+ 'NightlyTrain',
+ '1.0',
+ 'Nightly',
+ '%(now)s'
+ ), (
+ '6',
+ 'c@example.org',
+ 'NightlyTrain',
+ '1.0',
+ 'Nightly',
+ '%(now)s'
+ ), (
+ '7',
+ 'd@example.org',
+ 'NightlyTrain',
+ '1.0',
+ 'Nightly',
+ '%(now)s'
+ ), (
+ '8',
+ 'e@example.org',
+ 'NightlyTrain',
+ '1.0',
+ 'Nightly',
+ '%(now)s'
)
""" % {'now': now})
@@ -109,10 +144,7 @@ def setUp(self):
'someone@example.com',
'%(last_month)s'
), (
- 'someoneelse@example.com',
- '%(last_month)s'
- ), (
- 'anotherone@example.com',
+ 'quidam@example.com',
'%(last_month)s'
), (
'menime@example.com',
@@ -167,6 +199,7 @@ def _setup_simple_config(self):
'exacttarget_user': '',
'exacttarget_password': '',
'restrict_products': ['WaterWolf'],
+ 'email_template': 'socorro_dev_test'
}]
)
@@ -178,7 +211,8 @@ def _setup_test_mode_config(self):
'exacttarget_user': '',
'exacttarget_password': '',
'restrict_products': ['WaterWolf'],
- 'test_mode': True
+ 'test_mode': True,
+ 'email_template': 'socorro_dev_test'
}]
)
@@ -205,13 +239,33 @@ def test_cron_job(self, exacttarget_mock):
# Verify the last call to trigger_send
fields = {
- 'EMAIL_ADDRESS_': 'anotherone@example.com',
+ 'EMAIL_ADDRESS_': 'someone@example.com',
'EMAIL_FORMAT_': 'H',
- 'TOKEN': 'anotherone@example.com'
+ 'TOKEN': 'someone@example.com'
}
et_mock.trigger_send.assert_called_with('socorro_dev_test', fields)
+ # Verify that user's data was updated
+ cursor = self.conn.cursor()
+ emails_list = (
+ 'someone@example.com',
+ 'quidam@example.com',
+ 'anotherone@example.com'
+ )
+ sql = """
+ SELECT last_sending
+ FROM emails
+ WHERE email IN %s
+ """ % (emails_list,)
+ cursor.execute(sql)
+ self.assertEqual(cursor.rowcount, 3)
+ now = utc_now()
+ for row in cursor.fetchall():
+ self.assertEqual(row[0].year, now.year)
+ self.assertEqual(row[0].month, now.month)
+ self.assertEqual(row[0].day, now.day)
+
@mock.patch('socorro.external.exacttarget.exacttarget.ExactTarget')
def test_run(self, exacttarget_mock):
config_manager = self._setup_simple_config()
@@ -225,8 +279,9 @@ def test_run(self, exacttarget_mock):
@mock.patch('socorro.external.exacttarget.exacttarget.ExactTarget')
def test_send_email(self, exacttarget_mock):
list_service_mock = exacttarget_mock.return_value.list.return_value
- subscriber = list_service_mock.get_subscriber.return_value
- subscriber.SubscriberKey = 'fake@example.com'
+ list_service_mock.get_subscriber.return_value = {
+ 'token': 'fake@example.com'
+ }
config_manager = self._setup_simple_config()
with config_manager.context() as config:
@@ -253,8 +308,9 @@ def test_send_email(self, exacttarget_mock):
@mock.patch('socorro.external.exacttarget.exacttarget.ExactTarget')
def test_send_email_test_mode(self, exacttarget_mock):
list_service_mock = exacttarget_mock.return_value.list.return_value
- subscriber = list_service_mock.get_subscriber.return_value
- subscriber.SubscriberKey = 'fake@example.com'
+ list_service_mock.get_subscriber.return_value = {
+ 'token': 'fake@example.com'
+ }
config_manager = self._setup_test_mode_config()
with config_manager.context() as config:
@@ -282,11 +338,11 @@ def test_update_user(self):
config_manager = self._setup_simple_config()
with config_manager.context() as config:
job = automatic_emails.AutomaticEmailsCronApp(config, '')
+ now = utc_now()
report = {
'email': 'someone@example.com'
}
- now = utc_now()
job.update_user(report, now, self.conn)
cursor = self.conn.cursor()
@@ -297,3 +353,101 @@ def test_update_user(self):
self.assertEqual(cursor.rowcount, 1)
row = cursor.fetchone()
self.assertEqual(row[0], now)
+
+ # Test with a non-existing user
+ report = {
+ 'email': 'idonotexist@example.com'
+ }
+ job.update_user(report, now, self.conn)
+
+ cursor = self.conn.cursor()
+ cursor.execute("""
+ SELECT last_sending FROM emails WHERE email=%(email)s
+ """, report)
+
+ self.assertEqual(cursor.rowcount, 1)
+ row = cursor.fetchone()
+ self.assertEqual(row[0], now)
+
+ @mock.patch('socorro.external.exacttarget.exacttarget.ExactTarget')
+ def test_email_cannot_be_sent_twice(self, exacttarget_mock):
+ (config_manager, json_file) = self._setup_config_manager(
+ restrict_products=['NightlyTrain']
+ )
+ et_mock = exacttarget_mock.return_value
+
+ # Prepare failures
+ _failures = []
+ _email_sent = []
+
+ class SomeRandomError(Exception):
+ pass
+
+ def trigger_send(template, fields):
+ email = fields['EMAIL_ADDRESS_']
+ if email == 'c@example.org' and email not in _failures:
+ _failures.append(email)
+ raise SomeRandomError('This is an error. ')
+ else:
+ _email_sent.append(email)
+
+ et_mock.trigger_send = trigger_send
+
+ with config_manager.context() as config:
+ tab = crontabber.CronTabber(config)
+ tab.run_all()
+
+ information = json.load(open(json_file))
+ assert information['automatic-emails']
+ assert information['automatic-emails']['last_error']
+ self.assertEqual(
+ information['automatic-emails']['last_error']['type'],
+ str(SomeRandomError)
+ )
+
+ # Verify that user's data was updated, but not all of it
+ self.assertEqual(_email_sent, ['a@example.org', 'b@example.org'])
+ cursor = self.conn.cursor()
+ emails_list = (
+ 'a@example.org',
+ 'b@example.org',
+ 'c@example.org',
+ 'd@example.org',
+ 'e@example.org'
+ )
+ sql = """
+ SELECT email, last_sending
+ FROM emails
+ WHERE email IN %s
+ """ % (emails_list,)
+ cursor.execute(sql)
+ now = utc_now()
+ self.assertEqual(cursor.rowcount, 2)
+ for row in cursor.fetchall():
+ assert row[0] in ('a@example.org', 'b@example.org')
+ self.assertEqual(row[1].year, now.year)
+ self.assertEqual(row[1].month, now.month)
+ self.assertEqual(row[1].day, now.day)
+
+ # Run crontabber again and verify that all users are updated,
+ # and emails are not sent twice
+ self._wind_clock(json_file, hours=1)
+
+ # This forces a crontabber instance to reload the JSON file
+ tab._database = None
+
+ tab.run_all()
+
+ information = json.load(open(json_file))
+ assert information['automatic-emails']
+ assert not information['automatic-emails']['last_error']
+ assert information['automatic-emails']['last_success']
+
+ # Verify that users were not sent an email twice
+ self.assertEqual(_email_sent, [
+ 'a@example.org',
+ 'b@example.org',
+ 'c@example.org',
+ 'd@example.org',
+ 'e@example.org'
+ ])
View
32 socorro/unittest/cron/test_crontabber.py
@@ -21,33 +21,7 @@
#==============================================================================
-class CrontabberTestCaseBase(TestCaseBase):
-
- def _wind_clock(self, json_file, days=0, hours=0, seconds=0):
- # note that 'hours' and 'seconds' can be negative numbers
- if days:
- hours += days * 24
- if hours:
- seconds += hours * 60 * 60
-
- # modify ALL last_run and next_run to pretend time has changed
- db = crontabber.JSONJobDatabase()
- db.load(json_file)
-
- def _wind(data):
- for key, value in data.items():
- if isinstance(value, dict):
- _wind(value)
- else:
- if isinstance(value, datetime.datetime):
- data[key] = value - datetime.timedelta(seconds=seconds)
-
- _wind(db)
- db.save(json_file)
-
-
-#==============================================================================
-class TestJSONJobsDatabase(CrontabberTestCaseBase):
+class TestJSONJobsDatabase(TestCaseBase):
"""This has nothing to do with Socorro actually. It's just tests for the
underlying JSON database.
"""
@@ -123,7 +97,7 @@ def test_loading_broken_json(self):
#==============================================================================
-class TestCrontabber(CrontabberTestCaseBase):
+class TestCrontabber(TestCaseBase):
def setUp(self):
super(TestCrontabber, self).setUp()
@@ -1260,7 +1234,7 @@ def mock_utc_now():
#==============================================================================
@attr(integration='postgres') # for nosetests
-class TestFunctionalCrontabber(CrontabberTestCaseBase):
+class TestFunctionalCrontabber(TestCaseBase):
def setUp(self):
super(TestFunctionalCrontabber, self).setUp()
View
16 sql/upgrade/34/README.rst
@@ -1,16 +0,0 @@
-.. This Source Code Form is subject to the terms of the Mozilla Public
-.. License, v. 2.0. If a copy of the MPL was not distributed with this
-.. file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-34.0 Database Updates
-=====================
-
-This batch makes the following database changes:
-
-bug #814647
- Send an email to every user that crashes
-
-...
-
-The above changes should take only a few minutes to deploy.
-This upgrade does not require a downtime.
View
11 sql/upgrade/34/emails.sql
@@ -1,11 +0,0 @@
-\set ON_ERROR_STOP 1
-
--- create new table for emails
-
-SELECT create_table_if_not_exists('emails',
-$t$
-CREATE TABLE emails (
- email citext not null,
- last_sending timestamp with time zone,
- constraint emails_key primary key ( email )
-);$t$, 'breakpad_rw' );
View
22 sql/upgrade/34/upgrade.sh
@@ -1,22 +0,0 @@
-#!/bin/bash
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#please see README
-
-set -e
-
-CURDIR=$(dirname $0)
-DBNAME=$1
-: ${DBNAME:="breakpad"}
-VERSION=34.0
-
-echo '*********************************************************'
-echo 'Add emails table'
-echo 'bug 814647'
-psql -f ${CURDIR}/emails.sql $DBNAME
-
-echo "$VERSION upgrade done"
-
-exit 0
Please sign in to comment.
Something went wrong with that request. Please try again.