launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07314
[Merge] lp:~rvb/maas/edit-mac-bug-968276 into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/edit-mac-bug-968276 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #968276 in MAAS: "No UI to edit MAC address for or add MAC address to existing node"
https://bugs.launchpad.net/maas/+bug/968276
For more details, see:
https://code.launchpad.net/~rvb/maas/edit-mac-bug-968276/+merge/103681
This branch adds the UI to add and delete mac addresses of an existing node.
= Pre-imp =
I discussed that with Gavin based on a mockup of the list of the mac addresses on the "node edit" page. We could have forced the user to keep at lease one mac address per node but since pserv can deal with that just fine now we decided it was a better idea to keep the UI as simple as possible.
= Notes =
I've used (and modified) the un-used MACAddressForm that was previously simply created from the MACAddress model but not used anywhere.
--
https://code.launchpad.net/~rvb/maas/edit-mac-bug-968276/+merge/103681
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/edit-mac-bug-968276 into lp:maas.
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-04-26 07:57:26 +0000
+++ src/maasserver/forms.py 2012-04-26 12:53:28 +0000
@@ -144,6 +144,15 @@
class Meta:
model = MACAddress
+ def __init__(self, node, *args, **kwargs):
+ super(MACAddressForm, self).__init__(*args, **kwargs)
+ self.node = node
+
+ def save(self, *args, **kwargs):
+ mac = super(MACAddressForm, self).save(commit=False)
+ mac.node = self.node
+ return mac.save(*args, **kwargs)
+
class SSHKeyForm(ModelForm):
key = forms.CharField(
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-04-25 10:31:44 +0000
+++ src/maasserver/models/__init__.py 2012-04-26 12:53:28 +0000
@@ -634,6 +634,12 @@
def __unicode__(self):
return self.mac_address
+ def unique_error_message(self, model_class, unique_check):
+ if unique_check == ('mac_address',):
+ return "This MAC Address is already registered."
+ return super(
+ MACAddress, self).unique_error_message(model_class, unique_check)
+
GENERIC_CONSUMER = 'MAAS consumer'
=== added file 'src/maasserver/templates/maasserver/mac_confirm_delete.html'
--- src/maasserver/templates/maasserver/mac_confirm_delete.html 1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/mac_confirm_delete.html 2012-04-26 12:53:28 +0000
@@ -0,0 +1,22 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Delete MAC Address: {{ mac_to_delete.mac_address }}{% endblock %}
+{% block page-title %}Delete MAC Address: {{ mac_to_delete.mac_address }}{% endblock %}
+
+{% block content %}
+ <div class="block auto-width">
+ <h2>
+ Are you sure you want to delete the MAC Address "{{ mac_to_delete.mac_address }}"?
+ </h2>
+ <p>This action is permanent and can not be undone.</p>
+ <p>
+ <form action="." method="post">
+ <input type="hidden" name="post" value="yes" />
+ <input type="submit" value="Delete MAC Address" class="right" />
+ <a href="{% url 'node-edit' mac_to_delete.node.system_id %}">Cancel</a>
+ </form>
+ </p>
+ </div>
+{% endblock %}
+
=== added file 'src/maasserver/templates/maasserver/node_add_mac.html'
--- src/maasserver/templates/maasserver/node_add_mac.html 1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/node_add_mac.html 2012-04-26 12:53:28 +0000
@@ -0,0 +1,18 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Add MAC Address{% endblock %}
+{% block page-title %}Add MAC Address{% endblock %}
+
+{% block content %}
+ <form action="." method="post" class="block auto-width">
+ <ul>
+ {% for field in form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ </ul>
+ <input type="submit" class="right" value="Add MAC Address" />
+ <a class="link-button"
+ href="{% url 'node-edit' node.system_id %}">Cancel</a>
+ </form>
+{% endblock %}
=== modified file 'src/maasserver/templates/maasserver/node_edit.html'
--- src/maasserver/templates/maasserver/node_edit.html 2012-04-02 13:43:04 +0000
+++ src/maasserver/templates/maasserver/node_edit.html 2012-04-26 12:53:28 +0000
@@ -7,9 +7,31 @@
{% block content %}
<form action="." method="post" class="block auto-width">
<ul>
- {% for field in form %}
- {% include "maasserver/form_field.html" %}
- {% endfor %}
+ {% for field in form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ <li>
+ <label for="id_mac_addresses">Mac addresses</label>
+ {% for macaddress in node.macaddress_set.all %}
+ <p>
+ {{ macaddress }}
+ <a title="Delete mac address"
+ class="icon"
+ href="{% url 'mac-delete' macaddress.node.system_id macaddress.mac_address %}">
+ <img src="/static/img/delete.png" alt="delete"/>
+ </a>
+ </p>
+ {% empty %}
+ No MAC Address.
+ {% endfor %}
+ </li>
+ <li>
+ <a class="add-link add-mac-form"
+ href="{% url 'mac-add' node.system_id %}">
+ <img src="/static/img/inline_add.png" alt="+" class="icon"/>
+ Add additional MAC address
+ </a>
+ </li>
</ul>
<input type="submit" value="Save node" class="right" />
<a class="link-button" href="{% url 'node-view' node.system_id %}">Cancel</a>
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py 2012-04-20 15:16:41 +0000
+++ src/maasserver/testing/factory.py 2012-04-26 12:53:28 +0000
@@ -90,10 +90,12 @@
ncr.save()
return ncr
- def make_mac_address(self, address):
+ def make_mac_address(self, address=None, node=None):
"""Create a MAC address."""
- node = Node()
- node.save()
+ if node is None:
+ node = self.make_node()
+ if address is None:
+ address = self.getRandomMACAddress()
mac = MACAddress(mac_address=address, node=node)
mac.save()
return mac
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-04-26 07:57:26 +0000
+++ src/maasserver/tests/test_forms.py 2012-04-26 12:53:28 +0000
@@ -38,6 +38,7 @@
get_action_form,
HostnameFormField,
inhibit_delete,
+ MACAddressForm,
NewUserCreationForm,
NODE_ACTIONS,
NodeActionForm,
@@ -50,6 +51,7 @@
from maasserver.models import (
Config,
DEFAULT_CONFIG,
+ MACAddress,
)
from maasserver.provisioning import get_provisioning_api_proxy
from maasserver.testing.factory import factory
@@ -571,3 +573,26 @@
['username', 'last_name', 'email', 'password1', 'password2',
'is_superuser'],
list(form.fields))
+
+
+class TestMACAddressForm(TestCase):
+
+ def test_MACAddressForm_creates_mac_address(self):
+ node = factory.make_node()
+ mac = factory.getRandomMACAddress()
+ form = MACAddressForm(node=node, data={'mac_address': mac})
+ form.save()
+ self.assertTrue(
+ MACAddress.objects.filter(node=node, mac_address=mac).exists())
+
+ def test_MACAddressForm_displays_error_message_if_mac_already_used(self):
+ mac = factory.getRandomMACAddress()
+ node = factory.make_mac_address(address=mac)
+ node = factory.make_node()
+ form = MACAddressForm(node=node, data={'mac_address': mac})
+ self.assertFalse(form.is_valid())
+ self.assertEquals(
+ {'mac_address': ['This MAC Address is already registered.']},
+ form._errors)
+ self.assertFalse(
+ MACAddress.objects.filter(node=node, mac_address=mac).exists())
=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py 2012-04-26 07:57:26 +0000
+++ src/maasserver/tests/test_views_nodes.py 2012-04-26 12:53:28 +0000
@@ -26,7 +26,10 @@
)
from maasserver.exceptions import NoRabbit
from maasserver.forms import NodeActionForm
-from maasserver.models import Node
+from maasserver.models import (
+ MACAddress,
+ Node,
+ )
from maasserver.testing import (
get_content_links,
reload_object,
@@ -43,6 +46,10 @@
from maasserver.views.nodes import get_longpoll_context
from maastesting.rabbit import uses_rabbit_fixture
from provisioningserver.enum import POWER_TYPE_CHOICES
+from testtools.matchers import (
+ Contains,
+ MatchesAll,
+ )
class NodeViewsTest(LoggedInTestCase):
@@ -206,6 +213,39 @@
self.assertEqual(httplib.FOUND, response.status_code)
self.assertAttributes(node, params)
+ def test_edit_nodes_contains_list_of_macaddresses(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ macs = [
+ factory.make_mac_address(node=node).mac_address
+ for i in range(3)
+ ]
+ node_edit_link = reverse('node-edit', args=[node.system_id])
+ response = self.client.get(node_edit_link)
+ self.assertThat(
+ response.content,
+ MatchesAll(*[Contains(mac) for mac in macs]))
+
+ def test_edit_nodes_contains_links_to_delete_the_macaddresses(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ macs = [
+ factory.make_mac_address(node=node).mac_address
+ for i in range(3)
+ ]
+ node_edit_link = reverse('node-edit', args=[node.system_id])
+ response = self.client.get(node_edit_link)
+ self.assertThat(
+ response.content,
+ MatchesAll(
+ *[Contains(
+ reverse('mac-delete', args=[node, mac]))
+ for mac in macs]))
+
+ def test_edit_nodes_contains_link_to_add_a_macaddresses(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ node_edit_link = reverse('node-edit', args=[node.system_id])
+ response = self.client.get(node_edit_link)
+ self.assertIn(reverse('mac-add', args=[node]), response.content)
+
def test_view_node_has_button_to_accept_enlistement_for_user(self):
# A simple user can't see the button to enlist a declared node.
node = factory.make_node(status=NODE_STATUS.DECLARED)
@@ -340,6 +380,100 @@
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
+class NodeDeleteMacTest(LoggedInTestCase):
+
+ def test_node_delete_not_found_if_node_does_not_exist(self):
+ # This returns a 404 rather than returning to the node page
+ # with a nice error message because the node could not be found.
+ node_id = factory.getRandomString()
+ mac = factory.getRandomMACAddress()
+ mac_delete_link = reverse('mac-delete', args=[node_id, mac])
+ response = self.client.get(mac_delete_link)
+ self.assertEqual(httplib.NOT_FOUND, response.status_code)
+
+ def test_node_delete_redirects_if_mac_does_not_exist(self):
+ # If the MAC Address does not exist, the user is redirected
+ # to the node edit page.
+ node = factory.make_node(owner=self.logged_in_user)
+ mac = factory.getRandomMACAddress()
+ mac_delete_link = reverse('mac-delete', args=[node.system_id, mac])
+ response = self.client.get(mac_delete_link)
+ next_url = reverse('node-edit', args=[node.system_id])
+ self.assertEqual(
+ (httplib.FOUND, next_url),
+ (response.status_code, urlparse(response['Location']).path))
+
+ def test_node_delete_access_denied_if_user_cannot_edit_node(self):
+ node = factory.make_node(owner=factory.make_user())
+ mac = factory.make_mac_address(node=node)
+ mac_delete_link = reverse('mac-delete', args=[node.system_id, mac])
+ response = self.client.get(mac_delete_link)
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+ def test_node_delete_mac_contains_mac(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac = factory.make_mac_address(node=node)
+ mac_delete_link = reverse('mac-delete', args=[node.system_id, mac])
+ response = self.client.get(mac_delete_link)
+ self.assertIn(
+ 'Are you sure you want to delete the MAC Address "%s"' %
+ mac.mac_address,
+ response.content)
+
+ def test_node_delete_mac_POST_deletes_mac(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac = factory.make_mac_address(node=node)
+ mac_delete_link = reverse('mac-delete', args=[node.system_id, mac])
+ response = self.client.post(mac_delete_link, {'post': 'yes'})
+ next_url = reverse('node-edit', args=[node.system_id])
+ self.assertEqual(
+ (httplib.FOUND, next_url),
+ (response.status_code, urlparse(response['Location']).path))
+ self.assertFalse(MACAddress.objects.filter(id=mac.id).exists())
+
+ def test_node_delete_mac_POST_displays_message(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac = factory.make_mac_address(node=node)
+ mac_delete_link = reverse('mac-delete', args=[node.system_id, mac])
+ response = self.client.post(mac_delete_link, {'post': 'yes'})
+ response = self.client.get(urlparse(response['Location']).path)
+ self.assertEqual(
+ ["Mac address %s deleted." % mac.mac_address],
+ [message.message for message in response.context['messages']])
+
+
+class NodeAddeMacTest(LoggedInTestCase):
+
+ def test_node_add_mac_contains_form(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac_add_link = reverse('mac-add', args=[node.system_id])
+ response = self.client.get(mac_add_link)
+ doc = fromstring(response.content)
+ self.assertEqual(1, len(doc.cssselect('form input#id_mac_address')))
+
+ def test_node_add_mac_POST_adds_mac(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac_add_link = reverse('mac-add', args=[node.system_id])
+ mac = factory.getRandomMACAddress()
+ response = self.client.post(mac_add_link, {'mac_address': mac})
+ next_url = reverse('node-edit', args=[node.system_id])
+ self.assertEqual(
+ (httplib.FOUND, next_url),
+ (response.status_code, urlparse(response['Location']).path))
+ self.assertTrue(
+ MACAddress.objects.filter(node=node, mac_address=mac).exists())
+
+ def test_node_add_mac_POST_displays_message(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ mac_add_link = reverse('mac-add', args=[node.system_id])
+ mac = factory.getRandomMACAddress()
+ response = self.client.post(mac_add_link, {'mac_address': mac})
+ response = self.client.get(urlparse(response['Location']).path)
+ self.assertEqual(
+ ["MAC Address added."],
+ [message.message for message in response.context['messages']])
+
+
class AdminNodeViewsTest(AdminLoggedInTestCase):
def test_admin_can_edit_nodes(self):
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-04-23 14:02:51 +0000
+++ src/maasserver/urls.py 2012-04-26 12:53:28 +0000
@@ -33,6 +33,8 @@
)
from maasserver.views.combo import combo_view
from maasserver.views.nodes import (
+ MacAdd,
+ MacDelete,
NodeDelete,
NodeEdit,
NodeListView,
@@ -106,6 +108,12 @@
url(
r'^nodes/(?P<system_id>[\w\-]+)/delete/$', NodeDelete.as_view(),
name='node-delete'),
+ url(
+ r'^nodes/(?P<system_id>[\w\-]+)/macs/(?P<mac_address>.+)/delete/$',
+ MacDelete.as_view(), name='mac-delete'),
+ url(
+ r'^nodes/(?P<system_id>[\w\-]+)/macs/add/$',
+ MacAdd.as_view(), name='mac-add'),
)
=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py 2012-04-26 07:44:30 +0000
+++ src/maasserver/views/nodes.py 2012-04-26 12:53:28 +0000
@@ -11,6 +11,8 @@
__metaclass__ = type
__all__ = [
+ 'MacAdd',
+ 'MacDelete',
'NodeListView',
'NodeView',
'NodeEdit',
@@ -22,8 +24,10 @@
from django.contrib import messages
from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import reverse
+from django.shortcuts import get_object_or_404
from django.utils.safestring import mark_safe
from django.views.generic import (
+ CreateView,
ListView,
UpdateView,
)
@@ -37,11 +41,15 @@
)
from maasserver.forms import (
get_action_form,
+ MACAddressForm,
UIAdminNodeEditForm,
UINodeEditForm,
)
from maasserver.messages import messaging
-from maasserver.models import Node
+from maasserver.models import (
+ MACAddress,
+ Node,
+ )
from maasserver.views import HelpfulDeleteView
@@ -173,3 +181,62 @@
def name_object(self, obj):
"""See `HelpfulDeleteView`."""
return "Node %s" % obj.system_id
+
+
+class MacAdd(CreateView):
+ form_class = MACAddressForm
+ template_name = 'maasserver/node_add_mac.html'
+
+ def get_node(self):
+ system_id = self.kwargs.get('system_id', None)
+ node = Node.objects.get_node_or_404(
+ system_id=system_id, user=self.request.user,
+ perm=NODE_PERMISSION.EDIT)
+ return node
+
+ def get_form_kwargs(self):
+ kwargs = super(MacAdd, self).get_form_kwargs()
+ kwargs['node'] = self.get_node()
+ return kwargs
+
+ def form_valid(self, form):
+ res = super(MacAdd, self).form_valid(form)
+ messages.info(self.request, "MAC Address added.")
+ return res
+
+ def get_success_url(self):
+ node = self.get_node()
+ return reverse('node-edit', args=[node.system_id])
+
+ def get_context_data(self, **kwargs):
+ context = super(MacAdd, self).get_context_data(**kwargs)
+ context.update({'node': self.get_node()})
+ return context
+
+
+class MacDelete(HelpfulDeleteView):
+
+ template_name = 'maasserver/mac_confirm_delete.html'
+ context_object_name = 'mac_to_delete'
+ model = MACAddress
+
+ def get_node(self):
+ system_id = self.kwargs.get('system_id', None)
+ node = Node.objects.get_node_or_404(
+ system_id=system_id, user=self.request.user,
+ perm=NODE_PERMISSION.EDIT)
+ return node
+
+ def get_object(self):
+ node = self.get_node()
+ mac_address = self.kwargs.get('mac_address', None)
+ return get_object_or_404(
+ MACAddress, node=node, mac_address=mac_address)
+
+ def get_next_url(self):
+ node = self.get_node()
+ return reverse('node-edit', args=[node.system_id])
+
+ def name_object(self, obj):
+ """See `HelpfulDeleteView`."""
+ return "MAC Address %s" % obj.mac_address
