mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
fix(storages): a refused new volume comes back in its dialog
A New volume form the pool page refused, for a name with a space or a character not allowed, redirected back with the reasons at the top of the page and lost what was typed. The form now posts to the pool page itself: a valid one makes the volume and redirects as before, a refused one renders the page with the dialog open on what was entered, the refused field marked with its reason and focused. A pool stopped meanwhile has no dialog: its page shows the reasons. The separate create_volume view and URL go.
This commit is contained in:
catborise
parent
9d3e3bd315
commit
f77439f398
7 changed files with 63 additions and 81 deletions
|
|
@ -6,7 +6,7 @@ from interfaces.views import interface, interfaces
|
|||
from networks.views import network, networks
|
||||
from nodedevs.views import nodedevs
|
||||
from nwfilters.views import nwfilter, nwfilters
|
||||
from storages.views import create_volume, get_volumes, storage, storages
|
||||
from storages.views import get_volumes, storage, storages
|
||||
|
||||
from . import forms, views
|
||||
|
||||
|
|
@ -66,11 +66,6 @@ urlpatterns = [
|
|||
path("storages/", storages, name="storages"),
|
||||
path("storage/<str:pool>/volumes/", get_volumes, name="volumes"),
|
||||
path("storage/<str:pool>/", storage, name="storage"),
|
||||
path(
|
||||
"storage/<str:pool>/create_volume/",
|
||||
create_volume,
|
||||
name="create_volume",
|
||||
),
|
||||
path("networks/", networks, name="networks"),
|
||||
path("network/<str:pool>/", network, name="network"),
|
||||
path("interfaces/", interfaces, name="interfaces"),
|
||||
|
|
|
|||
|
|
@ -153,18 +153,6 @@ class InstancesTestCase(TestCase):
|
|||
)
|
||||
self.assertRaises(Http404)
|
||||
|
||||
# def test_create_volume(self):
|
||||
# response = self.client.post(
|
||||
# reverse('create_volume', args=[self.compute.id, 'default']),
|
||||
# {
|
||||
# 'name': 'test',
|
||||
# 'format': 'qcow2',
|
||||
# 'size': '1',
|
||||
# 'meta_prealloc': False,
|
||||
# },
|
||||
# )
|
||||
# self.assertRedirects(response, reverse('storage', args=[self.compute.id, 'default']))
|
||||
|
||||
def test_create_destroy_instance(self):
|
||||
# Create
|
||||
response = self.client.get(
|
||||
|
|
@ -1126,18 +1114,6 @@ class InstancesTestCase(TestCase):
|
|||
# del self.instance.status
|
||||
# self.assertEqual(self.instance.status, 1)
|
||||
|
||||
# # create volume
|
||||
# response = self.client.post(
|
||||
# reverse('create_volume', args=[compute.id, 'default']),
|
||||
# {
|
||||
# 'name': 'test3',
|
||||
# 'format': 'qcow2',
|
||||
# 'size': '1',
|
||||
# 'meta_prealloc': False,
|
||||
# },
|
||||
# )
|
||||
# self.assertRedirects(response, reverse('storage', args=[compute.id, 'default']))
|
||||
|
||||
# # delete volume
|
||||
# response = self.client.post(
|
||||
# reverse('instances:delete_vol', args=[instance.id]),
|
||||
|
|
|
|||
|
|
@ -41,7 +41,8 @@
|
|||
<span aria-hidden="true">{% bs_icon 'plus-lg' %}</span> {% trans "New volume" %}
|
||||
</button>
|
||||
|
||||
<div class="modal fade" id="AddImage" tabindex="-1" role="dialog" aria-labelledby="AddImageLabel" aria-hidden="true">
|
||||
<div class="modal fade" id="AddImage" tabindex="-1" role="dialog" aria-labelledby="AddImageLabel" aria-hidden="true"
|
||||
{% if form.errors %}data-open{% endif %}>
|
||||
<div class="modal-dialog">
|
||||
<div class="modal-content">
|
||||
<div class="modal-header">
|
||||
|
|
@ -49,7 +50,7 @@
|
|||
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="{% trans "Close" %}"></button>
|
||||
</div>
|
||||
<div class="modal-body">
|
||||
<form action="{% url 'create_volume' compute_id pool %}" id="create-volume" method="post" aria-label="{% trans "Create volume to storage form" %}">
|
||||
<form id="create-volume" method="post" aria-label="{% trans "Create volume to storage form" %}">
|
||||
{% csrf_token %}
|
||||
<div class="row">
|
||||
<div class="col-12">{% bootstrap_field form.name %}</div>
|
||||
|
|
@ -61,7 +62,7 @@
|
|||
</div>
|
||||
<div class="modal-footer">
|
||||
<button type="button" class="btn btn-outline-secondary" data-bs-dismiss="modal">{% trans "Cancel" %}</button>
|
||||
<button type="submit" form="create-volume" class="btn btn-primary">{% trans "Create" %}</button>
|
||||
<button type="submit" form="create-volume" class="btn btn-primary" name="create_volume">{% trans "Create" %}</button>
|
||||
</div>
|
||||
</div> <!-- /.modal-content -->
|
||||
</div> <!-- /.modal-dialog -->
|
||||
|
|
|
|||
|
|
@ -238,6 +238,13 @@
|
|||
});
|
||||
$('#clone-convert, #clone-format').on('change', showCloneRows);
|
||||
|
||||
// a refused new volume: its dialog opens again on what was entered, at the refused field
|
||||
var addImage = document.getElementById('AddImage');
|
||||
if (addImage && addImage.hasAttribute('data-open')) {
|
||||
addImage.addEventListener('shown.bs.modal', focusFirstError, {once: true});
|
||||
bootstrap.Modal.getOrCreateInstance(addImage).show();
|
||||
}
|
||||
|
||||
$(document).ready(function() {
|
||||
var pendingUpload = null;
|
||||
var uploadInProgress = false;
|
||||
|
|
|
|||
|
|
@ -179,8 +179,7 @@ class PoolAndVolumeRowTests(TransactionTestCase):
|
|||
])
|
||||
|
||||
def test_volume_add_and_delete(self):
|
||||
self.client.post(reverse("create_volume", args=[self.compute.pk, "default"]),
|
||||
{"name": "disk1", "format": "qcow2", "size": 1})
|
||||
self.client.post(self.url, {"name": "disk1", "format": "qcow2", "size": 1, "create_volume": ""})
|
||||
self.client.post(self.url, {"del_volume": "1", "volname": "disk1.qcow2"})
|
||||
with patch("storages.views.libvirt_compute_lock", side_effect=claims.Busy("compute:1")):
|
||||
self.client.post(self.url, {"del_volume": "1", "volname": "disk1.qcow2"})
|
||||
|
|
|
|||
|
|
@ -55,6 +55,34 @@ class PoolPageTests(TestCase):
|
|||
self.assertEqual(doc.xpath("//div[@id=$d]//button[@data-bs-dismiss='modal'][not(@aria-label)]/text()",
|
||||
d=dialog), ["Cancel"])
|
||||
|
||||
def test_a_new_volume_is_made_from_the_pool_page(self):
|
||||
self.conn.get_size.return_value = (10, 4)
|
||||
self.conn.create_volume.return_value = "disk1.qcow2"
|
||||
url = reverse("storage", args=[self.compute.pk, "default"])
|
||||
response = self.client.post(url, {"name": "disk1", "size": "2", "format": "qcow2", "meta_prealloc": "on",
|
||||
"create_volume": ""})
|
||||
self.assertRedirects(response, url, fetch_redirect_response=False)
|
||||
self.conn.create_volume.assert_called_once_with("disk1", 2, "qcow2", True, 0, 0)
|
||||
|
||||
def test_a_refused_new_volume_comes_back_with_what_was_entered(self):
|
||||
self.conn.get_size.return_value = (10, 4)
|
||||
self.conn.get_status.return_value = 1
|
||||
response = self.client.post(reverse("storage", args=[self.compute.pk, "default"]),
|
||||
{"name": "bad name!", "size": "3", "format": "raw", "create_volume": ""})
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.conn.create_volume.assert_not_called()
|
||||
doc = html.fromstring(response.content)
|
||||
# the dialog opens again on the values sent, the name marked with why
|
||||
self.assertEqual(doc.xpath("//div[@id='AddImage']/@data-open"), [""])
|
||||
self.assertEqual(doc.xpath("//div[@id='AddImage']//input[@name='name']/@value"), ["bad name!"])
|
||||
self.assertEqual(doc.xpath("//div[@id='AddImage']//input[@name='size']/@value"), ["3"])
|
||||
self.assertTrue(doc.xpath("//div[@id='AddImage']//input[@name='name'][contains(@class, 'is-invalid')]"))
|
||||
# a pool stopped meanwhile shows no dialog: the page says why
|
||||
self.conn.is_active.return_value = False
|
||||
response = self.client.post(reverse("storage", args=[self.compute.pk, "default"]),
|
||||
{"name": "bad name!", "size": "3", "format": "raw", "create_volume": ""})
|
||||
self.assertContains(response, "The image name must not contain any special characters")
|
||||
|
||||
def test_the_pool_is_measured_after_its_refresh(self):
|
||||
# update_volumes refreshes the pool: libvirt's space figures are as of the last refresh
|
||||
self.conn.get_size.return_value = (10, 4)
|
||||
|
|
|
|||
|
|
@ -117,7 +117,6 @@ def storage(request, compute_id, pool):
|
|||
:return:
|
||||
"""
|
||||
compute = get_object_or_404(Compute, pk=compute_id)
|
||||
meta_prealloc = False
|
||||
form = CreateVolumeForm()
|
||||
|
||||
conn = wvmStorage(
|
||||
|
|
@ -178,6 +177,27 @@ def storage(request, compute_id, pool):
|
|||
)
|
||||
return redirect(reverse("storage", args=[compute.id, pool]))
|
||||
# return HttpResponseRedirect(request.get_full_path())
|
||||
if "create_volume" in request.POST:
|
||||
form = CreateVolumeForm(request.POST)
|
||||
if form.is_valid():
|
||||
data = form.cleaned_data
|
||||
with audit(request, compute, subject, f"add volume {data['name']}") as entry:
|
||||
name = conn.create_volume(
|
||||
data["name"],
|
||||
data["size"],
|
||||
data["format"],
|
||||
data["meta_prealloc"] and data["format"] == "qcow2",
|
||||
int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_UID),
|
||||
int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_GID),
|
||||
)
|
||||
entry.action = f"add volume {name}"
|
||||
messages.success(request, _("Image file %(name)s is created successfully") % {"name": name})
|
||||
return redirect(reverse("storage", args=[compute.id, pool]))
|
||||
# refused: the page shows the dialog again, with what was entered and why;
|
||||
# a pool stopped meanwhile has no dialog, so the reasons go to the page
|
||||
if not state:
|
||||
for msg_err in form.errors.values():
|
||||
messages.error(request, msg_err.as_text())
|
||||
if "cln_volume" in request.POST:
|
||||
form = CloneImage(request.POST)
|
||||
if form.is_valid():
|
||||
|
|
@ -199,7 +219,7 @@ def storage(request, compute_id, pool):
|
|||
messages.error(request, msg_err.as_text())
|
||||
return redirect(reverse("storage", args=[compute.id, pool]))
|
||||
|
||||
# read only for the page: every action above redirects
|
||||
# read only for the page: every action above redirects, but a refused new volume
|
||||
# first: it refreshes the pool, and libvirt measures a pool's space only then
|
||||
volumes = conn.update_volumes() if state else None
|
||||
for volume in volumes or []:
|
||||
|
|
@ -224,50 +244,6 @@ def storage(request, compute_id, pool):
|
|||
return render(request, "storage.html", locals())
|
||||
|
||||
|
||||
@superuser_only
|
||||
def create_volume(request, compute_id, pool):
|
||||
"""
|
||||
:param request:
|
||||
:param compute_id: compute id
|
||||
:param pool: pool name
|
||||
:return:
|
||||
"""
|
||||
compute = get_object_or_404(Compute, pk=compute_id)
|
||||
meta_prealloc = False
|
||||
|
||||
conn = wvmStorage(
|
||||
compute.hostname, compute.login, compute.password, compute.type, pool
|
||||
)
|
||||
|
||||
form = CreateVolumeForm(request.POST or None)
|
||||
if form.is_valid():
|
||||
data = form.cleaned_data
|
||||
if data["meta_prealloc"] and data["format"] == "qcow2":
|
||||
meta_prealloc = True
|
||||
|
||||
disk_owner_uid = int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_UID)
|
||||
disk_owner_gid = int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_GID)
|
||||
|
||||
with audit(request, compute, f"Storage pool {pool}", f"add volume {data['name']}") as entry:
|
||||
name = conn.create_volume(
|
||||
data["name"],
|
||||
data["size"],
|
||||
data["format"],
|
||||
meta_prealloc,
|
||||
disk_owner_uid,
|
||||
disk_owner_gid,
|
||||
)
|
||||
entry.action = f"add volume {name}"
|
||||
messages.success(
|
||||
request, _("Image file %(name)s is created successfully") % {"name": name}
|
||||
)
|
||||
else:
|
||||
for msg_err in form.errors.values():
|
||||
messages.error(request, msg_err.as_text())
|
||||
|
||||
return redirect(reverse("storage", args=[compute.id, pool]))
|
||||
|
||||
|
||||
def get_volumes(request, compute_id, pool):
|
||||
"""
|
||||
:param request:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue