Use the revise api endpoints without current-year parameter

app/main/views/add_service.py

@@ -54,7 +54,7 @@ def _create_service(service_name, organisation_type, email_from, form):
         )
         session['service_id'] = service_id
 
-        billing_api_client.create_or_update_free_sms_fragment_limit_for_year(service_id, free_sms_fragment_limit)
+        billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit)
 
         return service_id, None
     except HTTPError as e:

app/main/views/dashboard.py

@@ -112,11 +112,12 @@ def template_history(service_id):
 def usage(service_id):
     year, current_financial_year = requested_and_current_financial_year(request)
 
+    free_sms_allowance = billing_api_client.get_free_sms_fragment_limit_for_year(service_id, year)
     return render_template(
         'views/usage.html',
         months=list(get_free_paid_breakdown_for_billable_units(
             year,
-            billing_api_client.get_free_sms_fragment_limit_for_year(service_id, year),
+            free_sms_allowance,
             billing_api_client.get_billable_units(service_id, year)
         )),
         selected_year=year,
@@ -126,7 +127,7 @@ def usage(service_id):
             end=current_financial_year + 1,
         ),
         **calculate_usage(billing_api_client.get_service_usage(service_id, year),
-                          billing_api_client.get_free_sms_fragment_limit_for_year(service_id, year))
+                          free_sms_allowance)
     )
 
 
@@ -291,8 +292,7 @@ def get_dashboard_totals(statistics):
 
 
 def calculate_usage(usage, free_sms_fragment_limit):
-    # TODO: Don't hardcode these - get em from the API
+    sms_free_allowance = free_sms_fragment_limit
-    sms_free_allowance = free_sms_fragment_limit         # VB-Progress
 
     sms_rate = 0 if len(usage) == 0 else usage[0].get("rate", 0)
     sms_sent = get_sum_billing_units(breakdown for breakdown in usage if breakdown['notification_type'] == 'sms')

app/main/views/service_settings.py

@@ -42,7 +42,6 @@ from app.main.forms import (
 )
 from app import user_api_client, current_service, organisations_client, inbound_number_client, billing_api_client
 from notifications_utils.formatters import formatted_list
-from app.utils import get_current_financial_year
 
 
 dummy_bearer_token = 'bearer_token_set'
@@ -737,22 +736,16 @@ def set_organisation_type(service_id):
 def set_free_sms_allowance(service_id):
 
     form = FreeSMSAllowance(free_sms_allowance=billing_api_client.get_free_sms_fragment_limit_for_year(service_id))
+
     if form.validate_on_submit():
         service_api_client.update_service(
             service_id,
             # TODO: Retire this after new end points are added.
             free_sms_fragment_limit=form.free_sms_allowance.data,
         )
-        # get a list of all the free sms allowance entries for this service
-        sms_list = billing_api_client.get_free_sms_fragment_limit_for_all_years(service_id)
 
-        for item in range(0, len(sms_list)):
+        billing_api_client.create_or_update_free_sms_fragment_limit(service_id, form.free_sms_allowance.data)
-            if sms_list[item]['financial_year_start'] >= get_current_financial_year():
+
-                form.set_free_sms_allowance = \
-                    billing_api_client.create_or_update_free_sms_fragment_limit_for_year(service_id,
-                                                                                         form.free_sms_allowance.data,
-                                                                                         sms_list[item][
-                                                                                             'financial_year_start'])
         return redirect(url_for('.service_settings', service_id=service_id))
 
     return render_template(

app/notify_client/billing_api_client.py

@@ -1,6 +1,4 @@
 from app.notify_client import NotifyAdminAPIClient
-from flask import current_app
-from notifications_python_client.errors import HTTPError
 
 
 class BillingAPIClient(NotifyAdminAPIClient):
@@ -27,44 +25,23 @@ class BillingAPIClient(NotifyAdminAPIClient):
         )
 
     def get_free_sms_fragment_limit_for_year(self, service_id, year=None):
-        try:
+        result = self.get(
-            if year is None:
+            '/service/{0}/billing/free-sms-fragment-limit'.format(service_id),
-                result = self.get(
+            params=dict(financial_year_start=year)
-                    '/service/{0}/billing/free-sms-fragment-limit/current-year'.format(service_id)
+        )
-                )
+        return result['free_sms_fragment_limit']
-            else:
-                result = self.get(
-                    '/service/{0}/billing/free-sms-fragment-limit'.format(service_id),
-                    params=dict(financial_year_start=year)
-                )
-            return result['free_sms_fragment_limit']
-        except HTTPError:
-            current_app.logger.info(
-                'Requested free_sms_fragment_limit entry for service {0} and year {1} does not exist'
-                .format(service_id, year))
-            return -1
 
     def get_free_sms_fragment_limit_for_all_years(self, service_id, year=None):
-        try:
+        return self.get(
-            return self.get(
+            '/service/{0}/billing/free-sms-fragment-limit'.format(service_id))
-                '/service/{0}/billing/free-sms-fragment-limit'.format(service_id),
+
-            )
+    def create_or_update_free_sms_fragment_limit(self, service_id, free_sms_fragment_limit, year=None):
-        except HTTPError:
+        # year = None will update current and future year in the API
-            current_app.logger.info(
+        data = {
-                'No free_sms_fragment_limit entry exists for service {0} '
+            "financial_year_start": year,
-                .format(service_id, year))
+            "free_sms_fragment_limit": free_sms_fragment_limit
-            return []
+        }
 
-    def create_or_update_free_sms_fragment_limit_for_year(self, service_id, free_sms_fragment_limit, year=None):
-        if year is None:
-            data = {
-                "free_sms_fragment_limit": free_sms_fragment_limit,
-            }
-        else:
-            data = {
-                "financial_year_start": year,
-                "free_sms_fragment_limit": free_sms_fragment_limit,
-            }
         return self.post(
             url='/service/{0}/billing/free-sms-fragment-limit'.format(service_id),
             data=data

tests/app/main/views/service_settings/test_service_setting_permissions.py

@@ -12,6 +12,7 @@ def get_service_settings_page(
     service_one,
     mock_get_inbound_number_for_service,
     mock_get_letter_organisations,
+    mock_get_free_sms_fragment_limit,
     no_reply_to_email_addresses,
     no_letter_contact_blocks,
     single_sms_sender,
@@ -83,11 +84,12 @@ def test_service_setting_toggles_dont_show(get_service_settings_page, service_on
 def test_normal_user_doesnt_see_any_toggle_buttons(
     client_request,
     service_one,
-    mock_get_inbound_number_for_service,
-    mock_get_letter_organisations,
     no_reply_to_email_addresses,
     no_letter_contact_blocks,
     single_sms_sender,
+    mock_get_letter_organisations,
+    mock_get_inbound_number_for_service,
+    mock_get_free_sms_fragment_limit,
 ):
     page = client_request.get('main.service_settings', service_id=service_one['id'])
     toggles = page.find('a', {'class': 'button'})

tests/app/main/views/test_dashboard.py

@@ -595,7 +595,8 @@ def test_menu_send_messages(
     mock_get_template_statistics,
     mock_get_detailed_service,
     mock_get_usage,
-    mock_get_inbound_sms_summary
+    mock_get_inbound_sms_summary,
+    mock_get_free_sms_fragment_limit,
 ):
     with app_.test_request_context():
         resp = _test_dashboard_menu(
@@ -626,7 +627,8 @@ def test_menu_manage_service(
     mock_get_template_statistics,
     mock_get_detailed_service,
     mock_get_usage,
-    mock_get_inbound_sms_summary
+    mock_get_inbound_sms_summary,
+    mock_get_free_sms_fragment_limit,
 ):
     with app_.test_request_context():
         resp = _test_dashboard_menu(
@@ -656,7 +658,8 @@ def test_menu_manage_api_keys(
     mock_get_template_statistics,
     mock_get_detailed_service,
     mock_get_usage,
-    mock_get_inbound_sms_summary
+    mock_get_inbound_sms_summary,
+    mock_get_free_sms_fragment_limit,
 ):
     with app_.test_request_context():
         resp = _test_dashboard_menu(
@@ -686,7 +689,8 @@ def test_menu_all_services_for_platform_admin_user(
     mock_get_template_statistics,
     mock_get_detailed_service,
     mock_get_usage,
-    mock_get_inbound_sms_summary
+    mock_get_inbound_sms_summary,
+    mock_get_free_sms_fragment_limit,
 ):
     with app_.test_request_context():
         resp = _test_dashboard_menu(

tests/app/main/views/test_service_settings.py

@@ -32,6 +32,15 @@ from tests.conftest import (
 from freezegun import freeze_time
 
 
+@pytest.fixture
+def mock_get_service_settings_page_common(
+    mock_get_letter_organisations,
+    mock_get_inbound_number_for_service,
+    mock_get_free_sms_fragment_limit,
+):
+    return
+
+
 @pytest.mark.parametrize('user, expected_rows', [
     (active_user_with_permissions, [
 
@@ -85,14 +94,12 @@ def test_should_show_overview(
         mocker,
         service_one,
         fake_uuid,
-        mock_get_letter_organisations,
         no_reply_to_email_addresses,
         no_letter_contact_blocks,
         single_sms_sender,
         user,
         expected_rows,
-        mock_get_inbound_number_for_service,
+        mock_get_service_settings_page_common,
-        mock_get_free_sms_fragment_limit
 ):
 
     service_one['permissions'] = ['sms', 'email']
@@ -161,8 +168,7 @@ def test_should_show_overview_for_service_with_more_things_set(
         single_letter_contact_block,
         single_sms_sender,
         mock_get_organisation,
-        mock_get_letter_organisations,
+        mock_get_service_settings_page_common,
-        mock_get_inbound_number_for_service,
         permissions,
         expected_rows
 ):
@@ -184,7 +190,6 @@ def test_should_show_overview_for_service_with_more_things_set(
 def test_service_settings_show_elided_api_url_if_needed(
     logged_in_platform_admin_client,
     service_one,
-    mock_get_letter_organisations,
     single_reply_to_email_address,
     single_sms_sender,
     single_letter_contact_block,
@@ -192,7 +197,7 @@ def test_service_settings_show_elided_api_url_if_needed(
     fake_uuid,
     url,
     elided_url,
-    mock_get_inbound_number_for_service
+    mock_get_service_settings_page_common,
 ):
     service_one['permissions'] = ['sms', 'email', 'inbound_sms']
     service_one['inbound_api'] = [fake_uuid]
@@ -221,8 +226,7 @@ def test_service_settings_show_elided_api_url_if_needed(
 def test_if_cant_send_letters_then_cant_see_letter_contact_block(
         logged_in_client,
         service_one,
-        mock_get_letter_organisations,
+        mock_get_service_settings_page_common,
-        mock_get_inbound_number_for_service
 ):
     response = logged_in_client.get(url_for(
         'main.service_settings', service_id=service_one['id']
@@ -237,8 +241,7 @@ def test_letter_contact_block_shows_none_if_not_set(
         single_reply_to_email_address,
         no_letter_contact_blocks,
         single_sms_sender,
-        mock_get_letter_organisations,
+        mock_get_service_settings_page_common,
-        mock_get_inbound_number_for_service
 ):
     service_one['permissions'] = ['letter']
     response = logged_in_client.get(url_for(
@@ -258,8 +261,7 @@ def test_escapes_letter_contact_block(
         single_reply_to_email_address,
         single_sms_sender,
         injected_letter_contact_block,
-        mock_get_letter_organisations,
+        mock_get_service_settings_page_common,
-        mock_get_inbound_number_for_service
 ):
     service_one['permissions'] = ['letter']
     response = logged_in_client.get(url_for(
@@ -305,11 +307,10 @@ def test_should_redirect_after_change_service_name(
 def test_show_restricted_service(
         logged_in_client,
         service_one,
-        mock_get_letter_organisations,
         single_reply_to_email_address,
         single_letter_contact_block,
         single_sms_sender,
-        mock_get_inbound_number_for_service
+        mock_get_service_settings_page_common,
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -343,8 +344,7 @@ def test_show_live_service(
         single_reply_to_email_address,
         single_letter_contact_block,
         single_sms_sender,
-        mock_get_letter_organisations,
+        mock_get_service_settings_page_common,
-        mock_get_inbound_number_for_service
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -357,7 +357,7 @@ def test_switch_service_to_restricted(
         service_one,
         mock_get_live_service,
         mock_update_service,
-        mock_get_inbound_number_for_service
+        mock_get_inbound_number_for_service,
 ):
     response = logged_in_platform_admin_client.get(
         url_for('main.service_switch_live', service_id=service_one['id']))
@@ -406,7 +406,7 @@ def test_should_redirect_after_service_name_confirmation(
         service_one,
         mock_update_service,
         mock_verify_password,
-        mock_get_inbound_number_for_service
+        mock_get_inbound_number_for_service,
 ):
     service_id = service_one['id']
     service_new_name = 'New Name'
@@ -470,8 +470,7 @@ def test_should_redirect_after_request_to_go_live(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common
-    mock_get_inbound_number_for_service,
 ):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
@@ -569,9 +568,8 @@ def test_route_permissions(
         single_reply_to_email_address,
         single_letter_contact_block,
         single_sms_sender,
-        mock_get_letter_organisations,
         route,
-        mock_get_inbound_number_for_service
+        mock_get_service_settings_page_common,
 ):
     validate_route_permission(
         mocker,
@@ -629,9 +627,8 @@ def test_route_for_platform_admin(
         single_reply_to_email_address,
         single_letter_contact_block,
         single_sms_sender,
-        mock_get_letter_organisations,
         route,
-        mock_get_inbound_number_for_service
+        mock_get_service_settings_page_common,
 ):
     validate_route_permission(mocker,
                               app_,
@@ -699,11 +696,10 @@ def test_enabling_and_disabling_email_and_sms(
 def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender(
         client_request,
         service_one,
-        mock_get_letter_organisations,
-        mock_get_inbound_number_for_service,
         multiple_reply_to_email_addresses,
         multiple_letter_contact_blocks,
-        multiple_sms_senders
+        multiple_sms_senders,
+        mock_get_service_settings_page_common,
 ):
     service_one['permissions'] = ['email', 'sms', 'letter']
 
@@ -1242,11 +1238,10 @@ def test_shows_research_mode_indicator(
     logged_in_client,
     service_one,
     mocker,
-    mock_get_letter_organisations,
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_inbound_number_for_service
+    mock_get_service_settings_page_common,
 ):
     service_one['research_mode'] = True
     mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
@@ -1262,11 +1257,10 @@ def test_shows_research_mode_indicator(
 def test_does_not_show_research_mode_indicator(
     logged_in_client,
     service_one,
-    mock_get_letter_organisations,
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_inbound_number_for_service
+    mock_get_service_settings_page_common,
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     assert response.status_code == 200
@@ -1577,6 +1571,7 @@ def test_should_set_organisation_type(
 
 def test_should_show_page_to_set_sms_allowance(
     logged_in_platform_admin_client,
+    mock_get_free_sms_fragment_limit
 ):
     response = logged_in_platform_admin_client.get(url_for(
         'main.set_free_sms_allowance',
@@ -1586,6 +1581,7 @@ def test_should_show_page_to_set_sms_allowance(
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
     assert normalize_spaces(page.select_one('label').text) == 'Numbers of text message fragments per year'
+    mock_get_free_sms_fragment_limit.assert_called_once_with(SERVICE_ONE_ID)
 
 
 @freeze_time("2017-04-01 11:09:00.061258")
@@ -1599,8 +1595,8 @@ def test_should_set_sms_allowance(
     mock_update_service,
     given_allowance,
     expected_api_argument,
+    mock_get_free_sms_fragment_limit,
     mock_create_or_update_free_sms_fragment_limit,
-    mock_get_free_sms_fragment_limit_for_all_years
 ):
 
     response = logged_in_platform_admin_client.post(
@@ -1621,10 +1617,8 @@ def test_should_set_sms_allowance(
     )
     mock_create_or_update_free_sms_fragment_limit.assert_called_with(
         SERVICE_ONE_ID,
-        expected_api_argument,
+        expected_api_argument
-        2017
     )
-    mock_get_free_sms_fragment_limit_for_all_years.assert_called_once_with(SERVICE_ONE_ID)
 
 
 def test_switch_service_enable_letters(
@@ -1843,8 +1837,7 @@ def test_archive_service_prompts_user(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common,
-    mock_get_inbound_number_for_service
 ):
     mocked_fn = mocker.patch('app.service_api_client.post')
 
@@ -1862,8 +1855,7 @@ def test_cant_archive_inactive_service(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common
-    mock_get_inbound_number_for_service
 ):
     service_one['active'] = False
 
@@ -1896,8 +1888,7 @@ def test_suspend_service_prompts_user(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common,
-    mock_get_inbound_number_for_service
 ):
     mocked_fn = mocker.patch('app.service_api_client.post')
 
@@ -1916,8 +1907,7 @@ def test_cant_suspend_inactive_service(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common,
-    mock_get_inbound_number_for_service
 ):
     service_one['active'] = False
 
@@ -1953,8 +1943,7 @@ def test_resume_service_prompts_user(
     single_letter_contact_block,
     single_sms_sender,
     mocker,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common,
-    mock_get_inbound_number_for_service
 ):
     service_one['active'] = False
     mocked_fn = mocker.patch('app.service_api_client.post')
@@ -1974,8 +1963,7 @@ def test_cant_resume_active_service(
     single_reply_to_email_address,
     single_letter_contact_block,
     single_sms_sender,
-    mock_get_letter_organisations,
+    mock_get_service_settings_page_common
-    mock_get_inbound_number_for_service
 ):
     response = logged_in_platform_admin_client.get(url_for('main.service_settings', service_id=service_one['id']))
 
@@ -2039,6 +2027,7 @@ def test_service_settings_when_inbound_number_is_not_set(
     single_sms_sender,
     mocker,
     mock_get_letter_organisations,
+    mock_get_free_sms_fragment_limit,
 ):
     mocker.patch('app.inbound_number_client.get_inbound_sms_number_for_service',
                  return_value={'data': {}})

tests/app/notify_client/test_billing_client.py

@@ -27,17 +27,6 @@ def test_get_get_service_usage_calls_correct_endpoint(mocker, api_user_active):
     mock_get.assert_called_once_with(expected_url, params={'year': 2017})
 
 
-def test_get_free_sms_fragment_limit_for_current_year_correct_endpoint(mocker, api_user_active):
-    service_id = uuid.uuid4()
-    expected_url = '/service/{}/billing/free-sms-fragment-limit/current-year'.format(service_id)
-    client = BillingAPIClient()
-
-    mock_get = mocker.patch('app.notify_client.billing_api_client.BillingAPIClient.get')
-
-    client.get_free_sms_fragment_limit_for_year(service_id)
-    mock_get.assert_called_once_with(expected_url)
-
-
 def test_get_free_sms_fragment_limit_for_year_correct_endpoint(mocker, api_user_active):
     service_id = uuid.uuid4()
     expected_url = '/service/{}/billing/free-sms-fragment-limit'.format(service_id)
@@ -51,11 +40,11 @@ def test_get_free_sms_fragment_limit_for_year_correct_endpoint(mocker, api_user_
 
 def test_post_free_sms_fragment_limit_for_current_year_endpoint(mocker, api_user_active):
     service_id = uuid.uuid4()
-    sms_limit_data = {'free_sms_fragment_limit': 1111}
+    sms_limit_data = {'free_sms_fragment_limit': 1111, 'financial_year_start': None}
     mock_post = mocker.patch('app.notify_client.billing_api_client.BillingAPIClient.post')
     client = BillingAPIClient()
 
-    client.create_or_update_free_sms_fragment_limit_for_year(service_id=service_id, free_sms_fragment_limit=1111)
+    client.create_or_update_free_sms_fragment_limit(service_id=service_id, free_sms_fragment_limit=1111)
 
     mock_post.assert_called_once_with(
         url='/service/{}/billing/free-sms-fragment-limit'.format(service_id),
@@ -69,9 +58,9 @@ def test_post_free_sms_fragment_limit_for_year_endpoint(mocker, api_user_active)
     mock_post = mocker.patch('app.notify_client.billing_api_client.BillingAPIClient.post')
     client = BillingAPIClient()
 
-    client.create_or_update_free_sms_fragment_limit_for_year(service_id=service_id,
+    client.create_or_update_free_sms_fragment_limit(service_id=service_id,
-                                                             free_sms_fragment_limit=1111,
+                                                    free_sms_fragment_limit=1111,
-                                                             year=2017)
+                                                    year=2017)
     mock_post.assert_called_once_with(
         url='/service/{}/billing/free-sms-fragment-limit'.format(service_id),
         data=sms_limit_data

tests/conftest.py

@@ -2436,7 +2436,7 @@ def mock_get_free_sms_fragment_limit(mocker):
 @pytest.fixture(scope='function')
 def mock_create_or_update_free_sms_fragment_limit(mocker):
     sample_limit = 250000
-    return mocker.patch('app.billing_api_client.create_or_update_free_sms_fragment_limit_for_year',
+    return mocker.patch('app.billing_api_client.create_or_update_free_sms_fragment_limit',
                         return_value=sample_limit)