launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06831
[Merge] lp:~rvb/maas/maas-edit-node into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-edit-node into lp:maas with lp:~rvb/maas/maas-view-node as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-edit-node/+merge/98709
Add "Edit node" page.
--
https://code.launchpad.net/~rvb/maas/maas-edit-node/+merge/98709
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-edit-node into lp:maas.
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-03-21 07:30:59 +0000
+++ src/maasserver/forms.py 2012-03-21 19:12:21 +0000
@@ -16,6 +16,8 @@
"MACAddressForm",
"MAASAndNetworkForm",
"UbuntuForm",
+ "UIAdminNodeEditForm",
+ "UINodeEditForm",
]
from django import forms
@@ -40,6 +42,7 @@
Node,
NODE_AFTER_COMMISSIONING_ACTION,
NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
+ UserProfile,
)
@@ -83,6 +86,26 @@
'architecture', 'power_type')
+class UINodeEditForm(ModelForm):
+ after_commissioning_action = forms.ChoiceField(
+ choices=NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
+
+ class Meta:
+ model = Node
+ fields = ('hostname', 'after_commissioning_action')
+
+
+class UIAdminNodeEditForm(ModelForm):
+ after_commissioning_action = forms.ChoiceField(
+ choices=NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
+ owner = forms.ModelChoiceField(queryset=UserProfile.objects.all_users())
+
+ class Meta:
+ model = Node
+ fields = (
+ 'hostname', 'after_commissioning_action', 'power_type', 'owner')
+
+
class MACAddressForm(ModelForm):
class Meta:
model = MACAddress
=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py 2012-03-21 14:20:54 +0000
+++ src/maasserver/models.py 2012-03-21 19:12:21 +0000
@@ -356,7 +356,7 @@
default=NODE_STATUS.DEFAULT_STATUS)
owner = models.ForeignKey(
- User, default=None, blank=True, null=True, editable=False)
+ User, default=None, blank=True, null=True, editable=True)
after_commissioning_action = models.IntegerField(
choices=NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
@@ -912,13 +912,14 @@
raise NotImplementedError(
'Invalid permission check (invalid object type).')
- # Only the generic 'access' permission is supported.
- if perm != 'access':
+ if perm == 'access':
+ return obj.owner in (None, user)
+ elif perm == 'edit':
+ return obj.owner == user
+ else:
raise NotImplementedError(
'Invalid permission check (invalid permission name).')
- return obj.owner in (None, user)
-
# 'provisioning' is imported so that it can register its signal handlers early
# on, before it misses anything.
=== added file 'src/maasserver/templates/maasserver/node_edit.html'
--- src/maasserver/templates/maasserver/node_edit.html 1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/node_edit.html 2012-03-21 19:12:21 +0000
@@ -0,0 +1,17 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Edit node{% endblock %}
+{% block page-title %}Edit node{% 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" value="Save node" class="right" />
+ <a class="link-button" href="{% url 'node-view' node.id %}">Cancel</a>
+ </form>
+{% endblock %}
=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html 2012-03-20 14:03:49 +0000
+++ src/maasserver/templates/maasserver/node_view.html 2012-03-21 19:12:21 +0000
@@ -7,6 +7,12 @@
{% block sidebar %}
<div class="block size3">
+ {% if can_edit %}
+ <h4>Node details</h4>
+ <a href="{% url 'node-edit' node.id %}" class="button secondary">
+ Edit node
+ </a>
+ {% endif%}
</div>
{% endblock %}
=== modified file 'src/maasserver/tests/test_auth.py'
--- src/maasserver/tests/test_auth.py 2012-03-15 13:58:32 +0000
+++ src/maasserver/tests/test_auth.py 2012-03-21 19:12:21 +0000
@@ -108,6 +108,22 @@
node = make_allocated_node()
self.assertTrue(backend.has_perm(node.owner, 'access', node))
+ def test_user_cannot_edit_nodes_owned_by_others(self):
+ backend = MAASAuthorizationBackend()
+ self.assertFalse(backend.has_perm(
+ factory.make_user(), 'edit', make_allocated_node()))
+
+ def test_user_cannot_access_unowned_node(self):
+ backend = MAASAuthorizationBackend()
+ self.assertFalse(backend.has_perm(
+ factory.make_user(), 'edit', make_unallocated_node()))
+
+ def test_user_can_edit_his_own_nodes(self):
+ backend = MAASAuthorizationBackend()
+ user = factory.make_user()
+ self.assertTrue(backend.has_perm(
+ user, 'edit', make_allocated_node(owner=user)))
+
class TestNodeVisibility(TestCase):
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-03-20 16:32:45 +0000
+++ src/maasserver/tests/test_forms.py 2012-03-21 19:12:21 +0000
@@ -11,6 +11,8 @@
__metaclass__ = type
__all__ = []
+import random
+
from django import forms
from django.core.exceptions import ValidationError
from django.http import QueryDict
@@ -21,12 +23,16 @@
NewUserCreationForm,
NodeWithMACAddressesForm,
ProfileForm,
+ UIAdminNodeEditForm,
+ UINodeEditForm,
validate_hostname,
)
from maasserver.models import (
ARCHITECTURE,
Config,
DEFAULT_CONFIG,
+ NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
+ POWER_TYPE_CHOICES,
)
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
@@ -160,6 +166,70 @@
hostname = HostnameFormField()
+class NodeEditForms(TestCase):
+
+ def test_UINodeEditForm_contains_limited_set_of_fields(self):
+ form = UINodeEditForm()
+
+ self.assertEqual(
+ ['hostname', 'after_commissioning_action'], list(form.fields))
+
+ def test_UINodeEditForm_changes_node(self):
+ node = factory.make_node()
+ hostname = factory.getRandomString()
+ after_commissioning_action = factory.getRandomChoice(
+ NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
+
+ form = UINodeEditForm(
+ data={
+ 'hostname': hostname,
+ 'after_commissioning_action': after_commissioning_action,
+ },
+ instance=node)
+ form.save()
+
+ self.assertEqual(hostname, node.hostname)
+ self.assertEqual(
+ after_commissioning_action, node.after_commissioning_action)
+
+ def test_UIAdminNodeEditForm_contains_limited_set_of_fields(self):
+ form = UIAdminNodeEditForm()
+
+ self.assertEqual(
+ ['hostname', 'after_commissioning_action', 'power_type', 'owner'],
+ list(form.fields))
+
+ def test_UIAdminNodeEditForm_changes_node(self):
+ node = factory.make_node()
+ hostname = factory.getRandomString()
+ after_commissioning_action = factory.getRandomChoice(
+ NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
+ power_type = factory.getRandomChoice(POWER_TYPE_CHOICES)
+ owner = random.choice([factory.make_user() for i in range(10)])
+ form = UIAdminNodeEditForm(
+ data={
+ 'hostname': hostname,
+ 'after_commissioning_action': after_commissioning_action,
+ 'power_type': power_type,
+ 'owner': owner.id,
+ },
+ instance=node)
+ form.save()
+
+ self.assertEqual(hostname, node.hostname)
+ self.assertEqual(
+ after_commissioning_action, node.after_commissioning_action)
+ self.assertEqual(power_type, node.power_type)
+ self.assertEqual(owner, node.owner)
+
+ def test_UIAdminNodeEditForm_owner_choices_contains_active_users(self):
+ form = UIAdminNodeEditForm()
+ user = factory.make_user()
+ user_ids = [choice[0] for choice in form.fields['owner'].choices]
+
+ self.assertItemsEqual(['', user.id], user_ids)
+
+
class TestHostnameFormField(TestCase):
def test_validate_hostname_validates_valid_hostnames(self):
=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py 2012-03-20 15:36:06 +0000
+++ src/maasserver/tests/test_views.py 2012-03-21 19:12:21 +0000
@@ -14,6 +14,7 @@
from collections import namedtuple
import httplib
import os
+import random
import urllib2
from django.conf import settings
@@ -26,9 +27,11 @@
from maasserver.models import (
Config,
NODE_AFTER_COMMISSIONING_ACTION,
+ POWER_TYPE_CHOICES,
SSHKeys,
UserProfile,
)
+from maasserver.testing import reload_object
from maasserver.testing.factory import factory
from maasserver.testing.testcase import (
LoggedInTestCase,
@@ -357,16 +360,34 @@
self.assertNotEqual(old_pw, user.password)
-class NodeViewTest(LoggedInTestCase):
+class AdminLoggedInTestCase(LoggedInTestCase):
+
+ def setUp(self):
+ super(AdminLoggedInTestCase, self).setUp()
+ # Promote the logged-in user to admin.
+ self.logged_in_user.is_superuser = True
+ self.logged_in_user.save()
+
+
+class NodeViewsTest(LoggedInTestCase):
+
+ def assertContainsLink(self, response, link, reverse=False):
+ doc = fromstring(response.content)
+ content_node = doc.cssselect('#content')[0]
+ all_links = [elem.get('href') for elem in content_node.cssselect('a')]
+ if not reverse:
+ self.assertIn(link, all_links)
+ else:
+ self.assertNotIn(link, all_links)
+
+ def assertDoesNotContainLink(self, response, link):
+ return self.assertContainsLink(response, link, reverse=True)
def test_node_list_contains_link_to_node_view(self):
node = factory.make_node()
response = self.client.get(reverse('node-list'))
- doc = fromstring(response.content)
- content_node = doc.cssselect('#content')[0]
- all_links = [elem.get('href') for elem in content_node.cssselect('a')]
node_link = reverse('node-view', args=[node.id])
- self.assertIn(node_link, all_links)
+ self.assertContainsLink(response, node_link)
def test_view_node_displays_node_info(self):
# The node page features the basic information about the node.
@@ -378,14 +399,78 @@
self.assertIn(node.hostname, content_text)
self.assertIn(node.display_status(), content_text)
-
-class AdminLoggedInTestCase(LoggedInTestCase):
-
- def setUp(self):
- super(AdminLoggedInTestCase, self).setUp()
- # Promote the logged-in user to admin.
- self.logged_in_user.is_superuser = True
- self.logged_in_user.save()
+ def test_view_node_displays_link_to_edit_if_user_owns_node(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ node_link = reverse('node-view', args=[node.id])
+ response = self.client.get(node_link)
+ node_edit_link = reverse('node-edit', args=[node.id])
+ self.assertContainsLink(response, node_edit_link)
+
+ def test_view_node_no_link_to_edit_someonelses_node(self):
+ node = factory.make_node(owner=factory.make_user())
+ node_link = reverse('node-view', args=[node.id])
+ response = self.client.get(node_link)
+ node_edit_link = reverse('node-edit', args=[node.id])
+ self.assertDoesNotContainLink(response, node_edit_link)
+
+ def test_user_cannot_edit_someonelses_node(self):
+ node = factory.make_node(owner=factory.make_user())
+ node_edit_link = reverse('node-edit', args=[node.id])
+ response = self.client.get(node_edit_link)
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+ def test_user_can_access_the_edition_page_for_his_nodes(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ node_edit_link = reverse('node-edit', args=[node.id])
+ response = self.client.get(node_edit_link)
+ self.assertEqual(httplib.OK, response.status_code)
+
+ def test_user_can_edit_his_nodes(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ node_edit_link = reverse('node-edit', args=[node.id])
+ hostname = factory.getRandomString()
+ after_commissioning_action = factory.getRandomEnum(
+ NODE_AFTER_COMMISSIONING_ACTION)
+ response = self.client.post(
+ node_edit_link,
+ data={
+ 'hostname': hostname,
+ 'after_commissioning_action': after_commissioning_action
+ })
+
+ node = reload_object(node)
+ self.assertEqual(httplib.FOUND, response.status_code)
+ self.assertEqual(hostname, node.hostname)
+ self.assertEqual(
+ after_commissioning_action, node.after_commissioning_action)
+
+
+class AdminNodeViewsTest(AdminLoggedInTestCase):
+
+ def test_admin_can_edit_nodes(self):
+ node = factory.make_node(owner=factory.make_user())
+ node_edit_link = reverse('node-edit', args=[node.id])
+ hostname = factory.getRandomString()
+ power_type = factory.getRandomChoice(POWER_TYPE_CHOICES)
+ after_commissioning_action = factory.getRandomEnum(
+ NODE_AFTER_COMMISSIONING_ACTION)
+ owner = random.choice([factory.make_user() for i in range(5)])
+ response = self.client.post(
+ node_edit_link,
+ data={
+ 'hostname': hostname,
+ 'after_commissioning_action': after_commissioning_action,
+ 'power_type': power_type,
+ 'owner': owner.id,
+ })
+
+ node = reload_object(node)
+ self.assertEqual(httplib.FOUND, response.status_code)
+ self.assertEqual(hostname, node.hostname)
+ self.assertEqual(owner, node.owner)
+ self.assertEqual(power_type, node.power_type)
+ self.assertEqual(
+ after_commissioning_action, node.after_commissioning_action)
class SettingsTest(AdminLoggedInTestCase):
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-03-21 16:40:50 +0000
+++ src/maasserver/urls.py 2012-03-21 19:12:21 +0000
@@ -36,6 +36,7 @@
login,
logout,
NodeListView,
+ NodeEdit,
NodesCreateView,
NodeView,
proxy_to_longpoll,
@@ -76,6 +77,7 @@
name='index'),
url(r'^nodes/$', NodeListView.as_view(model=Node), name='node-list'),
url(r'^nodes/(?P<id>\d*)/view/$', NodeView.as_view(), name='node-view'),
+ url(r'^nodes/(?P<id>\d*)/edit/$', NodeEdit.as_view(), name='node-edit'),
url(
r'^nodes/create/$', NodesCreateView.as_view(), name='node-create'),
)
=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py 2012-03-20 14:00:39 +0000
+++ src/maasserver/views.py 2012-03-21 19:12:21 +0000
@@ -25,6 +25,7 @@
parse_qs,
)
from django.conf import settings as django_settings
+from django.core.exceptions import PermissionDenied
from django.contrib import messages
from django.contrib.auth.forms import PasswordChangeForm as PasswordForm
from django.contrib.auth.models import User
@@ -61,6 +62,8 @@
NewUserCreationForm,
ProfileForm,
UbuntuForm,
+ UINodeEditForm,
+ UIAdminNodeEditForm,
)
from maasserver.messages import messaging
from maasserver.models import (
@@ -96,6 +99,35 @@
id = self.kwargs.get('id', None)
return get_object_or_404(Node, id=id)
+ def get_context_data(self, **kwargs):
+ context = super(NodeView, self).get_context_data(**kwargs)
+ node = self.get_object()
+ context.update(
+ {'can_edit': self.request.user.has_perm('edit', node)})
+ return context
+
+
+class NodeEdit(UpdateView):
+
+ template_name = 'maasserver/node_edit.html'
+
+ def get_object(self):
+ id = self.kwargs.get('id', None)
+ node = get_object_or_404(Node, id=id)
+ if self.request.user.has_perm('edit', node):
+ return node
+ else:
+ raise PermissionDenied
+
+ def get_form_class(self):
+ if self.request.user.is_superuser:
+ return UIAdminNodeEditForm
+ else:
+ return UINodeEditForm
+
+ def get_success_url(self):
+ return reverse('node-view', args=[self.get_object().id])
+
def get_longpoll_context():
if messaging is not None and django_settings.LONGPOLL_PATH is not None:
Follow ups
