From be79cf9669dcbac53e397a5622e5d1860bfa7a21 Mon Sep 17 00:00:00 2001 From: Will Pearson Date: Tue, 16 Jun 2026 12:56:21 +0100 Subject: [PATCH 1/2] Adding reason to provider changes This adds a single field to the provider ratio form for a reason for the change. This is propagated to the call to the notifications-api. Tests have been updated --- app/main/forms.py | 15 +++++++++--- app/main/views/providers.py | 3 ++- app/notify_client/provider_client.py | 4 ++-- app/templates/views/providers/provider.html | 10 +++++++- app/templates/views/providers/providers.html | 4 +++- tests/app/main/views/test_providers.py | 25 +++++++++++--------- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index b39543745c..34ff1bbd37 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1945,11 +1945,13 @@ def validate(self, *args, **kwargs): class AdminProviderRatioForm(OrderableFieldsForm): - def __init__(self, providers): + def __init__(self, providers, *args, **kwargs): self._providers = providers + super().__init__(*args, **kwargs) + # hack: https://github.com/wtforms/wtforms/issues/736 - self._unbound_fields = [ + fields = [ ( provider["identifier"], GovukIntegerField( @@ -1963,8 +1965,15 @@ def __init__(self, providers): ) for provider in providers ] + fields += [ + ( + "reason", + GovukTextInputField("Reason"), + ) + ] - super().__init__(data={provider["identifier"]: provider["priority"] for provider in providers}) + self._unbound_fields = fields + super().__init__(*args, data={provider["identifier"]: provider["priority"] for provider in providers}, **kwargs) def validate(self, *args, **kwargs): if not super().validate(*args, **kwargs): diff --git a/app/main/views/providers.py b/app/main/views/providers.py index c0ad79f6e1..76945199b3 100644 --- a/app/main/views/providers.py +++ b/app/main/views/providers.py @@ -54,9 +54,10 @@ def edit_sms_provider_ratio(): form = AdminProviderRatioForm(providers) if form.validate_on_submit(): + reason = form.reason for provider in providers: field = getattr(form, provider["identifier"]) - provider_client.update_provider(provider["id"], field.data) + provider_client.update_provider(provider["id"], field.data, reason=reason.data) return redirect(url_for(".view_providers")) return render_template( diff --git a/app/notify_client/provider_client.py b/app/notify_client/provider_client.py index 63f363874f..3a1167bad5 100644 --- a/app/notify_client/provider_client.py +++ b/app/notify_client/provider_client.py @@ -18,8 +18,8 @@ def get_provider_by_id(self, provider_id): def get_provider_versions(self, provider_id): return self.get(url=f"/provider-details/{provider_id}/versions") - def update_provider(self, provider_id, priority): - data = {"priority": priority} + def update_provider(self, provider_id, priority, reason=None): + data = {"priority": priority, "reason": reason} data = _attach_current_user(data) return self.post(url=f"/provider-details/{provider_id}", data=data) diff --git a/app/templates/views/providers/provider.html b/app/templates/views/providers/provider.html index 55fba94d3e..473d7e2e4b 100644 --- a/app/templates/views/providers/provider.html +++ b/app/templates/views/providers/provider.html @@ -27,7 +27,7 @@ caption='', caption_visible=False, empty_message='No history for this provider', - field_headings=['Version', 'Last Updated', 'Updated By', 'Priority', 'Active'], + field_headings=['Version', 'Last Updated', 'Updated By', 'Priority', 'Active', 'Reason'], field_headings_visible=True, give_rows_ids=False, ) %} @@ -46,10 +46,18 @@ {{ text_field('None') }} {% endif %} + {{ text_field(item.priority) }} {{ text_field(item.active) }} + {% if item.reason %} + {{ text_field(item.reason) }} + {% else %} + {{ text_field('None') }} + {% endif %} + + {% endcall %} diff --git a/app/templates/views/providers/providers.html b/app/templates/views/providers/providers.html index 43e96ee897..4f66f9ca11 100644 --- a/app/templates/views/providers/providers.html +++ b/app/templates/views/providers/providers.html @@ -18,7 +18,7 @@

SMS

caption="Domestic SMS providers", caption_visible=False, empty_message='No domestic sms providers', - field_headings=['Provider', 'Priority', '% monthly traffic', 'Active', 'Last Updated', 'Updated By'], + field_headings=['Provider', 'Priority', '% monthly traffic', 'Active', 'Last Updated', 'Updated By', 'Reason'], field_headings_visible=True, give_rows_ids=False, ) %} @@ -38,6 +38,8 @@

SMS

{{ optional_text_field(item.created_by_name, default='None') }} + {{ optional_text_field(item.reason, default='None') }} + {% endcall %}

Email

diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index d49533c8ca..9466dc2198 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -16,6 +16,7 @@ def provider_json(overrides): "identifier": "override-me", "notification_type": "sms", "updated_at": None, + "reason": None, "version": 1, "created_by_name": None, "supports_international": False, @@ -38,6 +39,7 @@ def sms_provider_1(): "updated_at": datetime(2017, 1, 16, 15, 20, 40).isoformat(), "created_by_name": "Test User", "current_month_billable_sms": 5020, + "reason": "reason", } ) @@ -52,6 +54,7 @@ def sms_provider_2(): "identifier": "sms_provider_2", "notification_type": "sms", "current_month_billable_sms": 6891, + "reason": "reason", } ) @@ -309,7 +312,7 @@ def test_edit_sms_provider_provider_ratio(client_request, platform_admin_user, m ) inputs = page.select('.govuk-input[type="text"]') - assert len(inputs) == 2 + assert len(inputs) == 3 first_input = page.select_one('.govuk-input[name="sms_provider_1"]') assert first_input.attrs["value"] == str(sms_provider_1["priority"]) @@ -335,24 +338,24 @@ def test_edit_sms_provider_provider_ratio_only_shows_active_providers( ) inputs = page.select('.govuk-input[type="text"]') - assert len(inputs) == 1 + assert len(inputs) == 2 @pytest.mark.parametrize( "post_data, expected_calls", [ ( - {"sms_provider_1": 10, "sms_provider_2": 90}, + {"sms_provider_1": 10, "sms_provider_2": 90, "reason": "reason"}, [ - call("sms_provider_1-id", 10), - call("sms_provider_2-id", 90), + call("sms_provider_1-id", 10, reason="reason"), + call("sms_provider_2-id", 90, reason="reason"), ], ), ( - {"sms_provider_1": 80, "sms_provider_2": 20}, + {"sms_provider_1": 80, "sms_provider_2": 20, "reason": "reason"}, [ - call("sms_provider_1-id", 80), - call("sms_provider_2-id", 20), + call("sms_provider_1-id", 80, reason="reason"), + call("sms_provider_2-id", 20, reason="reason"), ], ), ], @@ -383,9 +386,9 @@ def test_edit_sms_provider_ratio_submit( @pytest.mark.parametrize( "post_data, expected_error", [ - ({"sms_provider_1": 90, "sms_provider_2": 20}, "The total must add up to 100%"), - ({"sms_provider_1": 101, "sms_provider_2": 20}, "Must be between 0 and 100"), - ({"sms_provider_1": 99.9, "sms_provider_2": 0.1}, "Enter a percentage in digits"), + ({"sms_provider_1": 90, "sms_provider_2": 20, "reason": "Good reason"}, "The total must add up to 100%"), + ({"sms_provider_1": 101, "sms_provider_2": 20, "reason": "Good reason"}, "Must be between 0 and 100"), + ({"sms_provider_1": 99.9, "sms_provider_2": 0.1, "reason": "Good reason"}, "Enter a percentage in digits"), ], ) def test_edit_sms_provider_submit_invalid_percentages( From 12aab6a099ab9e9c269446e3b0a171ad9db14be4 Mon Sep 17 00:00:00 2001 From: Will Pearson Date: Mon, 13 Jul 2026 16:15:29 +0100 Subject: [PATCH 2/2] Update app/templates/views/providers/provider.html Co-authored-by: Chris Hill-Scott --- app/templates/views/providers/provider.html | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/templates/views/providers/provider.html b/app/templates/views/providers/provider.html index 473d7e2e4b..c7eddefcd0 100644 --- a/app/templates/views/providers/provider.html +++ b/app/templates/views/providers/provider.html @@ -1,5 +1,5 @@ {% extends "withoutnav_template.html" %} -{% from "components/table.html" import list_table, text_field %} +{% from "components/table.html" import list_table, text_field, optional_text_field %} {% from "components/page-header.html" import page_header %} {% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} @@ -51,11 +51,7 @@ {{ text_field(item.active) }} - {% if item.reason %} - {{ text_field(item.reason) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {{ optional_text_field(item.reason, default='Not given') }} {% endcall %}