From 549a17c1f7e2ea845a20827d401ea7b9fa5e9dbb Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 22 Sep 2016 09:53:33 +0100 Subject: [PATCH] separate get_job and get_jobs in job_api_client also remove the status parameter, as we never use it anywhere --- app/main/views/dashboard.py | 2 +- app/main/views/jobs.py | 2 +- app/notify_client/job_api_client.py | 22 +++++++------- app/statistics_utils.py | 4 +-- tests/app/notify_client/test_job_client.py | 35 ++-------------------- tests/conftest.py | 2 +- 6 files changed, 16 insertions(+), 51 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index def963624..c243f02ba 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -123,7 +123,7 @@ def get_dashboard_partials(service_id): ) jobs = add_rate_to_jobs([ - job for job in job_api_client.get_job(service_id, limit_days=7)['data'] + job for job in job_api_client.get_jobs(service_id, limit_days=7)['data'] if job['original_file_name'] != current_app.config['TEST_MESSAGE_FILENAME'] ]) scheduled_jobs = sorted([ diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 756394bcc..2c4d3b798 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -68,7 +68,7 @@ def view_jobs(service_id): return render_template( 'views/jobs/jobs.html', jobs=add_rate_to_jobs([ - job for job in job_api_client.get_job(service_id)['data'] + job for job in job_api_client.get_jobs(service_id)['data'] if job['job_status'] not in ['scheduled', 'cancelled'] ]) ) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 614a01560..b9d78f377 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -26,21 +26,19 @@ class JobApiClient(BaseAPIClient): results['requested'] += outcome['count'] return results - def get_job(self, service_id, job_id=None, limit_days=None, status=None): - if job_id: - params = {} - if status is not None: - params['status'] = status - job = self.get(url='/service/{}/job/{}'.format(service_id, job_id), params=params) + def get_job(self, service_id, job_id): + params = {} + job = self.get(url='/service/{}/job/{}'.format(service_id, job_id), params=params) - stats = self.__convert_statistics(job['data']) - job['data']['notifications_sent'] = stats['delivered'] + stats['failed'] - job['data']['notifications_delivered'] = stats['delivered'] - job['data']['notifications_failed'] = stats['failed'] - job['data']['notifications_requested'] = stats['requested'] + stats = self.__convert_statistics(job['data']) + job['data']['notifications_sent'] = stats['delivered'] + stats['failed'] + job['data']['notifications_delivered'] = stats['delivered'] + job['data']['notifications_failed'] = stats['failed'] + job['data']['notifications_requested'] = stats['requested'] - return job + return job + def get_jobs(self, service_id, limit_days=None): params = {} if limit_days is not None: params['limit_days'] = limit_days diff --git a/app/statistics_utils.py b/app/statistics_utils.py index 4581e069d..1902c6d6d 100644 --- a/app/statistics_utils.py +++ b/app/statistics_utils.py @@ -74,9 +74,7 @@ def statistics_by_state(statistics): def get_failure_rate_for_job(job): if not job.get('notifications_delivered'): - if job.get('notifications_failed'): - return 1 - return 0 + return 1 if job.get('notifications_failed') else 0 return ( job.get('notifications_failed', 0) / (job.get('notifications_failed', 0) + job.get('notifications_delivered', 0)) diff --git a/tests/app/notify_client/test_job_client.py b/tests/app/notify_client/test_job_client.py index fe70f5809..0e2bbee27 100644 --- a/tests/app/notify_client/test_job_client.py +++ b/tests/app/notify_client/test_job_client.py @@ -66,37 +66,6 @@ def test_client_gets_job_by_service_and_job(mocker): mock_get.assert_called_once_with(url=expected_url, params={}) -def test_client_gets_job_by_service_and_job_filtered_by_status(mocker): - mocker.patch('app.notify_client.current_user', id='1') - - service_id = 'service_id' - job_id = 'job_id' - - expected_url = '/service/{}/job/{}'.format(service_id, job_id) - - client = JobApiClient() - mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get') - - client.get_job(service_id, job_id, limit_days=1, status='failed') - - mock_get.assert_called_once_with(url=expected_url, params={'status': 'failed'}) - - -def test_client_gets_job_by_service_filtered_by_status(mocker): - mocker.patch('app.notify_client.current_user', id='1') - - service_id = 'service_id' - - expected_url = '/service/{}/job'.format(service_id) - - client = JobApiClient() - mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get') - - client.get_job(service_id, limit_days=1, status='failed') - - mock_get.assert_called_once_with(url=expected_url, params={'limit_days': 1}) - - def test_client_parses_job_stats(mocker): mocker.patch('app.notify_client.current_user', id='1') @@ -247,7 +216,7 @@ def test_client_parses_job_stats_for_service(mocker): client = JobApiClient() mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get', return_value=expected_data) - result = client.get_job(service_id) + result = client.get_jobs(service_id) mock_get.assert_called_once_with(url=expected_url, params={}) assert result['data'][0]['id'] == job_1_id @@ -309,7 +278,7 @@ def test_client_parses_empty_job_stats_for_service(mocker): client = JobApiClient() mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get', return_value=expected_data) - result = client.get_job(service_id) + result = client.get_jobs(service_id) mock_get.assert_called_once_with(url=expected_url, params={}) assert result['data'][0]['id'] == job_1_id diff --git a/tests/conftest.py b/tests/conftest.py index 7ec1ba9f2..5ab5ee18a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -923,7 +923,7 @@ def mock_get_jobs(mocker, api_user_active): ) ]} - return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs) + return mocker.patch('app.job_api_client.get_jobs', side_effect=_get_jobs) @pytest.fixture(scope='function')