diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index f9ce3643b..054d6b797 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -48,6 +48,12 @@ $tail-angle: 20deg; } +.sms-message-sender { + @include copy-19; + color: $secondary-text-colour; + margin: 0 0 -10px 0; +} + .sms-message-recipient { @include copy-19; color: $secondary-text-colour; diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 8f343a68f..a027fe4ff 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,3 +1,5 @@ +import calendar +from collections import defaultdict from datetime import datetime from functools import partial from flask import ( @@ -115,31 +117,18 @@ def template_usage(service_id): stats = sorted(stats, key=lambda x: (x['count']), reverse=True) - stats_by_month = list() - - for month in get_months_for_financial_year(year, time_format='%Y-%m'): - long_month = yyyy_mm_to_datetime(month).strftime('%B') - - template_used = list() - - for stat in stats: - if long_month == datetime(1900, stat['month'], 1).strftime("%B"): - template_used.append( - { - 'name': stat['name'], - 'type': stat['type'], - 'requested_count': stat['count'] - } - ) - - stats_by_month.append( - { - 'name': long_month, - 'templates_used': template_used - } - ) - - months = stats_by_month + templates_by_month = defaultdict(list) + for stat in stats: + templates_by_month[int(stat['month'])].append({ + 'name': stat['name'], + 'type': stat['type'], + 'requested_count': stat['count'] + }) + months = [{ + 'name': calendar.month_name[k], + 'templates_used': v + } for k, v in templates_by_month.items() + ] return render_template( 'views/dashboard/all-template-statistics.html', diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index ad5817fa1..effcea000 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -109,8 +109,7 @@ def service_settings(service_id): default_sms_sender=default_sms_sender, sms_sender_count=sms_sender_count, free_sms_fragment_limit=free_sms_fragment_limit, - prefix_sms_with_service_name=current_service['prefix_sms_with_service_name'], - + prefix_sms=current_service['prefix_sms'], ) @@ -477,7 +476,7 @@ def service_set_sms(service_id): def service_set_sms_prefix(service_id): form = SMSPrefixForm(enabled=( - 'on' if current_service['prefix_sms_with_service_name'] else 'off' + 'on' if current_service['prefix_sms'] else 'off' )) form.enabled.label.text = 'Start all text messages with ‘{}:’'.format(current_service['name']) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index cebf16b43..07439ba83 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -94,7 +94,7 @@ {% call row() %} {{ text_field('Text messages start with service name') }} - {{ boolean_field(prefix_sms_with_service_name) }} + {{ boolean_field(prefix_sms) }} {{ edit_field('Change', url_for('.service_set_sms_prefix', service_id=current_service.id)) }} {% endcall %} diff --git a/app/utils.py b/app/utils.py index 3a69f4486..ba19eddc7 100644 --- a/app/utils.py +++ b/app/utils.py @@ -272,7 +272,7 @@ def get_template( page_count=1, redact_missing_personalisation=False, email_reply_to=None, - sms_sender=None + sms_sender=None, ): if 'email' == template['template_type']: return EmailPreviewTemplate( @@ -288,7 +288,9 @@ def get_template( return SMSPreviewTemplate( template, prefix=service['name'], - sender=not service['prefix_sms_with_service_name'], + show_prefix=service['prefix_sms'], + sender=sms_sender, + show_sender=bool(sms_sender), show_recipient=show_recipient, redact_missing_personalisation=redact_missing_personalisation, ) diff --git a/tests/__init__.py b/tests/__init__.py index c8f7ec629..372d34f6c 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -56,7 +56,7 @@ def service_json( permissions=None, organisation_type='central', free_sms_fragment_limit=250000, - prefix_sms_with_service_name='Treat as None', + prefix_sms=True, ): if users is None: users = [] @@ -64,8 +64,6 @@ def service_json( permissions = ['email', 'sms'] if inbound_api is None: inbound_api = [] - if prefix_sms_with_service_name == 'Treat as None': - prefix_sms_with_service_name = (sms_sender == 'GOVUK') return { 'id': id_, 'name': name, @@ -86,7 +84,7 @@ def service_json( 'dvla_organisation': '001', 'permissions': permissions, 'inbound_api': inbound_api, - 'prefix_sms_with_service_name': prefix_sms_with_service_name, + 'prefix_sms': prefix_sms, } diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index fe912894e..7b2dcc370 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2377,3 +2377,54 @@ def test_reply_to_is_previewed_if_chosen( assert 'test@example.com' in email_meta else: assert 'test@example.com' not in email_meta + + +@pytest.mark.parametrize('endpoint, extra_args', [ + ('main.check_messages', {'template_type': 'sms', 'upload_id': fake_uuid()}), + ('main.send_one_off_step', {'template_id': fake_uuid(), 'step_index': 0}), +]) +@pytest.mark.parametrize('sms_sender', [ + None, + fake_uuid(), +]) +def test_sms_sender_is_previewed( + client_request, + mocker, + mock_get_service_template, + mock_s3_download, + mock_get_users_by_service, + mock_get_detailed_service_for_today, + get_default_sms_sender, + endpoint, + extra_args, + sms_sender, +): + + mocker.patch('app.main.views.send.s3download', return_value=""" + phone number,date,thing + 7700900986,foo,bar + """) + + with client_request.session_transaction() as session: + session['recipient'] = '7700900986' + session['placeholders'] = {} + session['upload_data'] = { + 'original_file_name': 'example.csv', + 'template_id': fake_uuid(), + 'notification_count': 1, + 'valid': True + } + session['sender_id'] = sms_sender + + page = client_request.get( + endpoint, + service_id=SERVICE_ONE_ID, + **extra_args + ) + + sms_sender_on_page = page.select_one('.sms-message-sender') + + if sms_sender: + assert sms_sender_on_page.text.strip() == 'From: GOVUK' + else: + assert not sms_sender_on_page