From 8ea38ba7b6214ece5812288e07ac667379578ab7 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 31 Oct 2017 11:22:57 +0000 Subject: [PATCH 1/8] Use annual_billing tables at backend for getting and updating free_sms_fragment_limit --- app/main/views/add_service.py | 9 +++- app/main/views/dashboard.py | 18 ++++--- app/main/views/service_settings.py | 15 ++++-- app/notify_client/billing_api_client.py | 46 +++++++++++++++++ app/templates/views/service-settings.html | 2 +- tests/app/main/views/test_add_service.py | 2 + tests/app/main/views/test_dashboard.py | 12 ++++- tests/app/main/views/test_service_settings.py | 9 +++- .../app/notify_client/test_billing_client.py | 51 +++++++++++++++++++ tests/conftest.py | 14 +++++ 10 files changed, 161 insertions(+), 17 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index a6779fc38..ecf2b9723 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -20,7 +20,8 @@ from app.notify_client.models import InvitedUser from app import ( invite_api_client, user_api_client, - service_api_client + service_api_client, + billing_api_client ) from app.utils import ( @@ -40,17 +41,21 @@ def _add_invited_user_to_service(invited_user): def _create_service(service_name, organisation_type, email_from, form): + free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get(organisation_type) try: service_id = service_api_client.create_service( service_name=service_name, organisation_type=organisation_type, message_limit=current_app.config['DEFAULT_SERVICE_LIMIT'], - free_sms_fragment_limit=current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get(organisation_type), + free_sms_fragment_limit=free_sms_fragment_limit, restricted=True, user_id=session['user_id'], email_from=email_from, ) session['service_id'] = service_id + + billing_api_client.create_or_update_free_sms_fragment_limit_for_year(service_id, free_sms_fragment_limit) + return service_id, None except HTTPError as e: if e.status_code == 400 and e.message['name']: diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index ce63fdc00..9901c3fda 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -115,7 +115,9 @@ def usage(service_id): return render_template( 'views/usage.html', months=list(get_free_paid_breakdown_for_billable_units( - year, billing_api_client.get_billable_units(service_id, year) + year, + billing_api_client.get_free_sms_fragment_limit_for_year(service_id, year), + billing_api_client.get_billable_units(service_id, year) )), selected_year=year, years=get_tuples_of_financial_years( @@ -123,7 +125,8 @@ def usage(service_id): start=current_financial_year - 1, end=current_financial_year + 1, ), - **calculate_usage(billing_api_client.get_service_usage(service_id, year)) + **calculate_usage(billing_api_client.get_service_usage(service_id, year), + billing_api_client.get_free_sms_fragment_limit_for_year(service_id, year)) ) @@ -287,9 +290,9 @@ def get_dashboard_totals(statistics): return statistics -def calculate_usage(usage): +def calculate_usage(usage, free_sms_fragment_limit): # TODO: Don't hardcode these - get em from the API - sms_free_allowance = 250000 + 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') @@ -355,14 +358,14 @@ def get_sum_billing_units(billing_units, month=None): return sum(b['billing_units'] * b.get('rate_multiplier', 1) for b in billing_units) -def get_free_paid_breakdown_for_billable_units(year, billing_units): +def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, billing_units): cumulative = 0 for month in get_months_for_financial_year(year): previous_cumulative = cumulative monthly_usage = get_sum_billing_units(billing_units, month) cumulative += monthly_usage breakdown = get_free_paid_breakdown_for_month( - cumulative, previous_cumulative, + free_sms_fragment_limit, cumulative, previous_cumulative, [billing_month for billing_month in billing_units if billing_month['month'] == month] ) yield { @@ -373,11 +376,12 @@ def get_free_paid_breakdown_for_billable_units(year, billing_units): def get_free_paid_breakdown_for_month( + free_sms_fragment_limit, cumulative, previous_cumulative, monthly_usage ): - allowance = 250000 + allowance = free_sms_fragment_limit total_monthly_billing_units = get_sum_billing_units(monthly_usage) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 4c2cef034..3e370437e 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -39,7 +39,7 @@ from app.main.forms import ( FreeSMSAllowance, ServiceEditInboundNumberForm, ) -from app import user_api_client, current_service, organisations_client, inbound_number_client +from app import user_api_client, current_service, organisations_client, inbound_number_client, billing_api_client from notifications_utils.formatters import formatted_list @@ -89,6 +89,9 @@ def service_settings(service_id): default_sms_sender = next( (Field(x['sms_sender'], html='escape') for x in sms_senders if x['is_default']), "None" ) + + free_sms_fragment_limit = billing_api_client.get_free_sms_fragment_limit_for_year(service_id) + return render_template( 'views/service-settings.html', organisation=organisation, @@ -103,7 +106,8 @@ def service_settings(service_id): default_letter_contact_block=default_letter_contact_block, letter_contact_details_count=letter_contact_details_count, default_sms_sender=default_sms_sender, - sms_sender_count=sms_sender_count + sms_sender_count=sms_sender_count, + free_sms_fragment_limit=free_sms_fragment_limit ) @@ -704,13 +708,16 @@ def set_organisation_type(service_id): @user_has_permissions(admin_override=True) def set_free_sms_allowance(service_id): - form = FreeSMSAllowance(free_sms_allowance=current_service['free_sms_fragment_limit']) - + 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, ) + form.set_free_sms_allowance = \ + billing_api_client.create_or_update_free_sms_fragment_limit_for_year(service_id, + form.free_sms_allowance.data) return redirect(url_for('.service_settings', service_id=service_id)) return render_template( diff --git a/app/notify_client/billing_api_client.py b/app/notify_client/billing_api_client.py index 2041f74fe..7e303ca7c 100644 --- a/app/notify_client/billing_api_client.py +++ b/app/notify_client/billing_api_client.py @@ -1,4 +1,6 @@ from app.notify_client import NotifyAdminAPIClient +from flask import current_app +from notifications_python_client.errors import HTTPError class BillingAPIClient(NotifyAdminAPIClient): @@ -23,3 +25,47 @@ class BillingAPIClient(NotifyAdminAPIClient): '/service/{0}/billing/yearly-usage-summary'.format(service_id), params=dict(year=year) ) + + def get_free_sms_fragment_limit_for_year(self, service_id, year=None): + try: + if year is None: + result = self.get( + '/service/{0}/billing/free-sms-fragment-limit/current-year'.format(service_id) + ) + 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( + '/service/{0}/billing/free-sms-fragment-limit'.format(service_id), + ) + except HTTPError: + current_app.logger.info( + 'No free_sms_fragment_limit entry exists for service {0} ' + .format(service_id, year)) + 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 + ) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 4e279f043..7157affcd 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -194,7 +194,7 @@ {% endcall %} {% call row() %} {{ text_field('Free text message allowance')}} - {{ text_field('{:,}'.format(current_service.free_sms_fragment_limit or 0)) }} + {{ text_field('{:,}'.format(free_sms_fragment_limit or 0)) }} {{ edit_field('Change', url_for('.set_free_sms_allowance', service_id=current_service.id)) }} {% endcall %} {% call row() %} diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index ee584a27a..0d4ead910 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -31,6 +31,7 @@ def test_should_add_service_and_redirect_to_tour_when_no_services( mock_create_service_template, mock_get_services_with_no_services, api_user_active, + mock_create_or_update_free_sms_fragment_limit, ): response = logged_in_client.post( url_for('main.add_service'), @@ -83,6 +84,7 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service( api_user_active, organisation_type, free_allowance, + mock_create_or_update_free_sms_fragment_limit ): response = logged_in_client.post( url_for('main.add_service'), diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 35a68dea4..f78ee5b17 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -502,6 +502,7 @@ def test_usage_page( logged_in_client, mock_get_usage, mock_get_billable_units, + mock_get_free_sms_fragment_limit ): response = logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID)) @@ -509,6 +510,7 @@ def test_usage_page( mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2011) mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2011) + mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2011) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -540,11 +542,14 @@ def test_usage_page( def test_usage_page_with_year_argument( logged_in_client, mock_get_usage, - mock_get_billable_units + mock_get_billable_units, + mock_get_free_sms_fragment_limit, + mock_create_or_update_free_sms_fragment_limit ): assert logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year=2000)).status_code == 200 mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2000) mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2000) + mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2000) def test_usage_page_for_invalid_year( @@ -558,11 +563,13 @@ def test_future_usage_page( logged_in_client, mock_get_future_usage, mock_get_future_billable_units, + mock_get_free_sms_fragment_limit ): assert logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year=2014)).status_code == 200 mock_get_future_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2014) mock_get_future_usage.assert_called_once_with(SERVICE_ONE_ID, 2014) + mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2014) def _test_dashboard_menu(mocker, app_, usr, service, permissions): @@ -866,9 +873,10 @@ def test_aggregate_status_types(dict_in, expected_failed, expected_requested): ] ) def test_get_free_paid_breakdown_for_billable_units(now, expected_number_of_months): + sms_allowance = 250000 with now: billing_units = get_free_paid_breakdown_for_billable_units( - 2016, [ + 2016, sms_allowance, [ { 'month': 'April', 'international': False, 'rate_multiplier': 1, 'notification_type': 'sms', 'rate': 1.65, 'billing_units': 100000 diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 3a7cc9884..4c553ef9e 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -88,7 +88,8 @@ def test_should_show_overview( single_sms_sender, user, expected_rows, - mock_get_inbound_number_for_service + mock_get_inbound_number_for_service, + mock_get_free_sms_fragment_limit ): service_one['permissions'] = ['sms', 'email'] @@ -1592,7 +1593,9 @@ def test_should_set_sms_allowance( mock_update_service, given_allowance, expected_api_argument, + mock_create_or_update_free_sms_fragment_limit ): + response = logged_in_platform_admin_client.post( url_for( 'main.set_free_sms_allowance', @@ -1609,6 +1612,10 @@ def test_should_set_sms_allowance( SERVICE_ONE_ID, free_sms_fragment_limit=expected_api_argument, ) + mock_create_or_update_free_sms_fragment_limit.assert_called_once_with( + SERVICE_ONE_ID, + expected_api_argument + ) def test_switch_service_enable_letters( diff --git a/tests/app/notify_client/test_billing_client.py b/tests/app/notify_client/test_billing_client.py index 250ad24cd..8e3d403a3 100644 --- a/tests/app/notify_client/test_billing_client.py +++ b/tests/app/notify_client/test_billing_client.py @@ -25,3 +25,54 @@ def test_get_get_service_usage_calls_correct_endpoint(mocker, api_user_active): client.get_service_usage(service_id, 2017) 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) + client = BillingAPIClient() + + mock_get = mocker.patch('app.notify_client.billing_api_client.BillingAPIClient.get') + + client.get_free_sms_fragment_limit_for_year(service_id, year=1999) + mock_get.assert_called_once_with(expected_url, params={'financial_year_start': 1999}) + + +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} + 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) + + mock_post.assert_called_once_with( + url='/service/{}/billing/free-sms-fragment-limit'.format(service_id), + data=sms_limit_data + ) + + +def test_post_free_sms_fragment_limit_for_year_endpoint(mocker, api_user_active): + service_id = uuid.uuid4() + sms_limit_data = {'free_sms_fragment_limit': 1111, 'financial_year_start': 2017} + 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, + year=2017) + mock_post.assert_called_once_with( + url='/service/{}/billing/free-sms-fragment-limit'.format(service_id), + data=sms_limit_data + ) diff --git a/tests/conftest.py b/tests/conftest.py index d3a2a1a7d..181d0c02d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2327,3 +2327,17 @@ def mock_get_aggregate_platform_stats(mocker): } return mocker.patch('app.service_api_client.get_aggregate_platform_stats', return_value=stats) + + +@pytest.fixture(scope='function') +def mock_get_free_sms_fragment_limit(mocker): + sample_limit = 250000 + return mocker.patch('app.billing_api_client.get_free_sms_fragment_limit_for_year', + return_value=sample_limit) + + +@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_value=sample_limit) From 8e6c284d7ba3bf662bf0a464cc82d7c40c5a1576 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 31 Oct 2017 11:56:51 +0000 Subject: [PATCH 2/8] add logic to change future free_sms_fragment_limit items when service setting changed --- app/main/views/service_settings.py | 14 +++++++++++--- tests/app/main/views/test_service_settings.py | 11 ++++++++--- tests/conftest.py | 9 +++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 3e370437e..ed89ada2c 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -41,6 +41,7 @@ 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' @@ -715,9 +716,16 @@ def set_free_sms_allowance(service_id): # TODO: Retire this after new end points are added. free_sms_fragment_limit=form.free_sms_allowance.data, ) - form.set_free_sms_allowance = \ - billing_api_client.create_or_update_free_sms_fragment_limit_for_year(service_id, - 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)): + 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( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 4c553ef9e..cdb33256b 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -29,6 +29,7 @@ from tests.conftest import ( get_inbound_number_sms_sender, SERVICE_ONE_ID ) +from freezegun import freeze_time @pytest.mark.parametrize('user, expected_rows', [ @@ -1583,6 +1584,7 @@ def test_should_show_page_to_set_sms_allowance( assert normalize_spaces(page.select_one('label').text) == 'Numbers of text message fragments per year' +@freeze_time("2017-04-01 11:09:00.061258") @pytest.mark.parametrize('given_allowance, expected_api_argument', [ ('1', 1), ('250000', 250000), @@ -1593,7 +1595,8 @@ def test_should_set_sms_allowance( mock_update_service, given_allowance, expected_api_argument, - mock_create_or_update_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( @@ -1612,10 +1615,12 @@ def test_should_set_sms_allowance( SERVICE_ONE_ID, free_sms_fragment_limit=expected_api_argument, ) - mock_create_or_update_free_sms_fragment_limit.assert_called_once_with( + 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( diff --git a/tests/conftest.py b/tests/conftest.py index 181d0c02d..101bfef4f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2341,3 +2341,12 @@ 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_value=sample_limit) + + +@pytest.fixture(scope='function') +def mock_get_free_sms_fragment_limit_for_all_years(mocker): + sample_limit = [{'financial_year_start': 2016, 'free_sms_fragment_limit': 250000}, + {'financial_year_start': 2017, 'free_sms_fragment_limit': 500000}] + + return mocker.patch('app.billing_api_client.get_free_sms_fragment_limit_for_all_years', + return_value=sample_limit) From 1ab4681ff523c65d93b5b3434a24871865db1c91 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 9 Nov 2017 13:18:09 +0000 Subject: [PATCH 3/8] Use the revise api endpoints without current-year parameter --- app/main/views/add_service.py | 2 +- app/main/views/dashboard.py | 8 +- app/main/views/service_settings.py | 13 +-- app/notify_client/billing_api_client.py | 51 +++-------- .../test_service_setting_permissions.py | 6 +- tests/app/main/views/test_dashboard.py | 12 ++- tests/app/main/views/test_service_settings.py | 85 ++++++++----------- .../app/notify_client/test_billing_client.py | 21 ++--- tests/conftest.py | 2 +- 9 files changed, 77 insertions(+), 123 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index ecf2b9723..e2e328977 100644 --- a/app/main/views/add_service.py +++ b/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: diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 9901c3fda..22a85e486 100644 --- a/app/main/views/dashboard.py +++ b/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 # VB-Progress + sms_free_allowance = free_sms_fragment_limit 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') diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 3306cdcbc..f4cb3c69e 100644 --- a/app/main/views/service_settings.py +++ b/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)): - 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']) + billing_api_client.create_or_update_free_sms_fragment_limit(service_id, form.free_sms_allowance.data) + return redirect(url_for('.service_settings', service_id=service_id)) return render_template( diff --git a/app/notify_client/billing_api_client.py b/app/notify_client/billing_api_client.py index 7e303ca7c..ab6e76e13 100644 --- a/app/notify_client/billing_api_client.py +++ b/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: - if year is None: - result = self.get( - '/service/{0}/billing/free-sms-fragment-limit/current-year'.format(service_id) - ) - 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 + 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'] def get_free_sms_fragment_limit_for_all_years(self, service_id, year=None): - try: - return self.get( - '/service/{0}/billing/free-sms-fragment-limit'.format(service_id), - ) - except HTTPError: - current_app.logger.info( - 'No free_sms_fragment_limit entry exists for service {0} ' - .format(service_id, year)) - return [] + return self.get( + '/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): + # year = None will update current and future year in the API + data = { + "financial_year_start": year, + "free_sms_fragment_limit": free_sms_fragment_limit + } - 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 diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index 0002e23bc..8996989a1 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/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'}) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index f78ee5b17..d8eb6c20e 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/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( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index fa7ca8424..47e0092b9 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/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_free_sms_fragment_limit + mock_get_service_settings_page_common, ): 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_inbound_number_for_service, + mock_get_service_settings_page_common, 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_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'] @@ -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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_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') @@ -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_inbound_number_for_service, + mock_get_service_settings_page_common ): 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, - 2017 + expected_api_argument ) - 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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_inbound_number_for_service + mock_get_service_settings_page_common ): 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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_inbound_number_for_service + mock_get_service_settings_page_common, ): 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_inbound_number_for_service + mock_get_service_settings_page_common ): 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': {}}) diff --git a/tests/app/notify_client/test_billing_client.py b/tests/app/notify_client/test_billing_client.py index 8e3d403a3..1ba7a4be0 100644 --- a/tests/app/notify_client/test_billing_client.py +++ b/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, - free_sms_fragment_limit=1111, - year=2017) + client.create_or_update_free_sms_fragment_limit(service_id=service_id, + free_sms_fragment_limit=1111, + year=2017) mock_post.assert_called_once_with( url='/service/{}/billing/free-sms-fragment-limit'.format(service_id), data=sms_limit_data diff --git a/tests/conftest.py b/tests/conftest.py index a2966b7fa..8ea18f652 100644 --- a/tests/conftest.py +++ b/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) From a9f06c23f7ae895d8c3c23b07976fae32b4c9039 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 14 Nov 2017 16:23:08 +0000 Subject: [PATCH 4/8] Commit out using the new end points until data is migrated --- app/main/views/add_service.py | 6 +++--- app/main/views/service_settings.py | 5 +++-- tests/app/main/views/test_service_settings.py | 9 +++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index e2e328977..6e816a409 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -20,8 +20,7 @@ from app.notify_client.models import InvitedUser from app import ( invite_api_client, user_api_client, - service_api_client, - billing_api_client + service_api_client ) from app.utils import ( @@ -54,7 +53,8 @@ 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(service_id, free_sms_fragment_limit) + # TODO: Comment out until data migration + # billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit) return service_id, None except HTTPError as e: diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 04006bc14..ad5817fa1 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -718,11 +718,12 @@ def set_free_sms_allowance(service_id): if form.validate_on_submit(): service_api_client.update_service( service_id, - # TODO: Retire this after new end points are added. + # TODO: Retire this eventually after using annual_billing free_sms_fragment_limit=form.free_sms_allowance.data, ) - billing_api_client.create_or_update_free_sms_fragment_limit(service_id, form.free_sms_allowance.data) + # TODO: Comment out until data migration + # billing_api_client.create_or_update_free_sms_fragment_limit(service_id, form.free_sms_allowance.data) return redirect(url_for('.service_settings', service_id=service_id)) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 47e0092b9..d2a9d45c8 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1615,10 +1615,11 @@ def test_should_set_sms_allowance( SERVICE_ONE_ID, free_sms_fragment_limit=expected_api_argument, ) - mock_create_or_update_free_sms_fragment_limit.assert_called_with( - SERVICE_ONE_ID, - expected_api_argument - ) + # Not assert until using the annual_billing end points + # mock_create_or_update_free_sms_fragment_limit.assert_called_with( + # SERVICE_ONE_ID, + # expected_api_argument + # ) def test_switch_service_enable_letters( From a8e62a564dffe56f761c86e77f778134fd1b63ca Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 14 Nov 2017 17:25:32 +0000 Subject: [PATCH 5/8] Add meta description tag to homepage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Google tries to auto-generate a snippet of a site’s content to show in search results. Currently it’s not doing a great job of this for Notify. There’s a chance that if we give it better content in the site’s meta description then it will use that instead. Worth a go… The content is adapted from the blue box on the product page. It’s 145 characters, which is within the 160 characters recommended[1] It matches the content in the page, and contains words that users are likely to be searching for (GOV.UK Notify, emails, text messages). It’s only on the homepage, because it shouldn’t be duplicated across multiple pages. https://yoast.com/meta-descriptions/ --- app/templates/admin_template.html | 2 ++ app/templates/views/signedout.html | 4 ++++ tests/app/main/views/test_index.py | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 1c8acd287..2789bfec9 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -18,6 +18,8 @@ + {% block meta %} + {% endblock %} {% endblock %} {% block page_title %} diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index 64776c5e6..37954b1b2 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -1,5 +1,9 @@ {% extends "fullwidth_template.html" %} +{% block meta %} + +{% endblock %} + {% block page_title %} GOV.UK Notify {% endblock %} diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 2e46706e9..1f6e46b33 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -1,7 +1,21 @@ import pytest +from bs4 import BeautifulSoup from flask import url_for +def test_non_logged_in_user_can_see_homepage( + client, +): + response = client.get(url_for('main.index')) + assert response.status_code == 200 + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.select_one('meta[name=description]')['content'].startswith( + 'GOV.UK Notify lets your send emails and text messages' + ) + + def test_logged_in_user_redirects_to_choose_service( logged_in_client, api_user_active, @@ -27,6 +41,10 @@ def test_static_pages( response = client.get(url_for('main.{}'.format(view))) assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert not page.select_one('meta[name=description]') + @pytest.mark.parametrize('view, expected_anchor', [ ('delivery_and_failure', 'messagedeliveryandfailure'), From 96b1241806956632864fecb206e75b4e1fb550fb Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Wed, 15 Nov 2017 12:05:25 +0000 Subject: [PATCH 6/8] Update pytest from 3.2.3 to 3.2.5 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 83822764e..177faa303 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,5 +1,5 @@ -r requirements.txt -pytest==3.2.3 +pytest==3.2.5 pytest-mock==1.6.3 pytest-cov==2.5.1 pytest-xdist==1.20.1 From 427300f201ce2b473c59d357c0bbd4c151f249a5 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 15 Nov 2017 13:24:04 +0000 Subject: [PATCH 7/8] Fix spelling mistake --- app/templates/views/signedout.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index 37954b1b2..5ef7221c6 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -1,7 +1,7 @@ {% extends "fullwidth_template.html" %} {% block meta %} - + {% endblock %} {% block page_title %} From d117f313a2c37607ac7d685bd01706f2ea30fcdf Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 15 Nov 2017 13:28:35 +0000 Subject: [PATCH 8/8] Fix typo in tests --- tests/app/main/views/test_index.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 1f6e46b33..d3961c54d 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -12,7 +12,7 @@ def test_non_logged_in_user_can_see_homepage( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select_one('meta[name=description]')['content'].startswith( - 'GOV.UK Notify lets your send emails and text messages' + 'GOV.UK Notify lets you send emails and text messages' )