From 9863aa3c48ce81b9d77e44c96c9ea4bca78d5a81 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 10 Apr 2019 17:20:51 +0100 Subject: [PATCH] Automate counting of live services and orgs Returns the data calculated by the API. Stored in Redis against a hardcoded key so that no-one hammering the home page is directly hitting the database. --- app/main/views/index.py | 9 +++++++-- app/main/views/service_settings.py | 7 +------ app/models/service.py | 3 +++ app/notify_client/organisations_api_client.py | 1 + app/notify_client/service_api_client.py | 8 ++++++++ app/notify_client/status_api_client.py | 7 +++++-- app/templates/views/signedout.html | 6 +++--- tests/app/main/views/test_headers.py | 14 ++++++++++++-- tests/app/main/views/test_index.py | 13 +++++++++++++ tests/conftest.py | 8 ++++++++ 10 files changed, 61 insertions(+), 15 deletions(-) diff --git a/app/main/views/index.py b/app/main/views/index.py index 1247c0aa2..2d7423661 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -12,7 +12,7 @@ from notifications_utils.international_billing_rates import ( ) from notifications_utils.template import HTMLEmailTemplate, LetterImageTemplate -from app import email_branding_client, letter_branding_client +from app import email_branding_client, letter_branding_client, status_api_client from app.main import main from app.main.forms import FieldWithNoneOption, SearchByNameForm from app.main.views.sub_navigation_dictionaries import features_nav @@ -21,9 +21,14 @@ from app.utils import AgreementInfo, get_logo_cdn_domain @main.route('/') def index(): + if current_user and current_user.is_authenticated: return redirect(url_for('main.choose_account')) - return render_template('views/signedout.html') + + return render_template( + 'views/signedout.html', + counts=status_api_client.get_count_of_live_services_and_organisations(), + ) @main.route('/robots.txt') diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index ba20ccbb1..f7989fe14 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -262,12 +262,7 @@ def service_switch_live(service_id): ) if form.validate_on_submit(): - current_service.update( - # TODO This limit should be set depending on the agreement signed by - # with Notify. - message_limit=250000 if form.enabled.data else 50, - restricted=(not form.enabled.data) - ) + current_service.update_status(live=form.enabled.data) return redirect(url_for('.service_settings', service_id=service_id)) return render_template( diff --git a/app/models/service.py b/app/models/service.py index 64805f430..30ee5fc0c 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -72,6 +72,9 @@ class Service(): def update(self, **kwargs): return service_api_client.update_service(self.id, **kwargs) + def update_status(self, live): + return service_api_client.update_status(self.id, live=live) + def switch_permission(self, permission): return self.force_permission( permission, diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index d1d082e82..fab82246e 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -25,6 +25,7 @@ class OrganisationsClient(NotifyAdminAPIClient): return self.get(url="/service/{}/organisation".format(service_id)) @cache.delete('service-{service_id}') + @cache.delete('live-service-and-organisation-counts') def update_service_organisation(self, service_id, org_id): data = { 'service_id': service_id diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index b2242ae1a..b79651dbd 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -98,6 +98,14 @@ class ServiceAPIClient(NotifyAdminAPIClient): endpoint = "/service/{0}".format(service_id) return self.post(endpoint, data) + @cache.delete('live-service-and-organisation-counts') + def update_status(self, service_id, live): + return self.update_service( + service_id, + message_limit=250000 if live else 50, + restricted=(not live), + ) + # This method is not cached because it calls through to one which is def update_service_with_properties(self, service_id, properties): return self.update_service(service_id, **properties) diff --git a/app/notify_client/status_api_client.py b/app/notify_client/status_api_client.py index 3925a77bd..7228621c6 100644 --- a/app/notify_client/status_api_client.py +++ b/app/notify_client/status_api_client.py @@ -1,5 +1,4 @@ - -from app.notify_client import NotifyAdminAPIClient +from app.notify_client import NotifyAdminAPIClient, cache class StatusApiClient(NotifyAdminAPIClient): @@ -7,5 +6,9 @@ class StatusApiClient(NotifyAdminAPIClient): def get_status(self, *params): return self.get(url='/_status', *params) + @cache.set('live-service-and-organisation-counts') + def get_count_of_live_services_and_organisations(self): + return self.get(url='/_status/live-service-and-organisation-counts') + status_api_client = StatusApiClient() diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index ae7ab262c..14b5d545e 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -115,17 +115,17 @@
-
+

Who’s using GOV.UK Notify

-
745
+
{{ counts.services }}
services
-
233
+
{{ counts.organisations }}
organisations
diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py index 853bbba1a..9f5858240 100644 --- a/tests/app/main/views/test_headers.py +++ b/tests/app/main/views/test_headers.py @@ -1,4 +1,10 @@ -def test_owasp_useful_headers_set(client, mocker): + + +def test_owasp_useful_headers_set( + client, + mocker, + mock_get_service_and_organisation_counts, +): mocker.patch('app.get_logo_cdn_domain', return_value='static-logos.test.com') response = client.get('/') @@ -19,7 +25,11 @@ def test_owasp_useful_headers_set(client, mocker): ) -def test_headers_non_ascii_characters_are_replaced(client, mocker): +def test_headers_non_ascii_characters_are_replaced( + client, + mocker, + mock_get_service_and_organisation_counts, +): mocker.patch('app.get_logo_cdn_domain', return_value='static-logos€æ.test.com') response = client.get('/') diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 211d2b25e..2d19801e0 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -14,6 +14,7 @@ from tests.conftest import ( def test_non_logged_in_user_can_see_homepage( client, + mock_get_service_and_organisation_counts, ): response = client.get(url_for('main.index')) assert response.status_code == 200 @@ -30,6 +31,18 @@ def test_non_logged_in_user_can_see_homepage( 'local authority, or the NHS.' ) + assert normalize_spaces(page.select_one('#whos-using-notify').text) == ( + 'Who’s using GOV.UK Notify ' + 'Services ' + '9999 services ' + 'Organisations ' + '111 organisations ' + 'See the list of services and organisations.' + ) + assert page.select_one('#whos-using-notify a')['href'] == ( + 'https://www.gov.uk/performance/govuk-notify/government-services' + ) + def test_logged_in_user_redirects_to_choose_account( client_request, diff --git a/tests/conftest.py b/tests/conftest.py index b07b375da..ddc2c3efa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3351,3 +3351,11 @@ def mock_move_to_template_folder(mocker): @pytest.fixture def mock_create_template_folder(mocker): return mocker.patch('app.template_folder_api_client.create_template_folder', return_value=sample_uuid()) + + +@pytest.fixture(scope='function') +def mock_get_service_and_organisation_counts(mocker): + return mocker.patch('app.status_api_client.get_count_of_live_services_and_organisations', return_value={ + 'organisations': 111, + 'services': 9999, + })