From 7f333ba5fe0e570284d47c55aa0b7b0c082558d1 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 6 Apr 2022 15:08:28 +0100 Subject: [PATCH] Rewrite SMS ratio form to cope with 3 providers This replaces the slider with an integer input for each provider. Unfortunately showing a variable number of inputs isn't easy to achieve in WTForms [^1], but we think this is the least worst way to do it vs e.g. not using WTForms at all. [^1]: https://github.com/wtforms/wtforms/issues/736 --- app/assets/stylesheets/views/history.scss | 63 ------------------- app/main/forms.py | 37 +++++------ app/main/views/providers.py | 12 ++-- .../providers/edit-sms-provider-ratio.html | 46 ++++---------- tests/app/main/views/test_providers.py | 20 +++--- 5 files changed, 46 insertions(+), 132 deletions(-) diff --git a/app/assets/stylesheets/views/history.scss b/app/assets/stylesheets/views/history.scss index a8d16514e..0a0ed11d1 100644 --- a/app/assets/stylesheets/views/history.scss +++ b/app/assets/stylesheets/views/history.scss @@ -29,67 +29,4 @@ $item-top-padding: govuk-spacing(3); display: block; color: $secondary-text-colour; } - - &-percentage { - - $axis-thickness: 2px; - border-bottom: $axis-thickness solid $black; - position: relative; - margin-bottom: 35px; - height: 35px; - - &:before, - &:after { - content: ""; - position: absolute; - bottom: -6px; - left: 0; - height: 10px; - width: $axis-thickness; - background: $black; - } - - &:after { - left: auto; - right: 0; - } - - &-without-border { - position: relative; - padding-top: 35px; - } - - &-marker { - $size: 20px; - $border-thickness: 7px; - $text-width: 100px; - display: block; - width: $size; - height: $size; - position: absolute; - top: 35px - ($size / 2) - ($border-thickness - ($axis-thickness / 2)); - margin-left: 0 - ($size / 2) - $border-thickness; - background: $black; - border-radius: $size; - border: $border-thickness solid $white; - } - - &-left-label, - &-right-label { - width: 100%; - position: absolute; - top: 0; - } - - &-left-label { - left: 0; - } - - &-right-label { - right: 0; - text-align: right; - } - - } - } diff --git a/app/main/forms.py b/app/main/forms.py index 6b1af4675..403e3317c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1740,29 +1740,26 @@ class EstimateUsageForm(StripWhitespaceForm): return super().validate(*args, **kwargs) -class AdminProviderRatioForm(StripWhitespaceForm): +class AdminProviderRatioForm(Form): + def __init__(self, providers): + # hack: https://github.com/wtforms/wtforms/issues/736 + self._unbound_fields = [ + ( + provider['identifier'], + GovukIntegerField( + provider['display_name'], + validators=[validators.NumberRange( + min=0, max=100, message="Must be between 0 and 100" + )] + ) + ) for provider in providers + ] - ratio = GovukRadiosField(choices=[ - (str(value), '{}% / {}%'.format(value, 100 - value)) - for value in range(100, -10, -10) - ], - param_extensions={ - "classes": "govuk-radios--inline", - "fieldset": { - "legend": { - "classes": "govuk-visually-hidden" - } - } + super().__init__(data={ + provider['identifier']: provider['priority'] + for provider in providers }) - @property - def percentage_left(self): - return int(self.ratio.data) - - @property - def percentage_right(self): - return 100 - self.percentage_left - class ServiceContactDetailsForm(StripWhitespaceForm): contact_details_type = RadioField( diff --git a/app/main/views/providers.py b/app/main/views/providers.py index 78bdcba89..80e8fbe0e 100644 --- a/app/main/views/providers.py +++ b/app/main/views/providers.py @@ -54,23 +54,21 @@ def edit_sms_provider_ratio(): and provider['active'] ], key=itemgetter('identifier')) - form = AdminProviderRatioForm(ratio=providers[0]['priority']) + form = AdminProviderRatioForm(providers) if len(providers) < 2: abort(400) - first_provider, second_provider = providers[0:2] - if form.validate_on_submit(): - provider_client.update_provider(first_provider['id'], form.percentage_left) - provider_client.update_provider(second_provider['id'], form.percentage_right) + for provider in providers: + field = getattr(form, provider['identifier']) + provider_client.update_provider(provider['id'], field.data) return redirect(url_for('.view_providers')) return render_template( 'views/providers/edit-sms-provider-ratio.html', form=form, - first_provider=providers[0]['display_name'], - second_provider=providers[1]['display_name'], + providers=providers ) diff --git a/app/templates/views/providers/edit-sms-provider-ratio.html b/app/templates/views/providers/edit-sms-provider-ratio.html index bbe3b0d04..53485cc26 100644 --- a/app/templates/views/providers/edit-sms-provider-ratio.html +++ b/app/templates/views/providers/edit-sms-provider-ratio.html @@ -4,44 +4,22 @@ {% from "components/form.html" import form_wrapper %} {% block per_page_title %} - Text message providers + Edit SMS provider priorities {% endblock %} {% block platform_admin_content %} - {{ page_header('Text message providers') }} + {{ page_header('Edit SMS provider priorities') }} +

+ If you change one priority you will need to change the others so they add up to 100%. +

-

- Now -

-
-
-
-
 
-
-
-
- {{ first_provider }} -
-
- {{ second_provider }} -
- {% call form_wrapper() %} -
- {{ form.ratio }} -
- — -
-
- — -
-
- {{ page_footer('Update', centered_button=True) }} - {% endcall %} -
-
-
-
-
+ {% call form_wrapper() %} + {% for field in form %} + {{ field }} + {% endfor %} + + {{ page_footer('Save') }} + {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index e1f10b5cd..1770493ff 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -288,27 +288,33 @@ def test_view_provider_shows_version_history( assert second_row[4].text.strip() == "True" -@pytest.mark.parametrize('posted_number, expected_calls', [ +@pytest.mark.parametrize('post_data, expected_calls', [ ( - '10', + { + sms_provider_1['identifier']: 10, + sms_provider_2['identifier']: 90 + }, [ call(sms_provider_1['id'], 10), call(sms_provider_2['id'], 90), ], ), ( - '80', + { + sms_provider_1['identifier']: 80, + sms_provider_2['identifier']: 20 + }, [ call(sms_provider_1['id'], 80), call(sms_provider_2['id'], 20), ], ), ]) -def test_edit_sms_provider_ratio_should_update_priority_of_first_two_sms_providers( +def test_edit_sms_provider_ratio_submit( client_request, platform_admin_user, mocker, - posted_number, + post_data, expected_calls, stub_providers, ): @@ -323,9 +329,7 @@ def test_edit_sms_provider_ratio_should_update_priority_of_first_two_sms_provide client_request.login(platform_admin_user) client_request.post( '.edit_sms_provider_ratio', - _data={ - 'ratio': posted_number, - }, + _data=post_data, _expected_redirect=url_for( '.view_providers', _external=True,