diff --git a/app/assets/images/email-template/crests/data_gov_uk_x2.png b/app/assets/images/email-template/crests/data_gov_uk_x2.png new file mode 100644 index 000000000..e006ef534 Binary files /dev/null and b/app/assets/images/email-template/crests/data_gov_uk_x2.png differ diff --git a/app/assets/stylesheets/_grids.scss b/app/assets/stylesheets/_grids.scss index c7becdee2..4fbd20f8f 100644 --- a/app/assets/stylesheets/_grids.scss +++ b/app/assets/stylesheets/_grids.scss @@ -26,6 +26,19 @@ @include grid-column(7/8); } +%top-gutter, +.top-gutter { + @extend %contain-floats; + display: block; + margin-top: $gutter; + clear: both; +} + +.top-gutter-4-3 { + @extend %top-gutter; + margin-top: $gutter * 4 / 3; +} + %bottom-gutter, .bottom-gutter { @extend %contain-floats; diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index 2375757dd..d2cfba1b0 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -129,3 +129,26 @@ } } + +.big-number-meta-wrapper { + + position: relative; + margin: $gutter-half 0 $gutter 0; + background: $govuk-blue; + + .big-number-meta { + + padding: ($gutter / 3) $gutter-half; + color: $white; + pointer-events: none; + + @include media(desktop) { + position: absolute; + bottom: 7px; + right: 5px; + text-align: right; + } + + } + +} diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index 04b6a31b1..e40c42a35 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -59,3 +59,21 @@ } } + +.align-button-with-textbox { + + .button { + + @include media(desktop) { + position: relative; + top: 32px; + left: -30px; + width: 100%; + margin-right: -30px; + padding-top: 8px; + box-sizing: content-box; + } + + } + +} diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index a5a79e6d6..96efbf308 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -1,6 +1,8 @@ -%sms-message-wrapper, +$tail-angle: 20deg; + .sms-message-wrapper { + position: relative; width: 100%; max-width: 464px; box-sizing: border-box; @@ -13,21 +15,20 @@ clear: both; word-wrap: break-word; - p { - margin: 0; - line-height: 1.6; + &:after { + content: ""; + display: block; + position: absolute; + bottom: -4px; + right: -20px; + border: 10px solid transparent; + border-left-width: 13px; + border-right-width: 13px; + border-bottom-color: $panel-colour; + border-left-color: $panel-colour; + transform: rotate($tail-angle); } - p + p { - margin-top: 20px; - } - -} - -.sms-message-wrapper-with-radio { - @extend %sms-message-wrapper; - padding-left: 45px; - cursor: pointer; } .sms-message-recipient { @@ -35,22 +36,3 @@ color: $secondary-text-colour; margin: 10px 0 0 0; } - -.sms-message-name { - @include bold-24; - margin: 20px 0 5px 0; -} - -.sms-message-picker { - display: block; - margin: 7px 0 0 0; - position: absolute; - left: 15px; - top: 50%; - z-index: 50; -} - -.sms-message-from { - @include bold-19; - display: block; -} diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 085b16e6f..152d209be 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -74,6 +74,12 @@ } + &-invisible-error { + border-left: 5px solid transparent; + padding-left: 7px; + display: block; + } + &-status { &-default { @@ -123,6 +129,7 @@ } &-index { + @include bold-16; width: 15px; } @@ -132,6 +139,10 @@ } +.table-font-xsmall td.table-field-index { // overrides GOV.UK Elements + @include bold-16; +} + .table-field-headings, .table-field-headings-visible { @@ -199,3 +210,11 @@ a.table-show-more-link { border-bottom: 1px solid $border-colour; padding: 0.75em 0 0.5625em 0; } + +.wide-left-hand-column { + display: block; + max-width: 560px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index a99623384..a1fed973a 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -94,3 +94,7 @@ } } + +.align-with-message-body { + margin-top: $gutter * 5 / 6; +} diff --git a/app/config.py b/app/config.py index 32cbe76d4..c19849a7e 100644 --- a/app/config.py +++ b/app/config.py @@ -57,9 +57,7 @@ class Config(object): CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' DESKPRO_PERSON_EMAIL = 'donotreply@notifications.service.gov.uk' ACTIVITY_STATS_LIMIT_DAYS = 7 - TEST_MESSAGE_FILENAME = 'Test message' - - SMS_FREE_TIER_AMOUNT = 250000 + TEST_MESSAGE_FILENAME = 'Report' STATSD_ENABLED = False STATSD_HOST = "statsd.hostedgraphite.com" @@ -83,6 +81,8 @@ class Config(object): r"hmcts\.net", r"scotent\.co\.uk", r"assembly\.wales", + r"cjsm\.net", + r"cqc\.org\.uk", ] @@ -100,6 +100,7 @@ class Test(Development): WTF_CSRF_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' NOTIFY_ENVIRONMENT = 'test' + TEMPLATE_PREVIEW_API_HOST = 'http://localhost:9999' class Preview(Config): diff --git a/app/main/forms.py b/app/main/forms.py index 34a6d8616..f8db6e577 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -7,6 +7,7 @@ from notifications_utils.recipients import ( validate_phone_number, InvalidPhoneError ) +from notifications_utils.columns import Columns from wtforms import ( validators, StringField, @@ -102,11 +103,26 @@ class UKMobileNumber(TelField): raise ValidationError(str(e)) -def mobile_number(): - return UKMobileNumber('Mobile number', +class InternationalPhoneNumber(TelField): + def pre_validate(self, form): + try: + validate_phone_number(self.data, international=True) + except InvalidPhoneError as e: + raise ValidationError(str(e)) + + +def mobile_number(label='Mobile number'): + return UKMobileNumber(label, validators=[DataRequired(message='Can’t be empty')]) +def international_phone_number(label='Mobile number'): + return InternationalPhoneNumber( + label, + validators=[DataRequired(message='Can’t be empty')] + ) + + def password(label='Password'): return PasswordField(label, validators=[DataRequired(message='Can’t be empty'), @@ -502,7 +518,11 @@ class ServiceSmsSender(Form): class ServiceLetterContactBlock(Form): - letter_contact_block = TextAreaField() + letter_contact_block = TextAreaField( + validators=[ + NoCommasInPlaceHolders() + ] + ) def validate_letter_contact_block(form, field): line_count = field.data.strip().count('\n') @@ -621,6 +641,11 @@ class SearchTemplatesForm(Form): search = SearchField('Search by name') +class SearchNotificationsForm(Form): + + to = SearchField('Search by phone number or email address') + + class PlaceholderForm(Form): pass @@ -629,15 +654,25 @@ class PlaceholderForm(Form): def get_placeholder_form_instance( placeholder_name, dict_to_populate_from, - optional_placeholder=False + optional_placeholder=False, + allow_international_phone_numbers=False, ): - PlaceholderForm.placeholder_value = StringField( - placeholder_name, - validators=[ + if Columns.make_key(placeholder_name) == 'emailaddress': + field = email_address(label=placeholder_name, gov_user=False) + elif Columns.make_key(placeholder_name) == 'phonenumber': + if allow_international_phone_numbers: + field = international_phone_number(label=placeholder_name) + else: + field = mobile_number(label=placeholder_name) + elif optional_placeholder: + field = StringField(placeholder_name) + else: + field = StringField(placeholder_name, validators=[ DataRequired(message='Can’t be empty') - ] if not optional_placeholder else [] - ) + ]) + + PlaceholderForm.placeholder_value = field return PlaceholderForm( placeholder_value=dict_to_populate_from.get(placeholder_name, '') diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 45736ac1e..4e8893f67 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -2,7 +2,6 @@ from datetime import datetime from functools import partial from flask import ( render_template, - current_app, url_for, session, jsonify, @@ -13,6 +12,7 @@ from flask_login import login_required from app.main import main from app import ( + current_service, job_api_client, service_api_client, template_statistics_client @@ -136,6 +136,20 @@ def monthly(service_id): ) +@main.route("/services//inbox") +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def inbox(service_id): + + if 'inbound_sms' not in current_service['permissions']: + abort(403) + + return render_template( + 'views/dashboard/inbox.html', + messages=service_api_client.get_inbound_sms(service_id), + ) + + def aggregate_usage(template_statistics, sort_key='count'): return sorted( template_statistics, @@ -167,6 +181,13 @@ def get_dashboard_partials(service_id): 'views/dashboard/_upcoming.html', scheduled_jobs=scheduled_jobs ), + 'inbox': render_template( + 'views/dashboard/_inbox.html', + inbound_sms_summary=( + service_api_client.get_inbound_sms_summary(service_id) + if 'inbound_sms' in current_service['permissions'] else None + ), + ), 'totals': render_template( 'views/dashboard/_totals.html', service_id=service_id, @@ -213,7 +234,8 @@ def calculate_free_tier_usage(usage, service): def calculate_usage(usage): - sms_free_allowance = current_app.config['SMS_FREE_TIER_AMOUNT'] + # TODO: Don't hardcode these - get em from the API + sms_free_allowance = 250000 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/feedback.py b/app/main/views/feedback.py index 775d79878..6f1631a2f 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -8,6 +8,11 @@ from app.main.forms import SupportType, Feedback, Problem, Triage from datetime import datetime +@main.route('/feedback', methods=['GET']) +def old_feedback(): + return redirect(url_for('.support')) + + @main.route('/support', methods=['GET', 'POST']) def support(): form = SupportType() diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index a44c41581..1e85faf0b 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -26,6 +26,7 @@ from app import ( current_service, format_datetime_short) from app.main import main +from app.main.forms import SearchNotificationsForm from app.utils import ( get_page_from_request, generate_next_dict, @@ -197,8 +198,10 @@ def view_notifications(service_id, message_type): 'views/notifications.html', partials=get_notifications(service_id, message_type), message_type=message_type, - status=request.args.get('status'), - page=request.args.get('page', 1) + status=request.args.get('status') or 'sending,delivered,failed', + page=request.args.get('page', 1), + to=request.args.get('to'), + search_form=SearchNotificationsForm(to=request.args.get('to')), ) @@ -241,7 +244,9 @@ def get_notifications(service_id, message_type, status_override=None): page=page, template_type=[message_type], status=filter_args.get('status'), - limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS']) + limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'], + to=request.args.get('to'), + ) url_args = { 'message_type': message_type, @@ -249,11 +254,11 @@ def get_notifications(service_id, message_type, status_override=None): } prev_page = None - if notifications['links'].get('prev', None): + if 'links' in notifications and notifications['links'].get('prev', None): prev_page = generate_previous_dict('main.view_notifications', service_id, page, url_args=url_args) next_page = None - if notifications['links'].get('next', None): + if 'links' in notifications and notifications['links'].get('next', None): next_page = generate_next_dict('main.view_notifications', service_id, page, url_args) return { diff --git a/app/main/views/send.py b/app/main/views/send.py index 638f67cc0..de6be4ff1 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -157,14 +157,18 @@ def get_example_csv(service_id, template_id): } -@main.route("/services//send//test") +@main.route("/services//send//test", endpoint='send_test') +@main.route("/services//send//one-off", endpoint='send_one_off') @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_test(service_id, template_id): session['send_test_values'] = dict() session['send_test_letter_page_count'] = None return redirect(url_for( - '.send_test_step', + { + 'main.send_test': '.send_test_step', + 'main.send_one_off': '.send_one_off_step', + }[request.endpoint], service_id=service_id, template_id=template_id, step_index=0, @@ -172,14 +176,28 @@ def send_test(service_id, template_id): )) -@main.route("/services//send//test/step-", methods=['GET', 'POST']) +@main.route( + "/services//send//test/step-", + methods=['GET', 'POST'], + endpoint='send_test_step', +) +@main.route( + "/services//send//one-off/step-", + methods=['GET', 'POST'], + endpoint='send_one_off_step', +) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_test_step(service_id, template_id, step_index): if 'send_test_values' not in session: return redirect(url_for( - '.send_test', service_id=service_id, template_id=template_id + { + 'main.send_test_step': '.send_test', + 'main.send_one_off_step': '.send_one_off', + }[request.endpoint], + service_id=service_id, + template_id=template_id, )) template = service_api_client.get_service_template(service_id, template_id)['data'] @@ -201,7 +219,10 @@ def send_test_step(service_id, template_id, step_index): page_count=session['send_test_letter_page_count'] ) - placeholders = fields_to_fill_in(template) + placeholders = fields_to_fill_in( + template, + prefill_current_user=(request.endpoint == 'main.send_test_step'), + ) if len(placeholders) == 0: return make_and_upload_csv_file(service_id, template) @@ -209,28 +230,33 @@ def send_test_step(service_id, template_id, step_index): try: current_placeholder = placeholders[step_index] except IndexError: + if all_placeholders_in_session(placeholders): + return make_and_upload_csv_file(service_id, template) return redirect(url_for( - '.send_test', service_id=service_id, template_id=template_id + { + 'main.send_test_step': '.send_test', + 'main.send_one_off_step': '.send_one_off', + }[request.endpoint], + service_id=service_id, + template_id=template_id, )) optional_placeholder = (current_placeholder in optional_address_columns) form = get_placeholder_form_instance( current_placeholder, dict_to_populate_from=get_normalised_send_test_values_from_session(), optional_placeholder=optional_placeholder, + allow_international_phone_numbers=current_service['can_send_international_sms'], ) if form.validate_on_submit(): session['send_test_values'][current_placeholder] = form.placeholder_value.data - if all( - get_normalised_send_test_values_from_session().get(placeholder, False) not in (False, None) - for placeholder in placeholders - ): + if all_placeholders_in_session(placeholders): return make_and_upload_csv_file(service_id, template) return redirect(url_for( - '.send_test_step', + request.endpoint, service_id=service_id, template_id=template_id, step_index=step_index + 1, @@ -247,7 +273,7 @@ def send_test_step(service_id, template_id, step_index): ) else: back_link = url_for( - '.send_test_step', + request.endpoint, service_id=service_id, template_id=template_id, step_index=step_index - 1, @@ -256,13 +282,27 @@ def send_test_step(service_id, template_id, step_index): template.values = get_normalised_send_test_values_from_session() template.values[current_placeholder] = None + if ( + request.endpoint == 'main.send_one_off_step' and + step_index == 0 and + template.template_type != 'letter' + ): + skip_link = ( + 'Use my {}'.format(first_column_headings[template.template_type][0]), + url_for('.send_test', service_id=service_id, template_id=template.id), + ) + else: + skip_link = None + return render_template( 'views/send-test.html', + page_title=get_send_test_page_title(template.template_type, get_help_argument()), template=template, form=form, + skip_link=skip_link, optional_placeholder=optional_placeholder, - help=get_help_argument(), back_link=back_link, + help=get_help_argument(), ) @@ -470,11 +510,11 @@ def get_check_messages_back_url(service_id, template_type): return url_for('main.choose_template', service_id=service_id) -def fields_to_fill_in(template): +def fields_to_fill_in(template, prefill_current_user=False): recipient_columns = first_column_headings[template.template_type] - if 'letter' == template.template_type: + if 'letter' == template.template_type or not prefill_current_user: return recipient_columns + list(template.placeholders) session['send_test_values'][recipient_columns[0]] = { @@ -513,3 +553,18 @@ def make_and_upload_csv_file(service_id, template): from_test=True, help=2 if get_help_argument() else 0 )) + + +def all_placeholders_in_session(placeholders): + return all( + get_normalised_send_test_values_from_session().get(placeholder, False) not in (False, None) + for placeholder in placeholders + ) + + +def get_send_test_page_title(template_type, help_argument): + if help_argument: + return 'Example text message' + if template_type == 'letter': + return 'Print a test letter' + return 'Send to one recipient' diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index d891164ea..97cf75fad 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -14,6 +14,8 @@ from flask_login import ( login_required, current_user ) + +from notifications_utils.field import Field from notifications_python_client.errors import HTTPError from app import service_api_client @@ -46,7 +48,9 @@ def service_settings(service_id): organisation=organisation, letter_branding=letter_branding_organisations.get( current_service.get('dvla_organisation', '001') - ) + ), + can_receive_inbound=('inbound_sms' in current_service['permissions']), + letter_contact_block=Field(current_service['letter_contact_block'], html='escape') ) @@ -264,14 +268,27 @@ def service_set_reply_to_email(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): form = ServiceSmsSender() + if form.validate_on_submit(): + set_inbound_sms = request.args.get('set_inbound_sms', False) + if set_inbound_sms == 'True': + permissions = current_service['permissions'] + if 'inbound_sms' in permissions: + permissions.remove('inbound_sms') + else: + permissions.append('inbound_sms') + service_api_client.update_service_with_properties( + current_service['id'], + {'permissions': permissions, + 'sms_sender': form.sms_sender.data or None} + ) + else: + service_api_client.update_service( + current_service['id'], + sms_sender=form.sms_sender.data or None + ) + return redirect(url_for('.service_settings', service_id=service_id)) if request.method == 'GET': form.sms_sender.data = current_service.get('sms_sender') - if form.validate_on_submit(): - service_api_client.update_service( - current_service['id'], - sms_sender=form.sms_sender.data or None - ) - return redirect(url_for('.service_settings', service_id=service_id)) return render_template( 'views/service-settings/set-sms-sender.html', form=form) diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 7ea6831e9..4a4399723 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -21,7 +21,8 @@ class NotificationApiClient(NotifyAdminAPIClient): limit_days=None, include_jobs=None, include_from_test_key=None, - format_for_csv=None + format_for_csv=None, + to=None, ): params = {} if page is not None: @@ -38,6 +39,8 @@ class NotificationApiClient(NotifyAdminAPIClient): params['include_from_test_key'] = include_from_test_key if format_for_csv is not None: params['format_for_csv'] = format_for_csv + if to is not None: + params['to'] = to if job_id: return self.get( url='/service/{}/job/{}/notifications'.format(service_id, job_id), diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index fd672a464..da9bb9d34 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,4 +1,5 @@ from __future__ import unicode_literals + from flask import url_for from app.utils import BrowsableItem from app.notify_client import _attach_current_user, NotifyAdminAPIClient @@ -94,6 +95,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): 'organisation', 'letter_contact_block', 'dvla_organisation', + 'permissions' } if disallowed_attributes: raise TypeError('Not allowed to update service attributes: {}'.format( @@ -242,6 +244,16 @@ class ServiceAPIClient(NotifyAdminAPIClient): params=dict(year=year) ) + def get_inbound_sms(self, service_id): + return self.get( + '/service/{}/inbound-sms'.format(service_id) + )['data'] + + def get_inbound_sms_summary(self, service_id): + return self.get( + '/service/{}/inbound-sms/summary'.format(service_id) + ) + class ServicesBrowsableItem(BrowsableItem): @property diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index af0e56383..048a0f3df 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -31,22 +31,25 @@ failure_percentage, danger_zone=False, failure_link=None, - link=None + link=None, + show_failures=True ) %}
{{ big_number(number, label, link=link) }} -
- {% if failures %} - {% if failure_link %} - + {% if show_failures %} +
+ {% if failures %} + {% if failure_link %} + + {{ "{:,}".format(failures) }} failed – {{ failure_percentage }}% + + {% else %} {{ "{:,}".format(failures) }} failed – {{ failure_percentage }}% - + {% endif %} {% else %} - {{ "{:,}".format(failures) }} failed – {{ failure_percentage }}% + No failures {% endif %} - {% else %} - No failures - {% endif %} -
+
+ {% endif %}
{% endmacro %} diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 2925ee2f2..2697cc6f6 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -73,9 +73,9 @@ {%- endmacro %} -{% macro index_field(text) -%} +{% macro index_field(text=None) -%} - {{ text }} + {{ text if text != None else caller() }} {%- endmacro %} diff --git a/app/templates/views/activity/notifications.html b/app/templates/views/activity/notifications.html index 42c6cf169..06158fb75 100644 --- a/app/templates/views/activity/notifications.html +++ b/app/templates/views/activity/notifications.html @@ -21,7 +21,10 @@ {{ item.to }}

- {% if item.job %} + {% if item.job and item.job.original_file_name == 'Report' %} + {{ item.template.name }} + sent to one recipient + {% elif item.job %} From {{ item.job.original_file_name }} {% else %} {{ item.template.name }} diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 933bda753..d7819fca2 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -150,7 +150,9 @@ {% endif %} - {{ template|string }} + {% if not errors %} + {{ template|string }} + {% endif %}

{% if errors %} @@ -188,14 +190,18 @@ caption=original_file_name, caption_visible=False, field_headings=[ - ''|safe + ''.format("table-field-invisible-error" if errors else "")|safe ] + recipients.column_headers ) %} - {{ index_field(item.index + 2) }} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} {% for column in recipients.column_headers %} {% if item['columns'][column].error and not recipients.missing_column_headers %} {% call field() %} - + {{ item['columns'][column].error }} {{ item['columns'][column].data if item['columns'][column].data != None }} @@ -234,4 +240,9 @@

{% endif %} + {% if errors %} +

Preview of {{ template.name }}

+ {{ template|string }} + {% endif %} + {% endblock %} diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html new file mode 100644 index 000000000..37ad932e4 --- /dev/null +++ b/app/templates/views/dashboard/_inbox.html @@ -0,0 +1,21 @@ +{% from "components/big-number.html" import big_number, big_number_with_status %} + +
+ {% if inbound_sms_summary != None %} +
+ {{ + big_number_with_status( + inbound_sms_summary.count, + 'text messages received', + link=url_for('.inbox', service_id=current_service.id), + show_failures=False + ) + }} +
+ {% if inbound_sms_summary.most_recent %} + latest message {{ inbound_sms_summary.most_recent | format_delta }} + {% endif %} +
+
+ {% endif %} +
diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index d1cb19821..281a66f7b 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -6,7 +6,7 @@
{{ big_number_with_status( statistics['email']['requested'], - message_count_label(statistics['email']['requested'], 'email', suffix=''), + message_count_label(statistics['email']['requested'], 'email', suffix='sent'), statistics['email']['failed'], statistics['email']['failed_percentage'], statistics['email']['show_warning'], @@ -17,7 +17,7 @@
{{ big_number_with_status( statistics['sms']['requested'], - message_count_label(statistics['sms']['requested'], 'sms', suffix=''), + message_count_label(statistics['sms']['requested'], 'sms', suffix='sent'), statistics['sms']['failed'], statistics['sms']['failed_percentage'], statistics['sms']['show_warning'], diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index c18fbb4e4..8e7e58f40 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -29,6 +29,8 @@ In the last 7 days + {{ ajax_block(partials, updates_url, 'inbox') }} + {{ ajax_block(partials, updates_url, 'totals') }} {{ show_more( url_for('.monthly', service_id=current_service.id), diff --git a/app/templates/views/dashboard/inbox.html b/app/templates/views/dashboard/inbox.html new file mode 100644 index 000000000..ace1cafbd --- /dev/null +++ b/app/templates/views/dashboard/inbox.html @@ -0,0 +1,43 @@ +{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading, row_heading %} +{% from "components/message-count-label.html" import message_count_label %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + Inbox +{% endblock %} + +{% block maincolumn_content %} + +

+ Received text messages +

+
+ {% call(item, row_number) list_table( + messages, + caption="Inbox", + caption_visible=False, + empty_message='When users text your service’s phone number ({}) you’ll see the messages here'.format(current_service.sms_sender), + field_headings=[ + 'From', + 'First two lines of message' + ], + field_headings_visible=False + ) %} + {% call field() %} + {{ item.user_number }} + {{ item.content }} + {% endcall %} + {% call field(align='right') %} + + {{ item.created_at | format_delta }} + + {% endcall %} + {% endcall %} + {% if messages %} +

+ Showing all {{ messages | length }} messages +

+ {% endif %} +
+{% endblock %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 61f16aff7..3190b4fa0 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -1,6 +1,8 @@ {% extends "withnav_template.html" %} {% from "components/ajax-block.html" import ajax_block %} {% from "components/message-count-label.html" import message_count_label, recipient_count_label %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/textbox.html" import textbox %} {% block service_page_title %} {{ message_count_label(99, message_type, suffix='') | capitalize }} @@ -18,9 +20,27 @@ 'counts' ) }} +
+
+ + {{ textbox( + search_form.to, + width='1-1', + label='Search by {}'.format('email address' if message_type == 'email' else 'phone number') + ) }} +
+
+ +
+
+ {{ ajax_block( partials, - url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page), + url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page, to=to), 'notifications' ) }} diff --git a/app/templates/views/send-test.html b/app/templates/views/send-test.html index c7b0b0e2e..bae3939cc 100644 --- a/app/templates/views/send-test.html +++ b/app/templates/views/send-test.html @@ -5,32 +5,30 @@ {% from "components/table.html" import list_table, field, text_field, index_field, index_field_heading %} {% block service_page_title %} - {% if request.args['help'] %} - Example text message - {% else %} - Send yourself a test - {% endif %} + {{ page_title }} {% endblock %} {% block maincolumn_content %}

- {% if request.args['help'] %} - Example text message - {% else %} - {% if template.template_type == 'letter' %} - Print a test letter - {% else %} - Send yourself a test - {% endif %} - {% endif %} + {{ page_title }}

- {{ textbox( - form.placeholder_value, - hint='Optional' if optional_placeholder else None - ) }} +
+
+ {{ textbox( + form.placeholder_value, + hint='Optional' if optional_placeholder else None, + width='1-1', + ) }} +
+ {% if skip_link %} +
+ {{ skip_link[0] }} +
+ {% endif %} +
{{ page_footer('Next', back_link=back_link) }}
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 153069c93..0a2490b9b 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -36,8 +36,13 @@ {% call row() %} {{ text_field('Text message sender') }} - {{ text_field(current_service.sms_sender or 'GOVUK') }} - {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }} + {{ text_field(current_service.sms_sender) }} + {% if current_user.has_permissions([], admin_override=True) or not can_receive_inbound %} + {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id, set_inbound_sms=False)) }} + {% else %} + {{ text_field('') }} + {% endif %} + {% endcall %} {% call row() %} @@ -56,7 +61,7 @@ {% call row() %} {{ text_field('Letter contact details') }} {% call field(status='' if current_service.letter_contact_block else 'default') %} - {{ current_service.letter_contact_block | escape | nl2br | safe }} + {{ letter_contact_block | string | nl2br | safe if current_service.letter_contact_block else 'None'}} {% endcall %} {{ edit_field('Change', url_for('.service_set_letter_contact_block', service_id=current_service.id)) }} {% endcall %} @@ -162,6 +167,11 @@ {% endif %} +
  • + + {{ 'Stop inbound sms' if can_receive_inbound else 'Allow inbound sms' }} + +
    • {% endif %} diff --git a/app/templates/views/service-settings/set-letter-contact-block.html b/app/templates/views/service-settings/set-letter-contact-block.html index c33121942..ddd46ffe6 100644 --- a/app/templates/views/service-settings/set-letter-contact-block.html +++ b/app/templates/views/service-settings/set-letter-contact-block.html @@ -18,7 +18,8 @@ label='How should users contact your service?
    This applies to all the letters you send.'|safe, hint='10 lines maximum', width='1-1', - rows=10 + rows=10, + highlight_tags=True ) }} {{ page_footer( 'Save', diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index 702924301..bd37ad7a5 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -117,7 +117,7 @@
    -
    51
    +
    54
    services
    diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index e74e8e0a5..6a2add4df 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -13,8 +13,8 @@
    - - {{ 'Print a test letter' if template.template_type == 'letter' else 'Send yourself a test' }} + + {{ 'Print a test letter' if template.template_type == 'letter' else 'Send to one recipient' }}
    {% endif %} diff --git a/requirements.txt b/requirements.txt index 738e5f12f..98ed8f5bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ notifications-python-client>=3.1,<3.2 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@17.0.1#egg=notifications-utils==17.0.1 +git+https://github.com/alphagov/notifications-utils.git@17.1.2#egg=notifications-utils==17.1.2 diff --git a/tests/__init__.py b/tests/__init__.py index 10a48f9f6..19e724a8a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -47,17 +47,20 @@ def service_json( restricted=True, email_from=None, reply_to_email_address=None, - sms_sender=None, + sms_sender='GOVUK', research_mode=False, can_send_letters=False, can_send_international_sms=False, organisation=None, branding='govuk', created_at=None, - letter_contact_block=None + letter_contact_block=None, + permissions=None, ): if users is None: users = [] + if permissions is None: + permissions = [] return { 'id': id_, 'name': name, @@ -76,6 +79,7 @@ def service_json( 'created_at': created_at or str(datetime.utcnow()), 'letter_contact_block': letter_contact_block, 'dvla_organisation': '001', + 'permissions': permissions, } diff --git a/tests/app/main/test_placeholder_form.py b/tests/app/main/test_placeholder_form.py index 07fac6675..4c6093944 100644 --- a/tests/app/main/test_placeholder_form.py +++ b/tests/app/main/test_placeholder_form.py @@ -1,3 +1,4 @@ +import pytest from app.main.forms import get_placeholder_form_instance from wtforms import Label @@ -16,3 +17,46 @@ def test_form_class_not_mutated(app_): assert str(form1.placeholder_value.label) == '' assert str(form2.placeholder_value.label) == '' + + +@pytest.mark.parametrize('service_can_send_international_sms, placeholder_name, value, expected_error', [ + + (False, 'email address', '', 'Can’t be empty'), + (False, 'email address', '12345', 'Enter a valid email address'), + (False, 'email address', 'test@example.com', None), + (False, 'email address', 'test@example.gov.uk', None), + + (False, 'phone number', '', 'Can’t be empty'), + (False, 'phone number', '+1-2345-678890', 'Not a UK mobile number'), + (False, 'phone number', '07900900123', None), + (False, 'phone number', '+44(0)7900 900-123', None), + + (True, 'phone number', '+123', 'Not enough digits'), + (True, 'phone number', '+44(0)7900 900-123', None), + (True, 'phone number', '+1-2345-678890', None), + + (False, 'anything else', '', 'Can’t be empty'), + +]) +def test_validates_recipients( + app_, + placeholder_name, + value, + service_can_send_international_sms, + expected_error, +): + with app_.test_request_context( + method='POST', + data={'placeholder_value': value} + ): + form = get_placeholder_form_instance( + placeholder_name, + {}, + allow_international_phone_numbers=service_can_send_international_sms, + ) + + if expected_error: + assert not form.validate_on_submit() + assert form.placeholder_value.errors[0] == expected_error + else: + assert form.validate_on_submit() diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 4818a1ee8..e9bf96db2 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -90,6 +90,9 @@ def _gen_mock_field(x): 'test@hmcts.net', 'test@scotent.co.uk', 'test@assembly.wales', + 'test@cjsm.net', + 'test@cqc.org.uk', + 'test@digital.cqc.org.uk', ]) def test_valid_list_of_white_list_email_domains( client, diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 731e5df03..587a1fe46 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -5,6 +5,7 @@ from unittest.mock import call, ANY from flask import url_for import pytest from bs4 import BeautifulSoup +from datetime import datetime, timedelta from freezegun import freeze_time from app.main.views.dashboard import ( @@ -18,7 +19,11 @@ from app.main.views.dashboard import ( ) from tests import validate_route_permission -from tests.conftest import SERVICE_ONE_ID +from tests.conftest import ( + SERVICE_ONE_ID, + mock_get_inbound_sms_summary, + mock_get_inbound_sms_summary_with_no_messages, +) from tests.app.test_utils import normalize_spaces stub_template_stats = [ @@ -44,6 +49,7 @@ def test_get_started( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', @@ -63,6 +69,7 @@ def test_get_started_is_hidden_once_templates_exist( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', @@ -74,6 +81,122 @@ def test_get_started_is_hidden_once_templates_exist( assert 'Get started' not in response.get_data(as_text=True) +def test_inbound_messages_not_visible_to_service_without_permissions( + logged_in_client, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_detailed_service, + mock_get_template_statistics, + mock_get_usage, + mock_get_inbound_sms_summary, + mock_get_yearly_sms_unit_count_and_cost +): + + service_one['permissions'] = [] + + response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + assert not page.select('.big-number-meta-wrapper') + assert mock_get_inbound_sms_summary.called is False + + +@pytest.mark.parametrize('inbound_summary_mock, expected_text', [ + (mock_get_inbound_sms_summary_with_no_messages, '0 text messages received'), + (mock_get_inbound_sms_summary, '99 text messages received latest message just now'), +]) +def test_inbound_messages_shows_count_of_messages( + logged_in_client, + mocker, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_detailed_service, + mock_get_template_statistics, + mock_get_usage, + inbound_summary_mock, + expected_text, + mock_get_yearly_sms_unit_count_and_cost +): + + service_one['permissions'] = ['inbound_sms'] + inbound_summary_mock(mocker) + + response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + assert normalize_spaces(page.select('.big-number-meta-wrapper')[0].text) == expected_text + assert page.select('.big-number-meta-wrapper a')[0]['href'] == url_for( + 'main.inbox', service_id=SERVICE_ONE_ID + ) + + +@pytest.mark.parametrize('index, expected_row', enumerate([ + '07900900000 foo 1 hour ago', + '07900900001 foo 2 hours ago', + '07900900002 foo 3 hours ago', + '07900900003 foo 4 hours ago', + '07900900004 foo 5 hours ago', +])) +def test_inbox_showing_inbound_messages( + logged_in_client, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_detailed_service, + mock_get_template_statistics, + mock_get_usage, + mock_get_inbound_sms, + index, + expected_row, +): + + service_one['permissions'] = ['inbound_sms'] + + response = logged_in_client.get(url_for('main.inbox', service_id=SERVICE_ONE_ID)) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + rows = page.select('tbody tr') + assert len(rows) == 5 + assert normalize_spaces(rows[index].text) == expected_row + + +def test_empty_inbox( + logged_in_client, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_detailed_service, + mock_get_template_statistics, + mock_get_usage, + mock_get_inbound_sms_with_no_messages, +): + + service_one['permissions'] = ['inbound_sms'] + + response = logged_in_client.get(url_for('main.inbox', service_id=SERVICE_ONE_ID)) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + assert normalize_spaces(page.select('tbody tr')) == ( + 'When users text your service’s phone number (GOVUK) you’ll see the messages here' + ) + + +def test_inbox_not_accessible_to_service_without_permissions( + logged_in_client, + service_one, +): + service_one['permissions'] = [] + response = logged_in_client.get(url_for('main.inbox', service_id=SERVICE_ONE_ID)) + + assert response.status_code == 403 + + def test_should_show_recent_templates_on_dashboard( logged_in_client, mocker, @@ -81,6 +204,7 @@ def test_should_show_recent_templates_on_dashboard( mock_get_jobs, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', @@ -147,6 +271,7 @@ def test_should_show_upcoming_jobs_on_dashboard( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -178,6 +303,7 @@ def test_should_show_recent_jobs_on_dashboard( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -297,6 +423,7 @@ def test_menu_send_messages( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): @@ -328,6 +455,7 @@ def test_menu_manage_service( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): @@ -358,6 +486,7 @@ def test_menu_manage_api_keys( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): @@ -388,6 +517,7 @@ 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_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): @@ -418,6 +548,7 @@ def test_route_for_service_permissions( mock_get_template_statistics, mock_get_detailed_service, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): with app_.test_request_context(): @@ -455,6 +586,7 @@ def test_service_dashboard_updates_gets_dashboard_totals( mock_get_detailed_service, mock_get_jobs, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={ @@ -677,11 +809,13 @@ def test_should_show_all_jobs_with_valid_statuses( logged_in_client, mock_get_template_statistics, mock_get_detailed_service, + mock_get_service_templates_when_no_templates_exist, mock_get_jobs, mock_get_usage, + mock_get_inbound_sms_summary, mock_get_yearly_sms_unit_count_and_cost ): - get_dashboard_partials(service_id=SERVICE_ONE_ID) + logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) first_call = mock_get_jobs.call_args_list[0] # first call - scheduled jobs only diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 6b4bfa0f2..97e1ae0d7 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -17,9 +17,18 @@ def no_redirect(): return lambda _external=True: None -def test_get_support_index_page(client): - resp = client.get(url_for('main.support')) - assert resp.status_code == 200 +@pytest.mark.parametrize('endpoint', [ + 'main.old_feedback', + 'main.support', +]) +def test_get_support_index_page( + client, + endpoint, +): + response = client.get(url_for('main.support'), follow_redirects=True) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Support' @freeze_time('2016-12-12 12:00:00.000000') diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 0980bfcaf..0625f37e7 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -8,6 +8,7 @@ from bs4 import BeautifulSoup from app.main.views.jobs import get_time_left, get_status_filters from tests import notification_json +from tests.conftest import SERVICE_ONE_ID from freezegun import freeze_time @@ -311,6 +312,13 @@ def test_should_show_updates_for_one_job_as_json( (None, 1) ] ) +@pytest.mark.parametrize( + "to_argument, expected_to_argument", [ + ('', ''), + ('+447900900123', '+447900900123'), + ('test@example.com', 'test@example.com'), + ] +) def test_can_show_notifications( logged_in_client, service_one, @@ -322,13 +330,17 @@ def test_can_show_notifications( expected_api_call, page_argument, expected_page_argument, + to_argument, + expected_to_argument, ): response = logged_in_client.get(url_for( 'main.view_notifications', service_id=service_one['id'], message_type=message_type, status=status_argument, - page=page_argument)) + page=page_argument, + to=to_argument, + )) assert response.status_code == 200 content = response.get_data(as_text=True) notifications = notification_json(service_one['id']) @@ -348,13 +360,16 @@ def test_can_show_notifications( assert query_dict['status'] == [status_argument] if expected_page_argument: assert query_dict['page'] == [str(expected_page_argument)] + if to_argument: + assert query_dict['to'] == [to_argument] mock_get_notifications.assert_called_with( limit_days=7, page=expected_page_argument, service_id=service_one['id'], status=expected_api_call, - template_type=[message_type] + template_type=[message_type], + to=expected_to_argument, ) json_response = logged_in_client.get(url_for( @@ -367,6 +382,62 @@ def test_can_show_notifications( assert json_content.keys() == {'counts', 'notifications'} +@pytest.mark.parametrize("initial_query_arguments, expected_status_field_value, expected_search_box_contents", [ + ( + { + 'message_type': 'sms', + }, + 'sending,delivered,failed', + '', + ), + ( + { + 'message_type': 'sms', + 'to': '+33(0)5-12-34-56-78', + }, + 'sending,delivered,failed', + '+33(0)5-12-34-56-78', + ), + ( + { + 'status': 'failed', + 'message_type': 'email', + 'page': '99', + 'to': 'test@example.com', + }, + 'failed', + 'test@example.com', + ), +]) +def test_search_recipient_form( + logged_in_client, + mock_get_notifications, + mock_get_detailed_service, + initial_query_arguments, + expected_status_field_value, + expected_search_box_contents, +): + response = logged_in_client.get(url_for( + 'main.view_notifications', + service_id=SERVICE_ONE_ID, + **initial_query_arguments + )) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + action_url = page.find("form")['action'] + url = urlparse(action_url) + assert url.path == '/services/{}/notifications/{}'.format( + SERVICE_ONE_ID, + initial_query_arguments['message_type'] + ) + query_dict = parse_qs(url.query) + assert query_dict == {} + + assert page.find("input", {'name': 'status'})['value'] == expected_status_field_value + assert page.find("input", {'name': 'to'})['value'] == expected_search_box_contents + + def test_should_show_notifications_for_a_service_with_next_previous( logged_in_client, service_one, diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index a90d690f1..69ebddf74 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -23,6 +23,7 @@ from tests.conftest import ( mock_get_international_service, mock_get_service_template, mock_get_service_email_template, + SERVICE_ONE_ID, ) template_types = ['email', 'sms'] @@ -204,32 +205,6 @@ def test_upload_csv_invalid_extension( assert "invalid.txt isn’t a spreadsheet that Notify can read" in resp.get_data(as_text=True) -def test_upload_valid_csv_shows_page_title( - logged_in_client, - mocker, - mock_get_service_template_with_placeholders, - mock_s3_upload, - mock_get_users_by_service, - mock_get_detailed_service_for_today, - service_one, - fake_uuid, -): - - mocker.patch('app.main.views.send.s3download', return_value=""" - phone number,name\n07700900986,Jo - """) - - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, - follow_redirects=True, - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.text.strip() == 'Preview of Two week reminder' - - def test_upload_valid_csv_shows_file_contents( logged_in_client, mocker, @@ -237,7 +212,6 @@ def test_upload_valid_csv_shows_file_contents( mock_s3_upload, mock_get_users_by_service, mock_get_detailed_service_for_today, - service_one, fake_uuid, ): @@ -247,15 +221,16 @@ def test_upload_valid_csv_shows_file_contents( """) response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), + url_for('main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid), data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, follow_redirects=True, ) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.text.strip() == 'Preview of Two week reminder' for index, cell in enumerate([ - ' 2 ', + ' 2 ', '
    07700900986
    ', '
    Jo
    ', ( @@ -311,7 +286,6 @@ def test_send_test_sms_message( mock_get_detailed_service_for_today, ): - expected_data = {'data': 'phone number\r\n07700 900762\r\n', 'file_name': 'Test message'} mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234') response = logged_in_client.get( @@ -319,29 +293,174 @@ def test_send_test_sms_message( follow_redirects=True ) assert response.status_code == 200 - mock_s3_upload.assert_called_with(service_one['id'], expected_data, 'eu-west-1') + mock_s3_upload.assert_called_with( + service_one['id'], + {'data': 'phone number\r\n07700 900762\r\n', 'file_name': 'Report'}, + 'eu-west-1' + ) +@pytest.mark.parametrize('endpoint, template_mock, expected_session_contents', [ + ('main.send_test_step', mock_get_service_template_with_placeholders, {'phone number': '07700 900762'}), + ('main.send_test_step', mock_get_service_email_template, {'email address': 'test@user.gov.uk'}), + ('main.send_test_step', mock_get_service_letter_template, {}), + ('main.send_one_off_step', mock_get_service_template, {}), + ('main.send_one_off_step', mock_get_service_email_template, {}), + ('main.send_one_off_step', mock_get_service_letter_template, {}), +]) def test_send_test_step_redirects_if_session_not_setup( + mocker, logged_in_client, - service_one, + mock_get_detailed_service_for_today, + mock_get_users_by_service, fake_uuid, - mock_get_service_email_template, + endpoint, + template_mock, + expected_session_contents, ): + template_mock(mocker) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + with logged_in_client.session_transaction() as session: assert 'send_test_values' not in session response = logged_in_client.get( - url_for('main.send_test_step', service_id=service_one['id'], template_id=fake_uuid, step_index=0), + url_for(endpoint, service_id=SERVICE_ONE_ID, template_id=fake_uuid, step_index=0), follow_redirects=True ) assert response.status_code == 200 with logged_in_client.session_transaction() as session: - assert session['send_test_values'] == {'email address': 'test@user.gov.uk'} + assert session['send_test_values'] == expected_session_contents +@pytest.mark.parametrize('template_mock, partial_url, expected_h1, tour_shown', [ + ( + mock_get_service_template_with_placeholders, + partial(url_for, 'main.send_test'), + 'Send to one recipient', + False, + ), + ( + mock_get_service_template_with_placeholders, + partial(url_for, 'main.send_one_off'), + 'Send to one recipient', + False, + ), + ( + mock_get_service_template_with_placeholders, + partial(url_for, 'main.send_test', help=1), + 'Example text message', + True, + ), + ( + mock_get_service_email_template, + partial(url_for, 'main.send_test', help=1), + 'Example text message', + True, + ), + ( + mock_get_service_email_template, + partial(url_for, 'main.send_test'), + 'Send to one recipient', + False, + ), + ( + mock_get_service_email_template, + partial(url_for, 'main.send_one_off'), + 'Send to one recipient', + False, + ), + ( + mock_get_service_letter_template, + partial(url_for, 'main.send_test'), + 'Print a test letter', + False, + ), + ( + mock_get_service_letter_template, + partial(url_for, 'main.send_one_off'), + 'Print a test letter', + False, + ), +]) +def test_send_one_off_or_test_has_correct_page_titles( + logged_in_client, + service_one, + fake_uuid, + mocker, + template_mock, + partial_url, + expected_h1, + tour_shown, +): + + template_mock(mocker) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + + response = logged_in_client.get( + partial_url(service_id=service_one['id'], template_id=fake_uuid, step_index=0), + follow_redirects=True, + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + assert page.h1.text.strip() == expected_h1 + + assert (len(page.select('.banner-tour')) == 1) == tour_shown + + +@pytest.mark.parametrize('template_mock, expected_link_text, expected_link_url', [ + (mock_get_service_template, 'Use my phone number', partial(url_for, 'main.send_test')), + (mock_get_service_email_template, 'Use my email address', partial(url_for, 'main.send_test')), + (mock_get_service_letter_template, None, None), +]) +def test_send_one_off_has_skip_link( + logged_in_client, + service_one, + fake_uuid, + mock_get_service_email_template, + mocker, + template_mock, + expected_link_text, + expected_link_url, +): + + template_mock(mocker) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + + response = logged_in_client.get( + url_for('main.send_one_off_step', service_id=service_one['id'], template_id=fake_uuid, step_index=0), + follow_redirects=True + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + skip_links = page.select('a.top-gutter-4-3') + + assert response.status_code == 200 + + if expected_link_text and expected_link_url: + assert skip_links[0].text.strip() == expected_link_text + assert skip_links[0]['href'] == expected_link_url( + service_id=service_one['id'], + template_id=fake_uuid, + ) + else: + assert not skip_links + + +@pytest.mark.parametrize('endpoint, expected_redirect, send_test_values', [ + ( + 'main.send_test_step', + 'main.send_test', + {'name': 'foo'}, + ), + ( + 'main.send_one_off_step', + 'main.send_one_off', + {'name': 'foo', 'phone number': '07900900123'}, + ), +]) def test_send_test_redirects_to_end_if_step_out_of_bounds( logged_in_client, service_one, @@ -350,13 +469,16 @@ def test_send_test_redirects_to_end_if_step_out_of_bounds( mock_s3_upload, mock_get_users_by_service, mock_get_detailed_service_for_today, + endpoint, + send_test_values, + expected_redirect, ): with logged_in_client.session_transaction() as session: - session['send_test_values'] = {'name': 'foo'} + session['send_test_values'] = send_test_values response = logged_in_client.get(url_for( - 'main.send_test_step', + endpoint, service_id=service_one['id'], template_id=fake_uuid, step_index=999, @@ -376,6 +498,10 @@ def test_send_test_redirects_to_end_if_step_out_of_bounds( ) +@pytest.mark.parametrize('endpoint, expected_redirect', [ + ('main.send_test_step', 'main.send_test'), + ('main.send_one_off_step', 'main.send_one_off'), +]) def test_send_test_redirects_to_start_if_you_skip_steps( logged_in_platform_admin_client, service_one, @@ -385,6 +511,8 @@ def test_send_test_redirects_to_start_if_you_skip_steps( mock_get_users_by_service, mock_get_detailed_service_for_today, mocker, + endpoint, + expected_redirect, ): with logged_in_platform_admin_client.session_transaction() as session: @@ -392,20 +520,24 @@ def test_send_test_redirects_to_start_if_you_skip_steps( session['send_test_values'] = {'address_line_1': 'foo'} response = logged_in_platform_admin_client.get(url_for( - 'main.send_test_step', + endpoint, service_id=service_one['id'], template_id=fake_uuid, step_index=7, # letter template has 7 placeholders – we’re at the end )) assert response.status_code == 302 assert response.location == url_for( - 'main.send_test', + expected_redirect, service_id=service_one['id'], template_id=fake_uuid, _external=True, ) +@pytest.mark.parametrize('endpoint, expected_redirect', [ + ('main.send_test_step', 'main.send_test'), + ('main.send_one_off_step', 'main.send_one_off'), +]) def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholders_empty( logged_in_client, service_one, @@ -414,13 +546,15 @@ def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholde mock_s3_download, mock_get_users_by_service, mock_get_detailed_service_for_today, + endpoint, + expected_redirect, ): with logged_in_client.session_transaction() as session: session['send_test_values'] = {'name': 'foo'} response = logged_in_client.get(url_for( - 'main.send_test_step', + endpoint, service_id=service_one['id'], template_id=fake_uuid, step_index=999, @@ -428,24 +562,30 @@ def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholde assert response.status_code == 302 assert response.location == url_for( - 'main.send_test', + expected_redirect, service_id=service_one['id'], template_id=fake_uuid, _external=True, ) +@pytest.mark.parametrize('endpoint, expected_redirect', [ + ('main.send_test', 'main.send_test_step'), + ('main.send_one_off', 'main.send_one_off_step'), +]) def test_send_test_sms_message_redirects_with_help_argument( logged_in_client, service_one, fake_uuid, + endpoint, + expected_redirect, ): response = logged_in_client.get( - url_for('main.send_test', service_id=service_one['id'], template_id=fake_uuid, help=1) + url_for(endpoint, service_id=service_one['id'], template_id=fake_uuid, help=1) ) assert response.status_code == 302 assert response.location == url_for( - 'main.send_test_step', + expected_redirect, service_id=service_one['id'], template_id=fake_uuid, step_index=0, @@ -465,7 +605,6 @@ def test_send_test_email_message_without_placeholders( fake_uuid, ): - expected_data = {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Test message'} mocker.patch('app.main.views.send.s3download', return_value='email address\r\ntest@user.gov.uk') response = logged_in_client.get( @@ -473,7 +612,11 @@ def test_send_test_email_message_without_placeholders( follow_redirects=True ) assert response.status_code == 200 - mock_s3_upload.assert_called_with(service_one['id'], expected_data, 'eu-west-1') + mock_s3_upload.assert_called_with( + service_one['id'], + {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Report'}, + 'eu-west-1' + ) def test_send_test_sms_message_with_placeholders_shows_first_field( @@ -692,7 +835,7 @@ def test_send_test_sms_message_puts_submitted_data_in_session_and_file( service_one['id'], { 'data': 'name,phone number\r\nJo,07700 900762\r\n', - 'file_name': 'Test message' + 'file_name': 'Report' }, 'eu-west-1' ) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 6fc6ea56e..4d9ba1814 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -92,6 +92,22 @@ def test_if_cant_send_letters_then_cant_see_letter_contact_block( assert 'Letter contact block' not in response.get_data(as_text=True) +def test_if_can_receive_inbound_then_cant_change_sms_sender( + logged_in_client, + service_one, + mock_get_letter_organisations, +): + service_one['permissions'] = ['inbound_sms'] + service_one['sms_sender'] = 'SomeNumber' + response = logged_in_client.get(url_for( + 'main.service_settings', service_id=service_one['id'] + )) + assert 'Text message sender SomeNumber Change' not in response.get_data(as_text=True) + assert url_for('.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=False) not in response.get_data(as_text=True) + assert 'SomeNumber' in response.get_data(as_text=True) + + def test_letter_contact_block_shows_none_if_not_set( logged_in_client, service_one, @@ -615,6 +631,80 @@ def test_set_text_message_sender( ) +def test_set_text_message_sender_and_inbound_sms( + logged_in_client, + service_one, + mock_get_letter_organisations, + mocker, +): + service_one['permissions'] = [] + update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', + return_value=service_one) + + data = {"sms_sender": "elevenchars"} + response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=True), + data=data, + follow_redirects=True) + assert response.status_code == 200 + + update_service_mock.assert_called_with( + service_one['id'], + {'permissions': ['inbound_sms'], + 'sms_sender': "elevenchars"} + ) + assert app.current_service['permissions'] == ['inbound_sms'] + + +def test_turn_inbound_sms_off( + logged_in_client, + service_one, + mock_get_letter_organisations, + mocker, +): + service_one['permissions'] = ['inbound_sms'] + update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', + return_value=service_one) + + data = {"sms_sender": "elevenchars"} + response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=True), + data=data, + follow_redirects=True) + assert response.status_code == 200 + + update_service_mock.assert_called_with( + service_one['id'], + {'permissions': [], + 'sms_sender': "elevenchars"} + ) + assert app.current_service['permissions'] == [] + + +def test_set_text_message_sender_and_not_inbound_sms( + logged_in_client, + service_one, + mock_get_letter_organisations, + mocker, +): + service_one['permissions'] = [] + update_service_mock = mocker.patch('app.service_api_client.update_service', + return_value=service_one) + + data = {"sms_sender": "elevenchars"} + response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=False), + data=data, + follow_redirects=True) + assert response.status_code == 200 + + update_service_mock.assert_called_with( + service_one['id'], + sms_sender="elevenchars" + ) + assert app.current_service['permissions'] == [] + + @pytest.mark.parametrize('content, expected_error', [ ("", "Can’t be empty"), ("twelvecharss", "Enter 11 characters or fewer"), diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 9c7a21f04..c17f5fa4a 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -25,6 +25,7 @@ def test_sign_out_user( mock_get_detailed_service, mock_get_yearly_sms_unit_count_and_cost, mock_get_usage, + mock_get_inbound_sms_summary, ): with logged_in_client.session_transaction() as session: assert session.get('user_id') is not None diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index cf0bc3fdd..acafae751 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -44,11 +44,11 @@ def test_should_show_page_for_one_template( ), ( ['send_texts', 'send_emails', 'send_letters'], - ['.send_messages', '.send_test'] + ['.send_messages', '.send_one_off'] ), ( ['send_texts', 'send_emails', 'send_letters', 'manage_templates'], - ['.send_messages', '.send_test', '.edit_service_template'] + ['.send_messages', '.send_one_off', '.edit_service_template'] ), ]) def test_should_be_able_to_view_a_template_with_links( diff --git a/tests/app/test_template_previews.py b/tests/app/test_template_previews.py index 8c8200576..e1088bbdf 100644 --- a/tests/app/test_template_previews.py +++ b/tests/app/test_template_previews.py @@ -29,15 +29,15 @@ def test_from_utils_template_calls_through( @pytest.mark.parametrize('partial_call, expected_url', [ ( partial(TemplatePreview.from_database_object, filetype='bar'), - 'http://localhost:6013/preview.bar', + 'http://localhost:9999/preview.bar', ), ( partial(TemplatePreview.from_database_object, filetype='baz'), - 'http://localhost:6013/preview.baz', + 'http://localhost:9999/preview.baz', ), ( partial(TemplatePreview.from_database_object, filetype='bar', page=99), - 'http://localhost:6013/preview.bar?page=99', + 'http://localhost:9999/preview.bar?page=99', ), ]) def test_from_database_object_makes_request( diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 0c0da896e..9bfe8828b 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -156,5 +156,7 @@ def test_generate_notifications_csv_calls_twice_if_next_link(mocker): assert mock_get_notifications.mock_calls[1][2]['page'] == 2 -def normalize_spaces(string): - return ' '.join(string.split()) +def normalize_spaces(input): + if isinstance(input, str): + return ' '.join(input.split()) + return normalize_spaces(' '.join(item.text for item in input)) diff --git a/tests/conftest.py b/tests/conftest.py index 2aa63a34b..899d0d8c2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -169,7 +169,8 @@ def mock_update_service(mocker): 'restricted', 'email_from', 'reply_to_email_address', - 'sms_sender' + 'sms_sender', + 'permissions' ]} ) return {'data': service} @@ -1071,6 +1072,7 @@ def mock_get_notifications(mocker, api_user_active): rows=5, include_jobs=None, include_from_test_key=None, + to=None, ): job = None if job_id is not None: @@ -1103,7 +1105,9 @@ def mock_get_notifications_with_previous_next(mocker): status=None, limit_days=None, include_jobs=None, - include_from_test_key=None): + include_from_test_key=None, + to=None, + ): return notification_json(service_id, with_links=True) return mocker.patch( @@ -1130,6 +1134,68 @@ def mock_get_notifications_with_no_notifications(mocker): ) +@pytest.fixture(scope='function') +def mock_get_inbound_sms(mocker): + def _get_inbound_sms( + service_id, + ): + return [{ + 'user_number': '0790090000' + str(i), + 'content': 'foo', + 'created_at': (datetime.utcnow() - timedelta(minutes=60 * (i + 1))).isoformat() + } for i in range(5)] + + return mocker.patch( + 'app.service_api_client.get_inbound_sms', + side_effect=_get_inbound_sms, + ) + + +@pytest.fixture(scope='function') +def mock_get_inbound_sms_with_no_messages(mocker): + def _get_inbound_sms( + service_id, + ): + return [] + + return mocker.patch( + 'app.service_api_client.get_inbound_sms', + side_effect=_get_inbound_sms, + ) + + +@pytest.fixture(scope='function') +def mock_get_inbound_sms_summary(mocker): + def _get_inbound_sms_summary( + service_id, + ): + return { + 'count': 99, + 'most_recent': datetime.utcnow().isoformat() + } + + return mocker.patch( + 'app.service_api_client.get_inbound_sms_summary', + side_effect=_get_inbound_sms_summary, + ) + + +@pytest.fixture(scope='function') +def mock_get_inbound_sms_summary_with_no_messages(mocker): + def _get_inbound_sms_summary( + service_id, + ): + return { + 'count': 0, + 'latest_message': None + } + + return mocker.patch( + 'app.service_api_client.get_inbound_sms_summary', + side_effect=_get_inbound_sms_summary, + ) + + @pytest.fixture(scope='function') def mock_has_permissions(mocker): def _has_permission(permissions=None, any_=False, admin_override=False):