mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-03 01:41:05 -05:00
Merge pull request #560 from alphagov/notification-500
fix notification 500
This commit is contained in:
@@ -18,7 +18,12 @@ from app.dao.services_dao import (
|
||||
from app.dao.templates_dao import (dao_get_template_by_id)
|
||||
from app.dao.notifications_dao import get_notifications_for_job
|
||||
|
||||
from app.schemas import job_schema, unarchived_template_schema, notifications_filter_schema, notification_status_schema
|
||||
from app.schemas import (
|
||||
job_schema,
|
||||
unarchived_template_schema,
|
||||
notifications_filter_schema,
|
||||
notification_with_template_schema
|
||||
)
|
||||
|
||||
from app.celery.tasks import process_job
|
||||
|
||||
@@ -57,7 +62,7 @@ def get_all_notifications_for_service_job(service_id, job_id):
|
||||
kwargs['service_id'] = service_id
|
||||
kwargs['job_id'] = job_id
|
||||
return jsonify(
|
||||
notifications=notification_status_schema.dump(pagination.items, many=True).data,
|
||||
notifications=notification_with_template_schema.dump(pagination.items, many=True).data,
|
||||
page_size=page_size,
|
||||
total=pagination.total,
|
||||
links=pagination_links(
|
||||
|
||||
@@ -18,7 +18,7 @@ from app.dao import (
|
||||
services_dao,
|
||||
notifications_dao
|
||||
)
|
||||
from app.models import NOTIFICATION_TYPE, SMS_TYPE
|
||||
from app.models import SMS_TYPE
|
||||
from app.notifications.process_client_response import (
|
||||
validate_callback_data,
|
||||
process_sms_client_response
|
||||
@@ -26,7 +26,7 @@ from app.notifications.process_client_response import (
|
||||
from app.schemas import (
|
||||
email_notification_schema,
|
||||
sms_template_notification_schema,
|
||||
notification_status_schema,
|
||||
notification_with_personalisation_schema,
|
||||
notifications_filter_schema,
|
||||
notifications_statistics_schema,
|
||||
day_schema,
|
||||
@@ -175,7 +175,7 @@ def get_notifications(notification_id):
|
||||
notification = notifications_dao.get_notification(str(api_user.service_id),
|
||||
notification_id,
|
||||
key_type=api_user.key_type)
|
||||
return jsonify(data={"notification": notification_status_schema.dump(notification).data}), 200
|
||||
return jsonify(data={"notification": notification_with_personalisation_schema.dump(notification).data}), 200
|
||||
|
||||
|
||||
@notifications.route('/notifications', methods=['GET'])
|
||||
@@ -193,7 +193,7 @@ def get_all_notifications():
|
||||
limit_days=limit_days,
|
||||
key_type=api_user.key_type)
|
||||
return jsonify(
|
||||
notifications=notification_status_schema.dump(pagination.items, many=True).data,
|
||||
notifications=notification_with_personalisation_schema.dump(pagination.items, many=True).data,
|
||||
page_size=page_size,
|
||||
total=pagination.total,
|
||||
links=pagination_links(
|
||||
|
||||
@@ -144,7 +144,7 @@ class NotificationModelSchema(BaseSchema):
|
||||
class Meta:
|
||||
model = models.Notification
|
||||
strict = True
|
||||
exclude = ("_personalisation",)
|
||||
exclude = ('_personalisation', 'job', 'service', 'template', 'api_key', '')
|
||||
|
||||
|
||||
class BaseTemplateSchema(BaseSchema):
|
||||
@@ -280,17 +280,18 @@ class SmsAdminNotificationSchema(SmsNotificationSchema):
|
||||
content = fields.Str(required=True)
|
||||
|
||||
|
||||
class NotificationStatusSchema(BaseSchema):
|
||||
|
||||
template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content", "subject"], dump_only=True)
|
||||
job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True)
|
||||
personalisation = fields.Dict(required=False)
|
||||
|
||||
class NotificationWithTemplateSchema(BaseSchema):
|
||||
class Meta:
|
||||
model = models.Notification
|
||||
strict = True
|
||||
exclude = ('_personalisation',)
|
||||
|
||||
template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content", "subject"], dump_only=True)
|
||||
job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True)
|
||||
personalisation = fields.Dict(required=False)
|
||||
|
||||
|
||||
class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema):
|
||||
@pre_dump
|
||||
def handle_personalisation_property(self, in_data):
|
||||
self.personalisation = in_data.personalisation
|
||||
@@ -516,8 +517,10 @@ sms_template_notification_schema = SmsTemplateNotificationSchema()
|
||||
job_sms_template_notification_schema = JobSmsTemplateNotificationSchema()
|
||||
email_notification_schema = EmailNotificationSchema()
|
||||
job_email_template_notification_schema = JobEmailTemplateNotificationSchema()
|
||||
notification_status_schema = NotificationStatusSchema()
|
||||
notification_status_schema_load_json = NotificationStatusSchema(load_json=True)
|
||||
notification_schema = NotificationModelSchema()
|
||||
notification_with_template_schema = NotificationWithTemplateSchema()
|
||||
notification_with_personalisation_schema = NotificationWithPersonalisationSchema()
|
||||
notification_with_personalisation_schema_load_json = NotificationWithPersonalisationSchema(load_json=True)
|
||||
invited_user_schema = InvitedUserSchema()
|
||||
permission_schema = PermissionSchema()
|
||||
email_data_request_schema = EmailDataSchema()
|
||||
|
||||
@@ -33,7 +33,7 @@ from app.schemas import (
|
||||
user_schema,
|
||||
from_to_date_schema,
|
||||
permission_schema,
|
||||
notification_status_schema,
|
||||
notification_with_template_schema,
|
||||
notifications_filter_schema,
|
||||
detailed_service_schema
|
||||
)
|
||||
@@ -224,7 +224,7 @@ def get_all_notifications_for_service(service_id):
|
||||
kwargs = request.args.to_dict()
|
||||
kwargs['service_id'] = service_id
|
||||
return jsonify(
|
||||
notifications=notification_status_schema.dump(pagination.items, many=True).data,
|
||||
notifications=notification_with_template_schema.dump(pagination.items, many=True).data,
|
||||
page_size=page_size,
|
||||
total=pagination.total,
|
||||
links=pagination_links(
|
||||
|
||||
@@ -10,7 +10,7 @@ api_spec = APISpec(
|
||||
version='0.0.0'
|
||||
)
|
||||
|
||||
api_spec.definition('NotificationStatusSchema', properties={
|
||||
api_spec.definition('NotificationWithTemplateSchema', properties={
|
||||
"content_char_count": {
|
||||
"format": "int32",
|
||||
"type": "integer"
|
||||
@@ -106,14 +106,14 @@ api_spec.definition('NotificationStatusSchema', properties={
|
||||
})
|
||||
api_spec.definition('NotificationSchema', properties={
|
||||
"notification": {
|
||||
"$ref": "#/definitions/NotificationStatusSchema"
|
||||
"$ref": "#/definitions/NotificationWithTemplateSchema"
|
||||
}
|
||||
})
|
||||
api_spec.definition('NotificationsSchema', properties={
|
||||
"notifications": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"$ref": "#/definitions/NotificationStatusSchema"
|
||||
"$ref": "#/definitions/NotificationWithTemplateSchema"
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@@ -11,6 +11,7 @@ from app.dao.api_key_dao import save_model_api_key
|
||||
from app.models import ApiKey, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST
|
||||
from tests import create_authorization_header
|
||||
from tests.app.conftest import sample_notification as create_sample_notification
|
||||
from notifications_utils.template import NeededByTemplateError
|
||||
|
||||
|
||||
def test_get_sms_notification_by_id(notify_api, sample_notification):
|
||||
@@ -602,10 +603,10 @@ def test_get_notifications_for_service_returns_merged_template_content(notify_ap
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
|
||||
auth_header = create_authorization_header()
|
||||
auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id)
|
||||
|
||||
response = client.get(
|
||||
path='/service/{}/notifications'.format(sample_template_with_placeholders.service.id),
|
||||
path='/notifications',
|
||||
headers=[auth_header])
|
||||
assert response.status_code == 200
|
||||
|
||||
@@ -615,6 +616,40 @@ def test_get_notifications_for_service_returns_merged_template_content(notify_ap
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=True, raises=NeededByTemplateError)
|
||||
def test_get_notification_selects_correct_template_for_personalisation(notify_api,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_template):
|
||||
|
||||
create_sample_notification(notify_db,
|
||||
notify_db_session,
|
||||
service=sample_template.service,
|
||||
template=sample_template)
|
||||
|
||||
sample_template.content = '((name))'
|
||||
notify_db.session.commit()
|
||||
|
||||
create_sample_notification(notify_db,
|
||||
notify_db_session,
|
||||
service=sample_template.service,
|
||||
template=sample_template,
|
||||
personalisation={"name": "foo"})
|
||||
|
||||
with notify_api.test_request_context(), notify_api.test_client() as client:
|
||||
auth_header = create_authorization_header(service_id=sample_template.service_id)
|
||||
|
||||
response = client.get(path='/notifications', headers=[auth_header])
|
||||
assert response.status_code == 200
|
||||
|
||||
resp = json.loads(response.get_data(as_text=True))
|
||||
assert len(resp['notifications']) == 2
|
||||
assert resp['notifications'][0]['template_version'] == 1
|
||||
assert resp['notifications'][0]['body'] == 'This is a template'
|
||||
assert resp['notifications'][1]['template_version'] == 2
|
||||
assert resp['notifications'][1]['body'] == 'foo'
|
||||
|
||||
|
||||
def _create_auth_header_from_key(api_key):
|
||||
token = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id))
|
||||
return [('Authorization', 'Bearer {}'.format(token))]
|
||||
|
||||
Reference in New Issue
Block a user