From 3ffd6c744c7058dfe6cfa84ae5b1a8d878a6d4fa Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 20 Jul 2016 11:46:29 +0100 Subject: [PATCH] separate detailed and normal service_api_client.get_service to make it easier to mock and control return values --- app/main/views/dashboard.py | 9 ++++----- app/notify_client/service_api_client.py | 8 +++++++- app/statistics_utils.py | 6 +++--- app/templates/views/dashboard/_totals.html | 4 ++-- tests/__init__.py | 10 +++++----- tests/app/main/views/test_api_keys.py | 4 ++-- tests/app/main/views/test_dashboard.py | 7 ++++++- tests/conftest.py | 22 ++++++++++++++++------ 8 files changed, 45 insertions(+), 25 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 0f368e360..fcb2a0fdf 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -148,8 +148,7 @@ def get_dashboard_partials(service_id): lambda job: job['original_file_name'] != current_app.config['TEST_MESSAGE_FILENAME'], job_api_client.get_job(service_id, limit_days=7)['data'] )) - - service = service_api_client.get_service(service_id, detailed=True) + service = service_api_client.get_detailed_service(service_id) return { 'totals': render_template( @@ -178,10 +177,10 @@ def get_dashboard_partials(service_id): def get_dashboard_totals(service): - for msg_type in service['statistics']: + for msg_type in service['data']['statistics'].values(): msg_type['failed_percentage'] = get_formatted_percentage(msg_type['failed'], msg_type['requested']) - msg_type['show_warning'] = msg_type['failed_percentage'] > 3 - return service['statistics'] + msg_type['show_warning'] = float(msg_type['failed_percentage']) > 3 + return service['data']['statistics'] def calculate_usage(usage): diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index ad31d21d4..f2896a49e 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -41,7 +41,13 @@ class ServiceAPIClient(NotificationsAPIClient): data = _attach_current_user({}) return self.delete(endpoint, data) - def get_service(self, service_id, detailed=False): + def get_service(self, service_id): + return self._get_service(service_id, False) + + def get_detailed_service(self, service_id): + return self._get_service(service_id, True) + + def _get_service(self, service_id, detailed): """ Retrieve a service. """ diff --git a/app/statistics_utils.py b/app/statistics_utils.py index 122407a67..dc3a803a0 100644 --- a/app/statistics_utils.py +++ b/app/statistics_utils.py @@ -31,9 +31,9 @@ def sum_of_statistics(delivery_statistics): def add_rates_to(delivery_statistics): return dict( - email_failure_rate=get_formatted_percentage( - delivery_statistics['email_failed'], - delivery_statistics['email_requested']), + emails_failure_rate=get_formatted_percentage( + delivery_statistics['emails_failed'], + delivery_statistics['emails_requested']), sms_failure_rate=get_formatted_percentage( delivery_statistics['sms_failed'], delivery_statistics['sms_requested']), diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index ce5df5482..a5dcc2f51 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -9,8 +9,8 @@ statistics['email']['failed'], statistics['email']['failed_percentage'], statistics['email']['show_warning'], - failure_link=url_for(".view_notifications", service_id=id, message_type='email', status='failed'), - link=url_for(".view_notifications", service_id=id, message_type='email', status='sending,delivered,failed') + failure_link=url_for(".view_notifications", service_id=service_id, message_type='email', status='failed'), + link=url_for(".view_notifications", service_id=service_id, message_type='email', status='sending,delivered,failed') ) }}
diff --git a/tests/__init__.py b/tests/__init__.py index ac64b3848..7841a02e7 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -13,11 +13,11 @@ class TestClient(FlaskClient): session['user_id'] = user.id session['_fresh'] = True if mocker: - get_user_mock = mocker.patch('app.user_api_client.get_user', return_value=user) - create_event_mock = mocker.patch('app.events_api_client.create_event') - if service: - session['service_id'] = service['id'] - get_service_mock = mocker.patch('app.service_api_client.get_service', return_value={'data': service}) + mocker.patch('app.user_api_client.get_user', return_value=user) + mocker.patch('app.events_api_client.create_event') + if mocker and service: + session['service_id'] = service['id'] + mocker.patch('app.service_api_client.get_service', return_value={'data': service}) login_user(user, remember=True) def login_fresh(self): diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 1b2561566..543929478 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -6,14 +6,14 @@ from tests import validate_route_permission def test_should_show_empty_api_keys_page(app_, - api_user_active, + api_user_pending, mock_login, mock_get_no_api_keys, mock_get_service, mock_has_permissions): with app_.test_request_context(): with app_.test_client() as client: - client.login(api_user_active) + client.login(api_user_pending) service_id = str(uuid.uuid4()) response = client.get(url_for('main.api_keys', service_id=service_id)) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 661ee7ee6..ce4dca2b3 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -421,8 +421,13 @@ def test_service_dashboard_updates_gets_dashboard_totals(mocker, app_, active_user_with_permissions, service_one, + mock_get_user, + mock_get_service_templates, + mock_get_template_statistics, + mock_get_jobs, mock_get_service_statistics, - mock_get_usage): + mock_get_usage, + mock_get_detailed_service): dashboard_totals = mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={ 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} diff --git a/tests/conftest.py b/tests/conftest.py index b33844ba4..106924dfd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,20 +55,30 @@ def fake_uuid(): @pytest.fixture(scope='function') def mock_get_service(mocker, api_user_active): - def _get(service_id, detailed=False): + def _get(service_id): service = service_json( service_id, "Test Service", [api_user_active.id], message_limit=1000, active=False, restricted=True) - if detailed: - service['statistics'] = { - 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} - } return {'data': service} return mocker.patch('app.service_api_client.get_service', side_effect=_get) +@pytest.fixture(scope='function') +def mock_get_detailed_service(mocker, api_user_active): + def _get(service_id): + service = service_json( + service_id, "Test Service", [api_user_active.id], message_limit=1000, + active=False, restricted=True) + service['statistics'] = { + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + } + return {'data': service} + + return mocker.patch('app.service_api_client.get_detailed_service', side_effect=_get) + + @pytest.fixture(scope='function') def mock_get_live_service(mocker, api_user_active): def _get(service_id):