From c8cd03189a7ea38ebdd97d613a88a50d255005c2 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Sun, 12 Mar 2017 20:55:20 +0000 Subject: [PATCH] Use BooleanField for admin_state_up form admin_state_up is a boolean value but the network and port edit forms expected a string version of True/False. As a result, True/False string was shown as the default value in these forms. The field is implemented as ChoiceField, but there is no special reason not to use BooleanField for a boolean field and admin_state(_up) fields are the only exceptions in horizon. This commit replaces all admin_state(_up) fields to use BooleanField. As far as I checked, this pattern is used only in the networking related panels and this patch clean them up. Change-Id: I9286f6c29d67fec7a88b74661bc8eca945fd9061 Closes-Bug: #1672213 --- .../dashboards/admin/networks/forms.py | 16 ++++++-------- .../dashboards/admin/networks/ports/forms.py | 3 +-- .../dashboards/project/firewalls/forms.py | 7 ++---- .../dashboards/project/firewalls/workflows.py | 11 +++------- .../dashboards/project/networks/forms.py | 9 +++----- .../project/networks/ports/forms.py | 15 +++++-------- .../dashboards/project/networks/workflows.py | 12 +++++----- .../dashboards/project/routers/forms.py | 14 +++++------- .../dashboards/project/routers/tests.py | 10 ++++----- .../dashboards/project/vpn/forms.py | 14 ++++-------- .../templates/vpn/_add_vpn_service_help.html | 4 ++-- .../dashboards/project/vpn/workflows.py | 22 +++++++++---------- 12 files changed, 55 insertions(+), 82 deletions(-) diff --git a/openstack_dashboard/dashboards/admin/networks/forms.py b/openstack_dashboard/dashboards/admin/networks/forms.py index 3eec873898..2796d13086 100644 --- a/openstack_dashboard/dashboards/admin/networks/forms.py +++ b/openstack_dashboard/dashboards/admin/networks/forms.py @@ -118,10 +118,9 @@ class CreateNetwork(forms.SelfHandlingForm): 'class': 'switched', 'data-switch-on': 'network_type', })) - admin_state = forms.ThemableChoiceField( - choices=[('True', _('UP')), - ('False', _('DOWN'))], - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + initial=True, + required=False) shared = forms.BooleanField(label=_("Shared"), initial=False, required=False) external = forms.BooleanField(label=_("External Network"), @@ -250,7 +249,7 @@ class CreateNetwork(forms.SelfHandlingForm): try: params = {'name': data['name'], 'tenant_id': data['tenant_id'], - 'admin_state_up': (data['admin_state'] == 'True'), + 'admin_state_up': data['admin_state'], 'shared': data['shared'], 'router:external': data['external']} if api.neutron.is_extension_supported(request, 'provider'): @@ -312,9 +311,8 @@ class UpdateNetwork(forms.SelfHandlingForm): network_id = forms.CharField(label=_("ID"), widget=forms.TextInput( attrs={'readonly': 'readonly'})) - admin_state = forms.ThemableChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + required=False) shared = forms.BooleanField(label=_("Shared"), required=False) external = forms.BooleanField(label=_("External Network"), required=False) failure_url = 'horizon:admin:networks:index' @@ -322,7 +320,7 @@ class UpdateNetwork(forms.SelfHandlingForm): def handle(self, request, data): try: params = {'name': data['name'], - 'admin_state_up': (data['admin_state'] == 'True'), + 'admin_state_up': data['admin_state'], 'shared': data['shared'], 'router:external': data['external']} network = api.neutron.network_update(request, diff --git a/openstack_dashboard/dashboards/admin/networks/ports/forms.py b/openstack_dashboard/dashboards/admin/networks/ports/forms.py index 308bfa4ccf..6fd9d8feda 100644 --- a/openstack_dashboard/dashboards/admin/networks/ports/forms.py +++ b/openstack_dashboard/dashboards/admin/networks/ports/forms.py @@ -102,7 +102,7 @@ class CreatePort(project_forms.CreatePort): params = { 'tenant_id': network.tenant_id, 'network_id': data['network_id'], - 'admin_state_up': data['admin_state'] == 'True', + 'admin_state_up': data['admin_state'], 'name': data['name'], 'device_id': data['device_id'], 'device_owner': data['device_owner'], @@ -165,7 +165,6 @@ class UpdatePort(project_forms.UpdatePort): try: LOG.debug('params = %s', data) extension_kwargs = {} - data['admin_state'] = (data['admin_state'] == 'True') if 'binding__vnic_type' in data: extension_kwargs['binding__vnic_type'] = \ data['binding__vnic_type'] diff --git a/openstack_dashboard/dashboards/project/firewalls/forms.py b/openstack_dashboard/dashboards/project/firewalls/forms.py index 701ab8ad72..ec50456177 100644 --- a/openstack_dashboard/dashboards/project/firewalls/forms.py +++ b/openstack_dashboard/dashboards/project/firewalls/forms.py @@ -134,10 +134,8 @@ class UpdateFirewall(forms.SelfHandlingForm): label=_("Description"), required=False) firewall_policy_id = forms.ThemableChoiceField(label=_("Policy")) - admin_state_up = forms.ThemableChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State")) - + admin_state_up = forms.BooleanField(label=_("Enable Admin State"), + required=False) failure_url = 'horizon:project:firewalls:index' def __init__(self, request, *args, **kwargs): @@ -165,7 +163,6 @@ class UpdateFirewall(forms.SelfHandlingForm): def handle(self, request, context): firewall_id = self.initial['firewall_id'] name_or_id = context.get('name') or firewall_id - context['admin_state_up'] = (context['admin_state_up'] == 'True') try: firewall = api.fwaas.firewall_update(request, firewall_id, **context) diff --git a/openstack_dashboard/dashboards/project/firewalls/workflows.py b/openstack_dashboard/dashboards/project/firewalls/workflows.py index 7e5fb12636..1723a79388 100644 --- a/openstack_dashboard/dashboards/project/firewalls/workflows.py +++ b/openstack_dashboard/dashboards/project/firewalls/workflows.py @@ -343,9 +343,9 @@ class AddFirewallAction(workflows.Action): label=_("Description"), required=False) firewall_policy_id = forms.ThemableChoiceField(label=_("Policy")) - admin_state_up = forms.ThemableChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State")) + admin_state_up = forms.BooleanField(label=_("Enable Admin State"), + initial=True, + required=False) def __init__(self, request, *args, **kwargs): super(AddFirewallAction, self).__init__(request, *args, **kwargs) @@ -380,11 +380,6 @@ class AddFirewallStep(workflows.Step): contributes = ("name", "firewall_policy_id", "description", "admin_state_up") - def contribute(self, data, context): - context = super(AddFirewallStep, self).contribute(data, context) - context['admin_state_up'] = (context['admin_state_up'] == 'True') - return context - class AddFirewall(workflows.Workflow): slug = "addfirewall" diff --git a/openstack_dashboard/dashboards/project/networks/forms.py b/openstack_dashboard/dashboards/project/networks/forms.py index f28a211bcf..3e2c3d3d79 100644 --- a/openstack_dashboard/dashboards/project/networks/forms.py +++ b/openstack_dashboard/dashboards/project/networks/forms.py @@ -38,11 +38,8 @@ class UpdateNetwork(forms.SelfHandlingForm): network_id = forms.CharField(label=_("ID"), widget=forms.TextInput( attrs={'readonly': 'readonly'})) - admin_state = forms.ThemableChoiceField( - choices=[('True', _('UP')), - ('False', _('DOWN'))], - required=False, - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + required=False) shared = forms.BooleanField(label=_("Shared"), required=False) failure_url = 'horizon:project:networks:index' @@ -54,7 +51,7 @@ class UpdateNetwork(forms.SelfHandlingForm): def handle(self, request, data): try: - params = {'admin_state_up': (data['admin_state'] == 'True'), + params = {'admin_state_up': data['admin_state'], 'name': data['name']} # Make sure we are not sending shared data when the user # doesnt'have admin rights because even if the user doesn't diff --git a/openstack_dashboard/dashboards/project/networks/ports/forms.py b/openstack_dashboard/dashboards/project/networks/ports/forms.py index 8272a66ff6..00d5cfd962 100644 --- a/openstack_dashboard/dashboards/project/networks/ports/forms.py +++ b/openstack_dashboard/dashboards/project/networks/ports/forms.py @@ -44,9 +44,9 @@ class CreatePort(forms.SelfHandlingForm): name = forms.CharField(max_length=255, label=_("Name"), required=False) - admin_state = forms.ChoiceField(choices=[('True', _('UP')), - ('False', _('DOWN'))], - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + initial=True, + required=False) device_id = forms.CharField(max_length=100, label=_("Device ID"), help_text=_("Device ID attached to the port"), required=False) @@ -132,7 +132,7 @@ class CreatePort(forms.SelfHandlingForm): try: params = { 'network_id': data['network_id'], - 'admin_state_up': data['admin_state'] == 'True', + 'admin_state_up': data['admin_state'], 'name': data['name'], 'device_id': data['device_id'], 'device_owner': data['device_owner'] @@ -179,10 +179,8 @@ class UpdatePort(forms.SelfHandlingForm): name = forms.CharField(max_length=255, label=_("Name"), required=False) - admin_state = forms.ThemableChoiceField( - choices=[('True', _('UP')), - ('False', _('DOWN'))], - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + required=False) failure_url = 'horizon:project:networks:detail' def __init__(self, request, *args, **kwargs): @@ -232,7 +230,6 @@ class UpdatePort(forms.SelfHandlingForm): exceptions.handle(self.request, msg) def handle(self, request, data): - data['admin_state'] = (data['admin_state'] == 'True') try: LOG.debug('params = %s', data) extension_kwargs = {} diff --git a/openstack_dashboard/dashboards/project/networks/workflows.py b/openstack_dashboard/dashboards/project/networks/workflows.py index 15a0c3d4c1..12fa00e905 100644 --- a/openstack_dashboard/dashboards/project/networks/workflows.py +++ b/openstack_dashboard/dashboards/project/networks/workflows.py @@ -37,13 +37,11 @@ class CreateNetworkInfoAction(workflows.Action): net_name = forms.CharField(max_length=255, label=_("Network Name"), required=False) - admin_state = forms.ThemableChoiceField( - choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State"), + admin_state = forms.BooleanField( + label=_("Enable Admin State"), + initial=True, required=False, - help_text=_("The state to start" - " the network in.")) + help_text=_("The state to start the network in.")) shared = forms.BooleanField(label=_("Shared"), initial=False, required=False) with_subnet = forms.BooleanField(label=_("Create Subnet"), @@ -463,7 +461,7 @@ class CreateNetwork(workflows.Workflow): def _create_network(self, request, data): try: params = {'name': data['net_name'], - 'admin_state_up': (data['admin_state'] == 'True'), + 'admin_state_up': data['admin_state'], 'shared': data['shared']} network = api.neutron.network_create(request, **params) self.context['net_id'] = network.id diff --git a/openstack_dashboard/dashboards/project/routers/forms.py b/openstack_dashboard/dashboards/project/routers/forms.py index 9b5e726ceb..51cd96a3e7 100644 --- a/openstack_dashboard/dashboards/project/routers/forms.py +++ b/openstack_dashboard/dashboards/project/routers/forms.py @@ -34,10 +34,9 @@ LOG = logging.getLogger(__name__) class CreateForm(forms.SelfHandlingForm): name = forms.CharField(max_length=255, label=_("Router Name"), required=False) - admin_state_up = forms.ThemableChoiceField(label=_("Admin State"), - choices=[(True, _('UP')), - (False, _('DOWN'))], - required=False) + admin_state_up = forms.BooleanField(label=_("Enable Admin State"), + initial=True, + required=False) external_network = forms.ThemableChoiceField(label=_("External Network"), required=False) mode = forms.ChoiceField(label=_("Router Type")) @@ -116,9 +115,8 @@ class CreateForm(forms.SelfHandlingForm): class UpdateForm(forms.SelfHandlingForm): name = forms.CharField(label=_("Name"), required=False) - admin_state = forms.ThemableChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State")) + admin_state = forms.BooleanField(label=_("Enable Admin State"), + required=False) router_id = forms.CharField(label=_("ID"), widget=forms.TextInput( attrs={'readonly': 'readonly'})) @@ -155,7 +153,7 @@ class UpdateForm(forms.SelfHandlingForm): def handle(self, request, data): try: - params = {'admin_state_up': (data['admin_state'] == 'True'), + params = {'admin_state_up': data['admin_state'], 'name': data['name']} if self.dvr_allowed: params['distributed'] = (data['mode'] == 'distributed') diff --git a/openstack_dashboard/dashboards/project/routers/tests.py b/openstack_dashboard/dashboards/project/routers/tests.py index ae2c271486..2f60aed9fa 100644 --- a/openstack_dashboard/dashboards/project/routers/tests.py +++ b/openstack_dashboard/dashboards/project/routers/tests.py @@ -264,7 +264,7 @@ class RouterActionTests(RouterMixin, test.TestCase): api.neutron.network_list(IsA(http.HttpRequest))\ .AndReturn(self.networks.list()) params = {'name': router.name, - 'admin_state_up': str(router.admin_state_up)} + 'admin_state_up': router.admin_state_up} api.neutron.router_create(IsA(http.HttpRequest), **params)\ .AndReturn(router) @@ -291,7 +291,7 @@ class RouterActionTests(RouterMixin, test.TestCase): api.neutron.network_list(IsA(http.HttpRequest))\ .AndReturn(self.networks.list()) params = {'name': router.name, - 'admin_state_up': str(router.admin_state_up)} + 'admin_state_up': router.admin_state_up} api.neutron.router_create(IsA(http.HttpRequest), **params)\ .AndReturn(router) @@ -322,7 +322,7 @@ class RouterActionTests(RouterMixin, test.TestCase): param = {'name': router.name, 'distributed': True, 'ha': True, - 'admin_state_up': str(router.admin_state_up)} + 'admin_state_up': router.admin_state_up} api.neutron.router_create(IsA(http.HttpRequest), **param)\ .AndReturn(router) @@ -352,7 +352,7 @@ class RouterActionTests(RouterMixin, test.TestCase): api.neutron.network_list(IsA(http.HttpRequest))\ .MultipleTimes().AndReturn(self.networks.list()) params = {'name': router.name, - 'admin_state_up': str(router.admin_state_up)} + 'admin_state_up': router.admin_state_up} api.neutron.router_create(IsA(http.HttpRequest), **params)\ .AndRaise(self.exceptions.neutron) self.mox.ReplayAll() @@ -380,7 +380,7 @@ class RouterActionTests(RouterMixin, test.TestCase): api.neutron.network_list(IsA(http.HttpRequest))\ .MultipleTimes().AndReturn(self.networks.list()) params = {'name': router.name, - 'admin_state_up': str(router.admin_state_up)} + 'admin_state_up': router.admin_state_up} api.neutron.router_create(IsA(http.HttpRequest), **params)\ .AndRaise(self.exceptions.neutron) self.mox.ReplayAll() diff --git a/openstack_dashboard/dashboards/project/vpn/forms.py b/openstack_dashboard/dashboards/project/vpn/forms.py index 1ada5d8c9c..e7b126f36b 100644 --- a/openstack_dashboard/dashboards/project/vpn/forms.py +++ b/openstack_dashboard/dashboards/project/vpn/forms.py @@ -34,15 +34,12 @@ class UpdateVPNService(forms.SelfHandlingForm): widget=forms.TextInput(attrs={'readonly': 'readonly'})) description = forms.CharField( required=False, max_length=80, label=_("Description")) - admin_state_up = forms.ChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State"), - required=False) + admin_state_up = forms.BooleanField(label=_("Enable Admin State"), + required=False) failure_url = 'horizon:project:vpn:index' def handle(self, request, context): - context['admin_state_up'] = (context['admin_state_up'] == 'True') try: data = {'vpnservice': {'name': context['name'], 'description': context['description'], @@ -279,10 +276,8 @@ class UpdateIPSecSiteConnection(forms.SelfHandlingForm): required=False, choices=[('bi-directional', _('bi-directional')), ('response-only', _('response-only'))]) - admin_state_up = forms.ChoiceField(choices=[(True, _('UP')), - (False, _('DOWN'))], - label=_("Admin State"), - required=False) + admin_state_up = forms.BooleanField(label=_("Enable Admin State"), + required=False) failure_url = 'horizon:project:vpn:index' @@ -297,7 +292,6 @@ class UpdateIPSecSiteConnection(forms.SelfHandlingForm): return cleaned_data def handle(self, request, context): - context['admin_state_up'] = (context['admin_state_up'] == 'True') try: data = {'ipsec_site_connection': {'name': context['name'], diff --git a/openstack_dashboard/dashboards/project/vpn/templates/vpn/_add_vpn_service_help.html b/openstack_dashboard/dashboards/project/vpn/templates/vpn/_add_vpn_service_help.html index 3ce676b30f..4d682dc94e 100644 --- a/openstack_dashboard/dashboards/project/vpn/templates/vpn/_add_vpn_service_help.html +++ b/openstack_dashboard/dashboards/project/vpn/templates/vpn/_add_vpn_service_help.html @@ -3,5 +3,5 @@

{% trans "Create VPN service for current project." %}

{% trans "The VPN service is attached to a router and references to a single subnet to push to a remote site." %}

{% trans "Specify a name, description, router, and subnet for the VPN Service." %}

-

{% trans "Admin State is UP (True) by default." %}

-

{% trans "The router, subnet and admin state fields are required. All others are optional." %}

+

{% trans "Admin State is enabled by default." %}

+

{% trans "The router, subnet and admin state fields require to be enabled. All others are optional." %}

diff --git a/openstack_dashboard/dashboards/project/vpn/workflows.py b/openstack_dashboard/dashboards/project/vpn/workflows.py index f8b52aa815..699089edf6 100644 --- a/openstack_dashboard/dashboards/project/vpn/workflows.py +++ b/openstack_dashboard/dashboards/project/vpn/workflows.py @@ -28,11 +28,11 @@ class AddVPNServiceAction(workflows.Action): max_length=80, label=_("Description")) router_id = forms.ChoiceField(label=_("Router")) subnet_id = forms.ChoiceField(label=_("Subnet")) - admin_state_up = forms.ChoiceField( - choices=[(True, _('UP')), (False, _('DOWN'))], - label=_("Admin State"), - help_text=_("The state of VPN service to start in. " - "If DOWN (False) VPN service does not forward packets."), + admin_state_up = forms.BooleanField( + label=_("Enable Admin State"), + help_text=_("The state of VPN service to start in. If disabled " + "(not checked), VPN service does not forward packets."), + initial=True, required=False) def __init__(self, request, *args, **kwargs): @@ -426,13 +426,13 @@ class AddIPSecSiteConnectionOptionalAction(workflows.Action): required=False, help_text=_("Valid integer greater than the DPD interval")) initiator = forms.ChoiceField(label=_("Initiator state"), required=False) - admin_state_up = forms.ChoiceField( - choices=[(True, _('UP')), (False, _('DOWN'))], - label=_("Admin State"), - required=False, + admin_state_up = forms.BooleanField( + label=_("Enable Admin State"), help_text=_("The state of IPSec site connection to start in. " - "If DOWN (False), IPSec site connection " - "does not forward packets.")) + "If disabled (not checked), IPSec site connection " + "does not forward packets."), + initial=True, + required=False) def __init__(self, request, *args, **kwargs): super(AddIPSecSiteConnectionOptionalAction, self).__init__(