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

@@ -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([

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']
         ])
     )

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):
+    def get_job(self, service_id, job_id):
-        if job_id:
+        params = {}
-            params = {}
+        job = self.get(url='/service/{}/job/{}'.format(service_id, job_id), params=params)
-            if status is not None:
-                params['status'] = status
-            job = self.get(url='/service/{}/job/{}'.format(service_id, job_id), params=params)
 
-            stats = self.__convert_statistics(job['data'])
+        stats = self.__convert_statistics(job['data'])
-            job['data']['notifications_sent'] = stats['delivered'] + stats['failed']
+        job['data']['notifications_sent'] = stats['delivered'] + stats['failed']
-            job['data']['notifications_delivered'] = stats['delivered']
+        job['data']['notifications_delivered'] = stats['delivered']
-            job['data']['notifications_failed'] = stats['failed']
+        job['data']['notifications_failed'] = stats['failed']
-            job['data']['notifications_requested'] = stats['requested']
+        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

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 if job.get('notifications_failed') else 0
-            return 1
-        return 0
     return (
         job.get('notifications_failed', 0) /
         (job.get('notifications_failed', 0) + job.get('notifications_delivered', 0))

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

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')