diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 9a1b05c57..f28724c93 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -1,7 +1,7 @@ // Extra CSS overlaying elements #global-header-bar { - background-color: $red; + background-color: $govuk-blue; } #global-header { @@ -174,3 +174,8 @@ details summary { } } + +.phase-banner-beta { + border: 0; + margin-bottom: -$gutter + 2px; +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index a6d147629..7f6450c7b 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -30,6 +30,7 @@ $path: '/static/images/'; @import 'elements/layout'; @import 'elements/lists'; @import 'elements/panels'; +@import 'elements/phase-banner'; @import 'elements/tables'; diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index a1d579ef1..f043ddfca 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,5 +1,4 @@ # -*- coding: utf-8 -*- - import time import dateutil from datetime import datetime, timedelta, timezone @@ -21,7 +20,6 @@ from app import ( job_api_client, notification_api_client, service_api_client, - statistics_api_client, current_service, format_datetime_short) from app.main import main @@ -30,7 +28,7 @@ from app.utils import ( generate_previous_next_dict, user_has_permissions, generate_notifications_csv) -from app.statistics_utils import sum_of_statistics, statistics_by_state, add_rate_to_jobs +from app.statistics_utils import add_rate_to_jobs from app.utils import get_help_argument @@ -171,9 +169,6 @@ def view_notifications(service_id, message_type): template_type=[message_type], status=filter_args.get('status'), limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS']) - service_statistics_by_state = statistics_by_state(sum_of_statistics( - statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] - )) view_dict = dict( message_type=message_type, status=request.args.get('status') @@ -223,23 +218,11 @@ def view_notifications(service_id, message_type): message_type=message_type, status=request.args.get('status') ), - status_filters=[ - [ - item[0], item[1], - url_for( - '.view_notifications', - service_id=current_service['id'], - message_type=message_type, - status=item[1] - ), - service_statistics_by_state[message_type][item[0]] - ] for item in [ - ['processed', 'sending,delivered,failed'], - ['sending', 'sending'], - ['delivered', 'delivered'], - ['failed', 'failed'], - ] - ] + status_filters=get_status_filters( + current_service, + message_type, + service_api_client.get_detailed_service(service_id)['data']['statistics'] + ) ) @@ -261,6 +244,34 @@ def view_notification(service_id, job_id, notification_id): ) +def get_status_filters(service, message_type, statistics): + stats = statistics[message_type] + stats['sending'] = stats['requested'] - stats['delivered'] - stats['failed'] + + filters = [ + # key, label, option + ('requested', 'processed', 'sending,delivered,failed'), + ('sending', 'sending', 'sending'), + ('delivered', 'delivered', 'delivered'), + ('failed', 'failed', 'failed'), + ] + return [ + # return list containing label, option, link, count + ( + label, + option, + url_for( + '.view_notifications', + service_id=service['id'], + message_type=message_type, + status=option + ), + stats[key] + ) + for key, label, option in filters + ] + + def _get_job_counts(job, help_argument): return [ ( diff --git a/app/notify_client/statistics_api_client.py b/app/notify_client/statistics_api_client.py index 8a9a329c7..ec0a5993a 100644 --- a/app/notify_client/statistics_api_client.py +++ b/app/notify_client/statistics_api_client.py @@ -1,5 +1,4 @@ from notifications_python_client.base import BaseAPIClient -from notifications_python_client.errors import HTTPError class StatisticsApiClient(BaseAPIClient): @@ -13,15 +12,6 @@ class StatisticsApiClient(BaseAPIClient): self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] self.secret = app.config['ADMIN_CLIENT_SECRET'] - def get_statistics_for_service(self, service_id, limit_days=None): - params = {} - if limit_days is not None: - params['limit_days'] = limit_days - return self.get( - url='/service/{}/notifications-statistics'.format(service_id), - params=params - ) - def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None): params = {} if date_from is not None: diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 73b195e10..02564240d 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -5,9 +5,11 @@ + {% if current_user.is_authenticated %} + {% endif %} diff --git a/app/templates/partials/check/too_many_messages.html b/app/templates/partials/check/too_many_messages.html new file mode 100644 index 000000000..457c7321e --- /dev/null +++ b/app/templates/partials/check/too_many_messages.html @@ -0,0 +1,26 @@ +
+ {% call banner_wrapper(type='dangerous') %} +

+ Too many recipients +

+

+ You can only send {{ current_service.message_limit }} messages per day + {%- if current_service.restricted %} + in trial mode + {%- endif -%} + . +

+

+ {% if statistics.emails_requested or statistics.sms_requested %} + You can still send + {{ current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0) }} + messages today, but + {% endif %} + ‘{{ original_file_name }}’ contains + {{ count_of_recipients }} {{ recipients.recipient_column_header }} + {%- if count_of_recipients != 1 -%} + {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }} + {%- endif %}. +

+ {% endcall %} +
diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index b9158511e..44e784b89 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -9,6 +9,13 @@ {% block maincolumn_content %} +
+

+ BETA + This is a new service – your feedback will help us to improve it. +

+
+ {% call banner_wrapper(type='intro') %}

GOV.UK Notify makes it easy to send text messages and emails to diff --git a/config.py b/config.py index e304d39f9..37df9d749 100644 --- a/config.py +++ b/config.py @@ -54,7 +54,8 @@ class Config(object): "nhs\.net", "police\.uk", "kainos\.com", - "salesforce\.com"] + "salesforce\.com", + "bitzesty\.com"] class Development(Config): diff --git a/config_live.py b/config_live.py index fa32346ba..e01535e00 100644 --- a/config_live.py +++ b/config_live.py @@ -14,3 +14,4 @@ class Live(Config): DESKPRO_API_KEY = os.environ['LIVE_DESKPRO_API_KEY'] DESKPRO_DEPT_ID = os.environ['LIVE_DESKPRO_DEPT_ID'] DESKPRO_ASSIGNED_AGENT_TEAM_ID = os.environ['LIVE_DESKPRO_ASSIGNED_AGENT_TEAM_ID'] + HEADER_COLOUR = '#B10E1E' # $red diff --git a/config_staging.py b/config_staging.py index 091ef44af..775a2d566 100644 --- a/config_staging.py +++ b/config_staging.py @@ -13,3 +13,4 @@ class Staging(Config): DESKPRO_API_KEY = os.environ['STAGING_DESKPRO_API_KEY'] DESKPRO_DEPT_ID = os.environ['STAGING_DESKPRO_DEPT_ID'] DESKPRO_ASSIGNED_AGENT_TEAM_ID = os.environ['STAGING_DESKPRO_ASSIGNED_AGENT_TEAM_ID'] + HEADER_COLOUR = '#F47738' # $orange diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index f707e310b..137846f55 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -311,7 +311,6 @@ def test_new_invited_user_verifies_and_added_to_service(app_, mock_accept_invite, mock_get_service, mock_get_service_templates, - mock_get_service_statistics, mock_get_template_statistics, mock_get_jobs, mock_has_permissions, diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index f93f70d7c..dc5a35d80 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -65,7 +65,6 @@ def test_get_started( api_user_active, mock_get_service, mock_get_service_templates_when_no_templates_exist, - mock_get_service_statistics, mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, @@ -95,7 +94,6 @@ def test_get_started_is_hidden_once_templates_exist( api_user_active, mock_get_service, mock_get_service_templates, - mock_get_service_statistics, mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, @@ -166,7 +164,6 @@ def test_should_show_all_templates_on_template_statistics_page( api_user_active, mock_get_service, mock_get_service_templates, - mock_get_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -207,7 +204,6 @@ def test_should_show_recent_jobs_on_dashboard( api_user_active, mock_get_service, mock_get_service_templates, - mock_get_service_statistics, mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, @@ -253,7 +249,6 @@ def _test_dashboard_menu(mocker, app_, usr, service, permissions): mocker.patch('app.user_api_client.get_user', return_value=usr) mocker.patch('app.user_api_client.get_user_by_email', return_value=usr) mocker.patch('app.service_api_client.get_service', return_value={'data': service}) - mocker.patch('app.statistics_api_client.get_statistics_for_service', return_value={'data': [{}]}) client.login(usr) return client.get(url_for('main.service_dashboard', service_id=service['id'])) @@ -392,7 +387,6 @@ def test_route_for_service_permissions(mocker, mock_get_user, mock_get_service_templates, mock_get_jobs, - mock_get_service_statistics, mock_get_template_statistics, mock_get_detailed_service, mock_get_usage): @@ -435,7 +429,6 @@ def test_service_dashboard_updates_gets_dashboard_totals(mocker, mock_get_template_statistics, mock_get_detailed_service, mock_get_jobs, - mock_get_service_statistics, mock_get_usage): dashboard_totals = mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={ 'email': {'requested': 123, 'delivered': 0, 'failed': 0}, diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 2a42ba219..7e3378e63 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -1,12 +1,13 @@ +import json +from urllib.parse import quote + import pytest from flask import url_for from bs4 import BeautifulSoup -import json + from app.utils import generate_notifications_csv -from app.main.views.jobs import get_time_left -from tests import (notification_json, job_json) -from tests.conftest import fake_uuid -from tests.conftest import mock_get_job as mock_get_job1 +from app.main.views.jobs import get_time_left, get_status_filters +from tests import notification_json from freezegun import freeze_time @@ -57,7 +58,6 @@ def test_should_show_page_for_one_job( service_one, active_user_with_permissions, mock_get_service_template, - mock_get_service_statistics, mock_get_job, mocker, mock_get_notifications, @@ -111,7 +111,6 @@ def test_should_show_not_show_csv_download_in_tour( service_one, active_user_with_permissions, mock_get_service_template, - mock_get_service_statistics, mock_get_job, mocker, mock_get_notifications, @@ -147,7 +146,6 @@ def test_should_show_updates_for_one_job_as_json( service_one, active_user_with_permissions, mock_get_notifications, - mock_get_service_statistics, mock_get_job, mocker, fake_uuid @@ -203,7 +201,7 @@ def test_can_show_notifications( service_one, active_user_with_permissions, mock_get_notifications, - mock_get_service_statistics, + mock_get_detailed_service, mocker, message_type, page_title, @@ -263,7 +261,7 @@ def test_should_show_notifications_for_a_service_with_next_previous( service_one, active_user_with_permissions, mock_get_notifications_with_previous_next, - mock_get_service_statistics, + mock_get_detailed_service, mocker ): with app_.test_request_context(): @@ -321,3 +319,41 @@ def test_should_download_notifications_for_a_job(app_, @freeze_time("2016-01-10 12:00:00.000000") def test_time_left(job_created_at, expected_message): assert get_time_left(job_created_at) == expected_message + + +STATISTICS = { + 'sms': { + 'requested': 6, + 'failed': 2, + 'delivered': 1 + } +} + + +def test_get_status_filters_calculates_stats(app_): + with app_.test_request_context(): + ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS) + + assert {label: count for label, _option, _link, count in ret} == { + 'processed': 6, + 'sending': 3, + 'failed': 2, + 'delivered': 1 + } + + +def test_get_status_filters_in_right_order(app_): + with app_.test_request_context(): + ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS) + + assert [label for label, _option, _link, _count in ret] == [ + 'processed', 'sending', 'delivered', 'failed' + ] + + +def test_get_status_filters_constructs_links(app_): + with app_.test_request_context(): + ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS) + + link = ret[0][2] + assert link == '/services/foo/notifications/sms?status={}'.format(quote('sending,delivered,failed')) diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 6cfe07a38..667a3e332 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -17,7 +17,6 @@ def test_sign_out_user(app_, mock_get_user_by_email, mock_login, mock_get_service_templates, - mock_get_service_statistics, mock_get_jobs, mock_has_permissions, mock_get_template_statistics, diff --git a/tests/app/notify_client/test_statistics_client.py b/tests/app/notify_client/test_statistics_client.py deleted file mode 100644 index 0c9cb67b8..000000000 --- a/tests/app/notify_client/test_statistics_client.py +++ /dev/null @@ -1,32 +0,0 @@ -import uuid -from datetime import datetime - -from app.notify_client.statistics_api_client import StatisticsApiClient - - -def test_notifications_statistics_client_calls_correct_api_endpoint(mocker, api_user_active): - - some_service_id = uuid.uuid4() - expected_url = '/service/{}/notifications-statistics'.format(some_service_id) - - client = StatisticsApiClient() - - mock_get = mocker.patch('app.notify_client.statistics_api_client.StatisticsApiClient.get') - - client.get_statistics_for_service(some_service_id) - - mock_get.assert_called_once_with(url=expected_url, params={}) - - -def test_notifications_statistics_client_calls_correct_api_endpoint_with_params(mocker, api_user_active): - - some_service_id = uuid.uuid4() - expected_url = '/service/{}/notifications-statistics'.format(some_service_id) - - client = StatisticsApiClient() - - mock_get = mocker.patch('app.notify_client.statistics_api_client.StatisticsApiClient.get') - - client.get_statistics_for_service(some_service_id, limit_days=99) - - mock_get.assert_called_once_with(url=expected_url, params={'limit_days': 99}) diff --git a/tests/conftest.py b/tests/conftest.py index 29b2b8c34..79883f9b7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -216,15 +216,6 @@ def mock_delete_service(mocker, mock_get_service): 'app.service_api_client.delete_service', side_effect=_delete) -@pytest.fixture(scope='function') -def mock_get_service_statistics(mocker): - def _create(service_id, limit_days=None): - return {'data': [{}]} - - return mocker.patch( - 'app.statistics_api_client.get_statistics_for_service', side_effect=_create) - - @pytest.fixture(scope='function') def mock_get_aggregate_service_statistics(mocker): def _create(service_id, limit_days=None):