Reduced the size of the pull request by only dealing with adding the data retention form.

app/main/views/jobs.py

@@ -265,8 +265,7 @@ def get_notifications(service_id, message_type, status_override=None):
             status_filters=get_status_filters(
                 current_service,
                 message_type,
-                service_api_client.get_service_statistics(service_id,
+                service_api_client.get_service_statistics(service_id, today_only=False)
-                                                          limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])
             )
         ),
         'notifications': render_template(

app/main/views/send.py

@@ -511,7 +511,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
     users = user_api_client.get_users_for_service(service_id=service_id)
 
     statistics = service_api_client.get_service_statistics(service_id, today_only=True)
-    remaining_messages = (current_service['message_limit'] - sum(stat['requested'] for stat in statistics.values()))
+    remaining_messages = (current_service.message_limit - sum(stat['requested'] for stat in statistics.values()))
 
     contents = s3download(service_id, upload_id)
 

app/notify_client/service_api_client.py

@@ -41,14 +41,8 @@ class ServiceAPIClient(NotifyAdminAPIClient):
         """
         return self.get('/service/{0}'.format(service_id))
 
-    # def get_service_statistics(self, service_id, today_only):
+    def get_service_statistics(self, service_id, today_only):
-    #     return self.get('/service/{0}/statistics'.format(service_id), params={'today_only': today_only})['data']
+        return self.get('/service/{0}/statistics'.format(service_id), params={'today_only': today_only})['data']
-
-    def get_service_statistics(self, service_id, limit_days=7):
-        return self.get('/service/{0}/statistics'.format(service_id), params={'limit_days': limit_days})['data']
-
-    def get_service_statistics_for_today(self, service_id):
-        return self.get('/service/{0}/statistics'.format(service_id), params={'today_only': True})['data']
 
     def get_services(self, params_dict=None):
         """

tests/app/main/views/test_send.py

@@ -384,7 +384,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
     mock_get_service_template_with_placeholders,
     mock_s3_upload,
     mock_get_users_by_service,
-    mock_get_service_statistics_for_today,
+    mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     fake_uuid,
 ):
@@ -509,7 +509,7 @@ def test_upload_csvfile_with_missing_columns_shows_error(
     mock_get_service_template_with_placeholders,
     mock_s3_upload,
     mock_get_users_by_service,
-    mock_get_service_statistics_for_today,
+    mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     service_one,
     fake_uuid,
@@ -590,7 +590,7 @@ def test_upload_valid_csv_shows_preview_and_table(
     mock_get_live_service,
     mock_get_service_template_with_placeholders,
     mock_get_users_by_service,
-    mock_get_service_statistics_for_today,
+    mock_get_service_statistics,
     mock_get_job_doesnt_exist,
     mock_s3_set_metadata,
     fake_uuid,

tests/app/notify_client/test_service_api_client.py

@@ -35,24 +35,15 @@ def test_client_gets_service(mocker):
     mock_get.assert_called_once_with('/service/foo')
 
 
-def test_client_gets_service_statistics(mocker):
+@pytest.mark.parametrize('today_only', [True, False])
+def test_client_gets_service_statistics(mocker, today_only):
     client = ServiceAPIClient()
     mock_get = mocker.patch.object(client, 'get', return_value={'data': {'a': 'b'}})
 
-    ret = client.get_service_statistics('foo')
+    ret = client.get_service_statistics('foo', today_only)
 
     assert ret == {'a': 'b'}
-    mock_get.assert_called_once_with('/service/foo/statistics')
+    mock_get.assert_called_once_with('/service/foo/statistics', params={'today_only': today_only})
-
-
-def test_client_gets_service_statistics_for_today(mocker):
-    client = ServiceAPIClient()
-    mock_get = mocker.patch.object(client, 'get', return_value={'data': {'a': 'b'}})
-
-    ret = client.get_service_statistics_for_today('foo')
-
-    assert ret == {'a': 'b'}
-    mock_get.assert_called_once_with('/service/foo/statistics', params={'today_only': True})
 
 
 def test_client_only_updates_allowed_attributes(mocker):

tests/conftest.py

@@ -523,7 +523,7 @@ def mock_get_international_service(mocker, api_user_active):
 
 @pytest.fixture(scope='function')
 def mock_get_service_statistics(mocker, api_user_active):
-    def _get(service_id, limit_days=7):
+    def _get(service_id, today_only):
         return {
             'email': {'requested': 0, 'delivered': 0, 'failed': 0},
             'sms': {'requested': 0, 'delivered': 0, 'failed': 0},
@@ -533,18 +533,6 @@ def mock_get_service_statistics(mocker, api_user_active):
     return mocker.patch('app.service_api_client.get_service_statistics', side_effect=_get)
 
 
-@pytest.fixture(scope='function')
-def mock_get_service_statistics_for_today(mocker, api_user_active):
-    def _get(service_id):
-        return {
-            'email': {'requested': 0, 'delivered': 0, 'failed': 0},
-            'sms': {'requested': 0, 'delivered': 0, 'failed': 0},
-            'letter': {'requested': 0, 'delivered': 0, 'failed': 0}
-        }
-
-    return mocker.patch('app.service_api_client.get_service_statistics_for_today', side_effect=_get)
-
-
 @pytest.fixture(scope='function')
 def mock_get_detailed_services(mocker, fake_uuid):
     service_one = service_json(