From 75fd2d4ffcc30225f787c57cc0e93e216958d3ab Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Mon, 18 Feb 2019 16:27:53 +0000 Subject: [PATCH] Add a new boolean radios fields and change forms to use it This adds a new OnOffField class that implements a boolean field that is rendered as two On / Off radio buttons. This allows us to avoid comparing 'on' and 'off' string values in the views since the field takes care of transforming form data into python booleans. This also adds a form class that can be used for any single On / Off switch forms (e.g. service permissions). --- app/main/forms.py | 34 +++++++++++++------ app/main/views/service_settings.py | 4 +-- tests/app/main/views/test_service_settings.py | 10 +++--- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index ddaf23e4e..e3585a15c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -696,23 +696,37 @@ class ServiceLetterContactBlockForm(StripWhitespaceForm): ) -class ServiceSwitchChannelForm(StripWhitespaceForm): +class OnOffField(RadioField): + def __init__(self, label, *args, **kwargs): + super().__init__(label, choices=[ + (True, 'On'), + (False, 'Off'), + ], *args, **kwargs) - def __init__(self, channel, *args, **kwargs): + def process_formdata(self, valuelist): + if valuelist: + value = valuelist[0] + self.data = (value == 'True') if value in ['True', 'False'] else value + + +class ServiceOnOffSettingForm(StripWhitespaceForm): + + def __init__(self, name, *args, **kwargs): super().__init__(*args, **kwargs) - self.enabled.label.text = 'Send {}'.format({ + self.enabled.label.text = name + + enabled = OnOffField('Choices') + + +class ServiceSwitchChannelForm(ServiceOnOffSettingForm): + def __init__(self, channel, *args, **kwargs): + name = 'Send {}'.format({ 'email': 'emails', 'sms': 'text messages', 'letter': 'letters', }.get(channel)) - enabled = RadioField( - 'Choices', - choices=[ - ('on', 'On'), - ('off', 'Off'), - ], - ) + super().__init__(name, *args, **kwargs) class FieldWithNoneOption(): diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index a09058bcf..0774c1e5d 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -518,13 +518,13 @@ def service_set_channel(service_id, channel): form = ServiceSwitchChannelForm( channel=channel, - enabled='on' if current_service.has_permission(channel) else 'off' + enabled=current_service.has_permission(channel) ) if form.validate_on_submit(): current_service.force_permission( channel, - on=(form.enabled.data == 'on'), + on=form.enabled.data, ) return redirect( url_for(".service_settings", service_id=service_id) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 552c9e75b..690db6e37 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2518,7 +2518,7 @@ def test_unknown_channel_404s( 'It costs between 30p and 76p to send a letter using Notify.', 'Send letters', ['email', 'sms'], - 'off', 'on', + 'False', 'True', ['email', 'sms', 'letter'], ), ( @@ -2526,7 +2526,7 @@ def test_unknown_channel_404s( 'It costs between 30p and 76p to send a letter using Notify.', 'Send letters', ['email', 'sms', 'letter'], - 'on', 'off', + 'True', 'False', ['email', 'sms'], ), ( @@ -2534,7 +2534,7 @@ def test_unknown_channel_404s( 'You have a free allowance of 250,000 text messages each financial year.', 'Send text messages', [], - 'off', 'on', + 'False', 'True', ['sms'], ), ( @@ -2542,7 +2542,7 @@ def test_unknown_channel_404s( 'It’s free to send emails through GOV.UK Notify.', 'Send emails', [], - 'off', 'on', + 'False', 'True', ['email'], ), ( @@ -2550,7 +2550,7 @@ def test_unknown_channel_404s( 'It’s free to send emails through GOV.UK Notify.', 'Send emails', ['email', 'sms', 'letter'], - 'on', 'on', + 'True', 'True', ['email', 'sms', 'letter'], ), ])