Skip to content

Commit

Permalink
[#190] db migration to make Notification "to" field nullable
Browse files Browse the repository at this point in the history
if there is no "to" field, the notification can still be processed
  • Loading branch information
marisahoenig committed Oct 9, 2020
1 parent a6f11c9 commit c6aed2f
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 13 deletions.
2 changes: 1 addition & 1 deletion app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ class Notification(db.Model):
__tablename__ = 'notifications'

id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
to = db.Column(db.String, nullable=False)
to = db.Column(db.String, nullable=True)
normalised_to = db.Column(db.String, nullable=True)
job_id = db.Column(UUID(as_uuid=True), db.ForeignKey('jobs.id'), index=True, unique=False)
job = db.relationship('Job', backref=db.backref('notifications', lazy='dynamic'))
Expand Down
4 changes: 2 additions & 2 deletions app/notifications/process_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ def persist_notification(
billable_units=billable_units
)

if notification_type == SMS_TYPE:
if notification_type == SMS_TYPE and notification.to:
formatted_recipient = validate_and_format_phone_number(recipient, international=True)
recipient_info = get_international_phone_info(formatted_recipient)
notification.normalised_to = formatted_recipient
notification.international = recipient_info.international
notification.phone_prefix = recipient_info.country_prefix
notification.rate_multiplier = recipient_info.billable_units
elif notification_type == EMAIL_TYPE:
elif notification_type == EMAIL_TYPE and notification.to:
notification.normalised_to = format_email_address(notification.to)
elif notification_type == LETTER_TYPE:
notification.postage = postage or template_postage
Expand Down
20 changes: 12 additions & 8 deletions app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,19 @@ def post_notification(notification_type):


def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number']

send_to = validate_and_format_recipient(send_to=form_send_to,
key_type=api_key.key_type,
service=service,
notification_type=notification_type)

# Do not persist or send notification to the queue if it is a simulated recipient
simulated = simulated_recipient(send_to, notification_type)
if 'email_address' in form or 'phone_number' in form:
form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number']
send_to = validate_and_format_recipient(send_to=form_send_to,
key_type=api_key.key_type,
service=service,
notification_type=notification_type)
# Do not persist or send notification to the queue if it is a simulated recipient
simulated = simulated_recipient(send_to, notification_type)
else:
form_send_to = None
# assumption - if we are not passing in contact info then we aren't simulating the recipient - need to verify
simulated = None

personalisation = process_document_uploads(form.get('personalisation'), service, simulated=simulated)

Expand Down
19 changes: 19 additions & 0 deletions migrations/versions/0311_make_to_field_nullable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
"""
Revision ID: 0311_make_to_field_nullable
Revises: 0310a_make_email_from_nullable
Create Date: 2020-10-09 09:42:09.093272
"""
from alembic import op

revision = '0311_make_to_field_nullable'
down_revision = '0310a_make_email_from_nullable'


def upgrade():
op.alter_column('notifications', 'to', nullable=True)


def downgrade():
op.alter_column('notifications', 'to', nullable=False)
37 changes: 35 additions & 2 deletions tests/app/v2/notifications/test_post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
SCHEDULE_NOTIFICATIONS,
SMS_TYPE,
UPLOAD_DOCUMENT,
INTERNATIONAL_SMS_TYPE
)
INTERNATIONAL_SMS_TYPE,
VA_PROFILE_ID)
from flask import json, current_app

from app.models import Notification
Expand Down Expand Up @@ -368,6 +368,39 @@ def test_should_not_persist_or_send_notification_if_simulated_recipient(
assert Notification.query.count() == 0


@pytest.mark.parametrize('notification_type', [
(EMAIL_TYPE),
(SMS_TYPE),
])
def test_should_persist_notification_without_recipient(
client,
sample_template,
sample_email_template,
notification_type,
sample_email_template_with_placeholders,
mocker):
apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
data = {
"va_identifier": {
"id_type": VA_PROFILE_ID,
"value": "foo"
},
"template_id": sample_template.id if notification_type == SMS_TYPE else sample_email_template.id,
"personalisation": {"name": "Bob"}
}

auth_header = create_authorization_header(service_id=sample_email_template_with_placeholders.service_id)
response = client.post(
path='/v2/notifications/{}'.format(notification_type),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), auth_header])

assert response.status_code == 201
assert json.loads(response.get_data(as_text=True))["id"]
assert Notification.query.count() == 1
apply_async.assert_called()


@pytest.mark.parametrize("notification_type, key_send_to, send_to",
[("sms", "phone_number", "6502532222"),
("email", "email_address", "[email protected]")])
Expand Down

0 comments on commit c6aed2f

Please sign in to comment.