Fix parsing names in switched fields

In the share creation form dialog, share network
selection is optionally provided based on whether
the share type chosen supports the DHSS extra-spec.
This selection breaks often when dealing with share
types that have a name matching the format: ".*-\d+.*".
For example: test-700. The root cause of this seems to
be in javascript code for "switchable" fields [1] that
doesn't get triggered as expected.

A similar issue manifests in the share Network Creation
form where we setup switched fields with the Neutron
network IDs (dashed format UUIDs) and in the Share
Group Creation form where we setup switched fields
with the Share Group Type IDs (dashed format UUIDs).

So, we could encode the "-" in these field names to
workaround this issue.

Unfortunately this isn't a clean cherry-pick since
we need to support users running manila-ui under
python2 in this branch - and unicode wrangling
isn't cleanly possible due to some assumptions
django makes about field names in the forms code as
well as in tests.

Closes-Bug: #1931641
[1] 647c2b7530/horizon/static/horizon/js/horizon.forms.js (L491-L613)

Change-Id: Id924fc55debdc38ae2131bf8cef396f28caa3e77
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
(cherry picked from commit 37e5b2f053)
(cherry picked from commit b7d87e886a)
(cherry picked from commit 6ad8fd882a)
(cherry picked from commit a97f5e6750)
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>

manila_ui/dashboards/project/share_groups/forms.py

@@ -23,6 +23,7 @@ from horizon.utils import memoized
 import six
 
 from manila_ui.api import manila
+from manila_ui.dashboards import utils
 
 
 class CreateShareGroupForm(forms.SelfHandlingForm):
@@ -127,7 +128,9 @@ class CreateShareGroupForm(forms.SelfHandlingForm):
                 }),
                 required=True)
             self.fields["sgt"].choices = (
-                [("", "")] + [(sgt.id, sgt.name) for sgt in share_group_types])
+                [("", "")] + [(utils.transform_dashed_name(sgt.id), sgt.name)
+                              for sgt in share_group_types]
+            )
 
             # NOTE(vponomaryov): create separate set of available share types
             # for each of share group types.
@@ -145,8 +148,8 @@ class CreateShareGroupForm(forms.SelfHandlingForm):
                         widget=forms.fields.SelectWidget(attrs={
                             "class": "switched",
                             "data-switch-on": "sgt",
-                            "data-sgt-%s" % sgt.id: _(
-                                "Share Types (one available)"),
+                            "data-sgt-%s" % utils.transform_dashed_name(
+                                sgt.id): _("Share Types (one available)"),
                         }),
                         required=True)
                 else:
@@ -158,8 +161,8 @@ class CreateShareGroupForm(forms.SelfHandlingForm):
                             "style": "max-height: %spx;" % height,
                             "class": "switched",
                             "data-switch-on": "sgt",
-                            "data-sgt-%s" % sgt.id: _(
-                                "Share Types (multiple available)"),
+                            "data-sgt-%s" % utils.transform_dashed_name(
+                                sgt.id): _("Share Types (multiple available)"),
                         }),
                         required=False)
                     st_field.initial = st_choices[0]
@@ -221,7 +224,7 @@ class CreateShareGroupForm(forms.SelfHandlingForm):
                 request,
                 name=data['name'],
                 description=data['description'],
-                share_group_type=data['sgt'],
+                share_group_type=utils.transform_dashed_name(data['sgt']),
                 share_types=None if snapshot_id else data.get('share_types'),
                 share_network=(
                     None if snapshot_id else data.get('share_network')),

manila_ui/dashboards/project/share_networks/forms.py

@@ -23,6 +23,7 @@ from openstack_dashboard.api import neutron
 
 from manila_ui.api import manila
 from manila_ui.api import network
+from manila_ui.dashboards import utils
 
 
 class Create(forms.SelfHandlingForm):
@@ -36,20 +37,24 @@ class Create(forms.SelfHandlingForm):
         net_choices = network.network_list(request)
         if self.neutron_enabled:
             self.fields['neutron_net_id'] = forms.ChoiceField(
-                choices=[(' ', ' ')] + [(choice.id, choice.name_or_id)
-                                        for choice in net_choices],
+                choices=[(' ', ' ')] +
+                        [(utils.transform_dashed_name(choice.id),
+                          choice.name_or_id) for choice in net_choices],
                 label=_("Neutron Net"), widget=forms.Select(
                     attrs={'class': 'switchable', 'data-slug': 'net'}))
             for net in net_choices:
                 # For each network create switched choice field with
                 # the its subnet choices
-                subnet_field_name = 'subnet-choices-%s' % net.id
+                subnet_field_name = (
+                    'subnet-choices-%s' % utils.transform_dashed_name(net.id)
+                )
                 subnet_field = forms.ChoiceField(
                     choices=(), label=_("Neutron Subnet"),
                     widget=forms.Select(attrs={
                         'class': 'switched',
                         'data-switch-on': 'net',
-                        'data-net-%s' % net.id: _("Neutron Subnet")
+                        'data-net-%s' % utils.transform_dashed_name(net.id):
+                            _("Neutron Subnet")
                     }))
                 self.fields[subnet_field_name] = subnet_field
                 subnet_choices = neutron.subnet_list(
@@ -63,10 +68,11 @@ class Create(forms.SelfHandlingForm):
             send_data = {'name': data['name']}
             if data['description']:
                 send_data['description'] = data['description']
-            share_net_id = data.get('neutron_net_id')
-            if self.neutron_enabled and share_net_id:
-                send_data['neutron_net_id'] = share_net_id.strip()
-                subnet_key = 'subnet-choices-%s' % share_net_id
+            neutron_net_id = data.get('neutron_net_id')
+            if self.neutron_enabled and neutron_net_id:
+                send_data['neutron_net_id'] = utils.transform_dashed_name(
+                    neutron_net_id.strip())
+                subnet_key = 'subnet-choices-%s' % neutron_net_id
                 if subnet_key in data:
                     send_data['neutron_subnet_id'] = data[subnet_key]
             share_network = manila.share_network_create(request, **send_data)

manila_ui/dashboards/project/shares/forms.py

@@ -58,7 +58,10 @@ class CreateForm(forms.SelfHandlingForm):
         share_networks = manila.share_network_list(request)
         share_types = manila.share_type_list(request)
         self.fields['share_type'].choices = (
-            [("", "")] + [(st.name, st.name) for st in share_types])
+            [("", "")] +
+            [(utils.transform_dashed_name(st.name), st.name)
+             for st in share_types]
+        )
 
         availability_zones = manila.availability_zone_list(request)
         self.fields['availability_zone'].choices = (
@@ -83,14 +86,18 @@ class CreateForm(forms.SelfHandlingForm):
                 sn_choices = (
                     [('', '')] +
                     [(sn.id, sn.name or sn.id) for sn in share_networks])
-                sn_field_name = self.sn_field_name_prefix + st.name
+                sn_field_name = (
+                    self.sn_field_name_prefix +
+                    utils.transform_dashed_name(st.name)
+                )
                 sn_field = forms.ChoiceField(
                     label=_("Share Network"), required=True,
                     choices=sn_choices,
                     widget=forms.Select(attrs={
                         'class': 'switched',
                         'data-switch-on': 'sharetype',
-                        'data-sharetype-%s' % st.name: _("Share Network"),
+                        'data-sharetype-%s' % utils.transform_dashed_name(
+                            st.name): _("Share Network"),
                     }))
                 self.fields[sn_field_name] = sn_field
 
@@ -176,20 +183,19 @@ class CreateForm(forms.SelfHandlingForm):
         cleaned_data = super(CreateForm, self).clean()
         form_errors = list(self.errors)
 
-        # NOTE(vponomaryov): skip errors for share-network fields that are not
-        # related to chosen share type.
-        for error in form_errors:
-            st_name = error.split(self.sn_field_name_prefix)[-1]
-            chosen_st = cleaned_data.get('share_type')
-            if (error.startswith(self.sn_field_name_prefix) and
-                    st_name != chosen_st):
-                cleaned_data[error] = 'Not set'
-                self.errors.pop(error, None)
+        chosen_share_type = cleaned_data.get('share_type')
+        if chosen_share_type:
+            # NOTE(vponomaryov): skip errors for share-network fields that are
+            # not related to chosen share type.
+            for error in form_errors:
+                st_name = error.split(self.sn_field_name_prefix)[-1]
+                if (error.startswith(self.sn_field_name_prefix) and
+                        st_name != chosen_share_type):
+                    cleaned_data[error] = 'Not set'
+                    self.errors.pop(error, None)
 
-        share_type = cleaned_data.get('share_type')
-        if share_type:
             cleaned_data['share_network'] = cleaned_data.get(
-                self.sn_field_name_prefix + share_type)
+                self.sn_field_name_prefix + cleaned_data.get('share_type'))
 
         return cleaned_data
 
@@ -230,7 +236,7 @@ class CreateForm(forms.SelfHandlingForm):
                 proto=data['share_proto'],
                 share_network=share_network,
                 snapshot_id=snapshot_id,
-                share_type=data['share_type'],
+                share_type=utils.transform_dashed_name(data['share_type']),
                 is_public=is_public,
                 metadata=metadata,
                 availability_zone=data['availability_zone'],

manila_ui/dashboards/utils.py

@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -15,6 +16,7 @@
 from django.forms import ValidationError
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext_lazy as _
+import six
 
 
 html_escape_table = {
@@ -111,3 +113,15 @@ def calculate_longest_str_size(str_list):
         if current_size > size:
             size = current_size
     return size
+
+
+def transform_dashed_name(name):
+    """Add or remove unicode text separators & transformations for hyphens."""
+    if not name:
+        return
+    separator = u'__ಠ__' if six.PY3 else '____'
+    if '-' in name:
+        name = separator + name.replace('-', '__u2010') + separator
+    elif '__u2010' in name:
+        name = name.replace('__u2010', '-').strip(separator)
+    return name

manila_ui/tests/dashboards/project/share_groups/tests.py

@@ -18,6 +18,7 @@ from django.urls import reverse
 import mock
 
 from manila_ui.api import manila as api_manila
+from manila_ui.dashboards import utils
 from manila_ui.tests.dashboards.project import test_data
 from manila_ui.tests import helpers as test
 
@@ -255,9 +256,11 @@ class ShareGroupTests(test.TestCase):
             'method': 'CreateShareGroupForm',
             'name': 'fake_sg_name',
             'description': 'fake SG description',
-            'sgt': test_data.share_group_type.id,
-            'share-type-choices-%s' % test_data.share_group_type.id: (
-                test_data.share_group_type.share_types[0]),
+            'sgt': utils.transform_dashed_name(test_data.share_group_type.id),
+            'share-type-choices-%s' % utils.transform_dashed_name(
+                test_data.share_group_type.id): (
+                    test_data.share_group_type.share_types[0]
+            ),
             'availability_zone': '',
             'share_network': test_data.inactive_share_network.id,
             'snapshot': '',

manila_ui/tests/dashboards/project/share_networks/tests.py

@@ -20,6 +20,7 @@ from openstack_dashboard import api
 
 from manila_ui.api import manila as api_manila
 from manila_ui.api import network as api_manila_network
+from manila_ui.dashboards import utils
 from manila_ui.tests.dashboards.project import test_data
 from manila_ui.tests import helpers as test
 
@@ -36,10 +37,12 @@ class ShareNetworksViewTests(test.TestCase):
             'name': 'new_share_network',
             'description': 'This is test share network',
             'method': 'CreateForm',
-            'neutron_net_id': neutron_net_id,
+            'neutron_net_id': utils.transform_dashed_name(neutron_net_id),
         }
         for net in self.networks.list():
-            formData['subnet-choices-%s' % net.id] = net.subnets[0].id
+            sanitized_net_id = utils.transform_dashed_name(net.id)
+            subnet_choices_field = 'subnet-choices-%s' % sanitized_net_id
+            formData[subnet_choices_field] = net.subnets[0].id
         self.mock_object(
             api.neutron, "subnet_list",
             mock.Mock(return_value=self.subnets.list()))
@@ -52,9 +55,11 @@ class ShareNetworksViewTests(test.TestCase):
 
         self.client.post(url, formData)
 
-        api_manila.share_network_create.assert_called_with(
+        sanitized_neutron_net_field = formData[
+            'subnet-choices-%s' % utils.transform_dashed_name(neutron_net_id)]
+        api_manila.share_network_create.assert_called_once_with(
             mock.ANY, name=formData['name'], neutron_net_id=neutron_net_id,
-            neutron_subnet_id=formData['subnet-choices-%s' % neutron_net_id],
+            neutron_subnet_id=sanitized_neutron_net_field,
             description=formData['description'])
         api_manila_network.network_list.assert_called_once_with(mock.ANY)
         api.neutron.subnet_list.assert_has_calls([

manila_ui/tests/dashboards/project/shares/tests.py

@@ -93,6 +93,11 @@ class ShareViewTests(test.APITestCase):
         share = test_data.share
         share_net = test_data.active_share_network
         share_nets = [share_net]
+        fake_share_type = mock.Mock()
+        fake_share_type.name = 'fake-type'
+        fake_share_type.id = '5f3f4705-153d-4864-9930-a01c6bbea0bb'
+        fake_share_type.get_keys = mock.Mock(return_value={
+            'driver_handles_share_servers': 'True'})
         formData = {
             'name': u'new_share',
             'description': u'This is test share',
@@ -100,8 +105,9 @@ class ShareViewTests(test.APITestCase):
             'share_network': share_net.id,
             'size': 1,
             'share_proto': u'NFS',
-            'share_type': 'fake',
-            'share-network-choices-fake': share_net.id,
+            'share_type': utils.transform_dashed_name('fake-type'),
+            'share-network-choices-%s' % utils.transform_dashed_name(
+                'fake-type'): share_net.id,
             'availability_zone': 'fake_az',
         }
 
@@ -115,7 +121,7 @@ class ShareViewTests(test.APITestCase):
             mock.Mock(return_value=share_nets))
         self.mock_object(
             api_manila, "share_type_list",
-            mock.Mock(return_value=[self.fake_share_type, ]))
+            mock.Mock(return_value=[fake_share_type, ]))
 
         res = self.client.post(url, formData)
 
@@ -125,7 +131,7 @@ class ShareViewTests(test.APITestCase):
             description=formData['description'], proto=formData['share_proto'],
             snapshot_id=None, is_public=False,
             share_group_id=None, share_network=share_net.id,
-            metadata={}, share_type=formData['share_type'],
+            metadata={}, share_type='fake-type',
             availability_zone=formData['availability_zone'])
         api_manila.share_snapshot_list.assert_called_once_with(mock.ANY)
         api_manila.share_network_list.assert_called_once_with(mock.ANY)

releasenotes/notes/bug-1931641-fix-handling-share-type-name-with-hyphens-8a9f16af36da5852.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Visibility of "switchable" fields in the share creation form involving
+    share types that had hyphens in their names is now fixed. See `Launchpad
+    bug #1931641 <https://launchpad.net/bugs/1931641>`_ for more details.