Merge "Fix share replica details page for non-admins"
This commit is contained in:
commit
3d224a93bd
|
@ -87,6 +87,9 @@ class DetailReplicaView(tabs.TabView):
|
||||||
try:
|
try:
|
||||||
replica_id = self.kwargs['replica_id']
|
replica_id = self.kwargs['replica_id']
|
||||||
replica = manila.share_replica_get(self.request, replica_id)
|
replica = manila.share_replica_get(self.request, replica_id)
|
||||||
|
try:
|
||||||
|
# The default policy for this API does not allow
|
||||||
|
# non-admins to retrieve export locations.
|
||||||
replica.export_locations = (
|
replica.export_locations = (
|
||||||
manila.share_instance_export_location_list(
|
manila.share_instance_export_location_list(
|
||||||
self.request, replica_id))
|
self.request, replica_id))
|
||||||
|
@ -95,12 +98,14 @@ class DetailReplicaView(tabs.TabView):
|
||||||
]
|
]
|
||||||
replica.el_size = ui_utils.calculate_longest_str_size(
|
replica.el_size = ui_utils.calculate_longest_str_size(
|
||||||
export_locations)
|
export_locations)
|
||||||
|
except Exception:
|
||||||
|
replica.export_locations = []
|
||||||
except Exception:
|
except Exception:
|
||||||
redirect = reverse(self._redirect_url)
|
redirect = reverse(self._redirect_url)
|
||||||
exceptions.handle(
|
exceptions.handle(
|
||||||
self.request,
|
self.request,
|
||||||
_('Unable to retrieve replica %sdetails.') % replica_id,
|
_('Unable to retrieve details of replica %s') %
|
||||||
redirect=redirect)
|
self.kwargs['replica_id'], redirect=redirect)
|
||||||
return replica
|
return replica
|
||||||
|
|
||||||
def get_tabs(self, request, *args, **kwargs):
|
def get_tabs(self, request, *args, **kwargs):
|
||||||
|
|
|
@ -13,6 +13,7 @@
|
||||||
<dd>{{ replica.status|capfirst }}</dd>
|
<dd>{{ replica.status|capfirst }}</dd>
|
||||||
<dt>{% trans "Replica state" %}</dt>
|
<dt>{% trans "Replica state" %}</dt>
|
||||||
<dd>{{ replica.replica_state|capfirst }}</dd>
|
<dd>{{ replica.replica_state|capfirst }}</dd>
|
||||||
|
{% if replica.export_locations %}
|
||||||
<dt>{% trans "Export locations" %}</dt>
|
<dt>{% trans "Export locations" %}</dt>
|
||||||
{% for el in replica.export_locations %}
|
{% for el in replica.export_locations %}
|
||||||
<dd><p>
|
<dd><p>
|
||||||
|
@ -27,6 +28,7 @@
|
||||||
{% endif %}
|
{% endif %}
|
||||||
</p></dd>
|
</p></dd>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
|
{% endif %}
|
||||||
<dt>{% trans "Availability zone" %}</dt>
|
<dt>{% trans "Availability zone" %}</dt>
|
||||||
<dd>{{ replica.availability_zone }}</dd>
|
<dd>{{ replica.availability_zone }}</dd>
|
||||||
|
|
||||||
|
|
|
@ -103,17 +103,22 @@ class ReplicasTests(test.TestCase):
|
||||||
mock.Mock(side_effect=Exception("Fake exception")))
|
mock.Mock(side_effect=Exception("Fake exception")))
|
||||||
self._create_post()
|
self._create_post()
|
||||||
|
|
||||||
def test_detail(self):
|
@ddt.data(True, False)
|
||||||
|
def test_detail_with_export_locations_available(self, exports_available):
|
||||||
url = reverse(
|
url = reverse(
|
||||||
"horizon:project:shares:replica_detail",
|
"horizon:project:shares:replica_detail",
|
||||||
args=[self.share_replica.id])
|
args=[self.share_replica.id])
|
||||||
export_locations = test_data.export_locations
|
|
||||||
|
export_locations_call_behavior = (
|
||||||
|
{'return_value': test_data.export_locations} if exports_available
|
||||||
|
else {'side_effect': Exception("Access denied to this resource")}
|
||||||
|
)
|
||||||
self.mock_object(
|
self.mock_object(
|
||||||
api_manila, "share_replica_get",
|
api_manila, "share_replica_get",
|
||||||
mock.Mock(return_value=self.share_replica))
|
mock.Mock(return_value=self.share_replica))
|
||||||
self.mock_object(
|
self.mock_object(
|
||||||
api_manila, "share_instance_export_location_list",
|
api_manila, "share_instance_export_location_list",
|
||||||
mock.Mock(return_value=export_locations))
|
mock.Mock(**export_locations_call_behavior))
|
||||||
|
|
||||||
res = self.client.get(url)
|
res = self.client.get(url)
|
||||||
|
|
||||||
|
@ -125,10 +130,12 @@ class ReplicasTests(test.TestCase):
|
||||||
self.assertContains(res, "<dd>%s</dd>" % self.share_replica.id, 1, 200)
|
self.assertContains(res, "<dd>%s</dd>" % self.share_replica.id, 1, 200)
|
||||||
self.assertContains(
|
self.assertContains(
|
||||||
res, "<dd>%s</dd>" % self.share.availability_zone, 1, 200)
|
res, "<dd>%s</dd>" % self.share.availability_zone, 1, 200)
|
||||||
for el in export_locations:
|
if exports_available:
|
||||||
|
for el in test_data.export_locations:
|
||||||
self.assertContains(res, "value=\"%s\"" % el.path, 1, 200)
|
self.assertContains(res, "value=\"%s\"" % el.path, 1, 200)
|
||||||
self.assertContains(
|
self.assertContains(
|
||||||
res, "<div><b>Preferred:</b> %s</div>" % el.preferred, 1, 200)
|
res, "<div><b>Preferred:</b> %s</div>" % el.preferred,
|
||||||
|
1, 200)
|
||||||
self.assertContains(
|
self.assertContains(
|
||||||
res, "<div><b>Is admin only:</b> %s</div>" % el.is_admin_only,
|
res, "<div><b>Is admin only:</b> %s</div>" % el.is_admin_only,
|
||||||
1, 200)
|
1, 200)
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
The Share replica details page has been fixed to render correctly for
|
||||||
|
users with non-admin roles.
|
Loading…
Reference in New Issue