From 02db3be37c8ffd986baef3b946efc5068676d50e Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Mon, 22 May 2017 10:12:18 +0100 Subject: [PATCH] General refactor --- app/celery/tasks.py | 25 +++++++++++-------------- tests/app/celery/test_tasks.py | 16 +++++++++++++--- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 48923d228..cce6b05f0 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -1,5 +1,3 @@ -import random - from datetime import (datetime) from collections import namedtuple @@ -361,21 +359,20 @@ def get_template_class(template_type): @statsd(namespace="tasks") def update_letter_notifications_statuses(self, filename): bucket_location = '{}-ftp'.format(current_app.config['NOTIFY_EMAIL_DOMAIN']) - response_file = s3.get_s3_file(bucket_location, filename) + response_file_content = s3.get_s3_file(bucket_location, filename) try: - NotificationUpdate = namedtuple('NotificationUpdate', ['reference', 'status', 'page_count', 'cost_threshold']) - notification_updates = [NotificationUpdate(*line.split('|')) for line in response_file.splitlines()] - + notification_updates = process_updates_from_file(response_file_content) except TypeError: current_app.logger.exception('DVLA response file: {} has an invalid format'.format(filename)) raise - else: - if notification_updates: - for update in notification_updates: - current_app.logger.info('DVLA update: {}'.format(str(update))) - # TODO: Update notifications with desired status - return notification_updates - else: - current_app.logger.exception('DVLA response file contained no updates') + for update in notification_updates: + current_app.logger.info('DVLA update: {}'.format(str(update))) + # TODO: Update notifications with desired status + + +def process_updates_from_file(response_file): + NotificationUpdate = namedtuple('NotificationUpdate', ['reference', 'status', 'page_count', 'cost_threshold']) + notification_updates = [NotificationUpdate(*line.split('|')) for line in response_file.splitlines()] + return notification_updates diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 560f68c27..27c9833a4 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -24,7 +24,8 @@ from app.celery.tasks import ( persist_letter, get_template_class, update_job_to_sent_to_dvla, - update_letter_notifications_statuses + update_letter_notifications_statuses, + process_updates_from_file ) from app.dao import jobs_dao, services_dao from app.models import ( @@ -1094,10 +1095,19 @@ def test_update_letter_notifications_statuses_calls_with_correct_bucket_location s3_mock.assert_called_with('{}-ftp'.format(current_app.config['NOTIFY_EMAIL_DOMAIN']), 'foo.txt') -def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mocker): +def test_update_letter_notifications_statuses_builds_updates_from_content(notify_api, mocker): valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) - updates = update_letter_notifications_statuses(filename='foo.txt') + update_mock = mocker.patch('app.celery.tasks.process_updates_from_file') + + update_letter_notifications_statuses(filename='foo.txt') + + update_mock.assert_called_with('ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted') + + +def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mocker): + valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' + updates = process_updates_from_file(valid_file) assert len(updates) == 2