diff --git a/manila_ui/dashboards/project/share_groups/forms.py b/manila_ui/dashboards/project/share_groups/forms.py index abd2640c..97449b07 100644 --- a/manila_ui/dashboards/project/share_groups/forms.py +++ b/manila_ui/dashboards/project/share_groups/forms.py @@ -22,6 +22,7 @@ from horizon import messages from horizon.utils import memoized from manila_ui.api import manila +from manila_ui.dashboards import utils class CreateShareGroupForm(forms.SelfHandlingForm): @@ -126,7 +127,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. @@ -144,8 +147,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: @@ -157,8 +160,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] @@ -220,7 +223,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')), diff --git a/manila_ui/dashboards/project/share_networks/forms.py b/manila_ui/dashboards/project/share_networks/forms.py index 415fbd8b..81a8f21f 100644 --- a/manila_ui/dashboards/project/share_networks/forms.py +++ b/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) diff --git a/manila_ui/dashboards/project/shares/forms.py b/manila_ui/dashboards/project/shares/forms.py index 9b1aa0fd..b0e4f3da 100644 --- a/manila_ui/dashboards/project/shares/forms.py +++ b/manila_ui/dashboards/project/shares/forms.py @@ -57,7 +57,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 = ( @@ -81,14 +84,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 @@ -174,20 +181,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 @@ -228,7 +234,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'], diff --git a/manila_ui/dashboards/utils.py b/manila_ui/dashboards/utils.py index af3c7946..33d905ed 100644 --- a/manila_ui/dashboards/utils.py +++ b/manila_ui/dashboards/utils.py @@ -111,3 +111,14 @@ 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 + if '-' in name: + name = '__ಠ__' + name.replace('-', '__u2010') + '__ಠ__' + elif '__u2010' in name: + name = name.replace('__u2010', '-').strip('__ಠ__') + return name diff --git a/manila_ui/tests/dashboards/project/share_groups/tests.py b/manila_ui/tests/dashboards/project/share_groups/tests.py index de664fc7..122ff01e 100644 --- a/manila_ui/tests/dashboards/project/share_groups/tests.py +++ b/manila_ui/tests/dashboards/project/share_groups/tests.py @@ -18,6 +18,7 @@ from django.urls import reverse from unittest 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 @@ -264,9 +265,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': '', diff --git a/manila_ui/tests/dashboards/project/share_networks/tests.py b/manila_ui/tests/dashboards/project/share_networks/tests.py index ee305694..bc54c6a6 100644 --- a/manila_ui/tests/dashboards/project/share_networks/tests.py +++ b/manila_ui/tests/dashboards/project/share_networks/tests.py @@ -20,6 +20,7 @@ from unittest import mock 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([ diff --git a/manila_ui/tests/dashboards/project/shares/tests.py b/manila_ui/tests/dashboards/project/shares/tests.py index d4556749..e62fc0cb 100644 --- a/manila_ui/tests/dashboards/project/shares/tests.py +++ b/manila_ui/tests/dashboards/project/shares/tests.py @@ -92,6 +92,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', @@ -99,8 +104,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', } @@ -114,7 +120,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) @@ -124,7 +130,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) diff --git a/releasenotes/notes/bug-1931641-fix-handling-share-type-name-with-hyphens-8a9f16af36da5852.yaml b/releasenotes/notes/bug-1931641-fix-handling-share-type-name-with-hyphens-8a9f16af36da5852.yaml new file mode 100644 index 00000000..b6628f86 --- /dev/null +++ b/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 `_ for more details.