mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-02 17:31:14 -05:00
Fix for missing template IDs.
- Problem was that on notification creation we pass the template ID not the template onto the new notification object - We then set the history object from this notification object by copying all the fields. This is OK at this point. - We then set the relationship on the history object based on the template, which we haven't passed in. We only passed the ID. This means that SQLAlchemy nulls the relationship, removing the template_id. - Later we update the history row when we send the message, this fixes the data. BUT if we ever have a send error, then this never happens and the template is never set on the history table. Fix: Set only the template ID when creating the history object.
This commit is contained in:
@@ -667,7 +667,6 @@ class NotificationHistory(db.Model):
|
|||||||
@classmethod
|
@classmethod
|
||||||
def from_notification(cls, notification):
|
def from_notification(cls, notification):
|
||||||
history = cls(**{c.name: getattr(notification, c.name) for c in cls.__table__.columns})
|
history = cls(**{c.name: getattr(notification, c.name) for c in cls.__table__.columns})
|
||||||
history.template = notification.template
|
|
||||||
return history
|
return history
|
||||||
|
|
||||||
def update_from_notification(self, notification):
|
def update_from_notification(self, notification):
|
||||||
|
|||||||
@@ -434,6 +434,7 @@ def sample_notification(notify_db,
|
|||||||
'job': job,
|
'job': job,
|
||||||
'service_id': service.id,
|
'service_id': service.id,
|
||||||
'service': service,
|
'service': service,
|
||||||
|
'template_id': template.id if template else None,
|
||||||
'template': template,
|
'template': template,
|
||||||
'template_version': template.version,
|
'template_version': template.version,
|
||||||
'status': status,
|
'status': status,
|
||||||
|
|||||||
@@ -42,17 +42,38 @@ def test_create_content_for_notification_fails_with_additional_personalisation(s
|
|||||||
|
|
||||||
|
|
||||||
@freeze_time("2016-01-01 11:09:00.061258")
|
@freeze_time("2016-01-01 11:09:00.061258")
|
||||||
def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key, mocker):
|
def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key, sample_job, mocker):
|
||||||
mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
|
mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
|
||||||
|
|
||||||
assert Notification.query.count() == 0
|
assert Notification.query.count() == 0
|
||||||
assert NotificationHistory.query.count() == 0
|
assert NotificationHistory.query.count() == 0
|
||||||
notification = persist_notification(sample_template.id, sample_template.version, '+447111111111',
|
notification = persist_notification(sample_template.id, sample_template.version, '+447111111111',
|
||||||
sample_template.service.id, {}, 'sms', sample_api_key.id,
|
sample_template.service.id, {}, 'sms', sample_api_key.id,
|
||||||
sample_api_key.key_type)
|
sample_api_key.key_type, job_id=sample_job.id,
|
||||||
assert Notification.query.count() == 1
|
job_row_number=100, reference="ref")
|
||||||
|
|
||||||
assert Notification.query.get(notification.id) is not None
|
assert Notification.query.get(notification.id) is not None
|
||||||
assert NotificationHistory.query.count() == 1
|
assert NotificationHistory.query.get(notification.id) is not None
|
||||||
|
|
||||||
|
notification_from_db = Notification.query.one()
|
||||||
|
notification_history_from_db = NotificationHistory.query.one()
|
||||||
|
|
||||||
|
assert notification_from_db.id == notification_history_from_db.id
|
||||||
|
assert notification_from_db.template_id == notification_history_from_db.template_id
|
||||||
|
assert notification_from_db.template_version == notification_history_from_db.template_version
|
||||||
|
assert notification_from_db.api_key_id == notification_history_from_db.api_key_id
|
||||||
|
assert notification_from_db.key_type == notification_history_from_db.key_type
|
||||||
|
assert notification_from_db.key_type == notification_history_from_db.key_type
|
||||||
|
assert notification_from_db.billable_units == notification_history_from_db.billable_units
|
||||||
|
assert notification_from_db.notification_type == notification_history_from_db.notification_type
|
||||||
|
assert notification_from_db.created_at == notification_history_from_db.created_at
|
||||||
|
assert not notification_from_db.sent_at
|
||||||
|
assert not notification_history_from_db.sent_at
|
||||||
|
assert notification_from_db.updated_at == notification_history_from_db.updated_at
|
||||||
|
assert notification_from_db.status == notification_history_from_db.status
|
||||||
|
assert notification_from_db.reference == notification_history_from_db.reference
|
||||||
|
assert notification_from_db.client_reference == notification_history_from_db.client_reference
|
||||||
|
|
||||||
mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count")
|
mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count")
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -3,8 +3,6 @@ import json
|
|||||||
|
|
||||||
from freezegun import freeze_time
|
from freezegun import freeze_time
|
||||||
|
|
||||||
from app import db
|
|
||||||
from app.models import TemplateStatistics
|
|
||||||
from tests import create_authorization_header
|
from tests import create_authorization_header
|
||||||
from tests.app.conftest import sample_template as create_sample_template, sample_template, sample_notification, \
|
from tests.app.conftest import sample_template as create_sample_template, sample_template, sample_notification, \
|
||||||
sample_email_template
|
sample_email_template
|
||||||
|
|||||||
Reference in New Issue
Block a user