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.
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>
This commit is contained in:
parent
4edb837a1b
commit
37e5b2f053
|
@ -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')),
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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 = (
|
||||
|
@ -82,14 +85,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'],
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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': '',
|
||||
|
|
|
@ -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([
|
||||
|
|
|
@ -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': 'new_share',
|
||||
'description': 'This is test share',
|
||||
|
@ -99,8 +104,9 @@ class ShareViewTests(test.APITestCase):
|
|||
'share_network': share_net.id,
|
||||
'size': 1,
|
||||
'share_proto': '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)
|
||||
|
|
|
@ -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.
|
Loading…
Reference in New Issue