launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-admin-approve-nodes-ui into lp:maas with lp:~rvb/maas/maas-admin-approve-nodes as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-admin-approve-nodes-ui/+merge/100176

This branch adds the UI to enlist a node.  This only adds the button that is on a "node view" page but it is done in a very generic fashion that will allow us to add other actions easily.

The necessity of the method get_transition_form that simply creates a subclass of NodeTransitionForm with a proper user set might not be apparent if you don't know about Django generic classes (https://docs.djangoproject.com/en/dev/topics/class-based-views/).  This is all to be able to provide a form class in NodeView.get_form_class that has the right shape be exploited by Django's machinery.
-- 
https://code.launchpad.net/~rvb/maas/maas-admin-approve-nodes-ui/+merge/100176
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-admin-approve-nodes-ui into lp:maas.

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-03-29 11:40:39 +0000
+++ src/maasserver/forms.py	2012-03-30 16:19:33 +0000
@@ -11,10 +11,12 @@
 __metaclass__ = type
 __all__ = [
     "CommissioningForm",
+    "get_transition_form",
     "HostnameFormField",
     "NodeForm",
     "MACAddressForm",
     "MAASAndNetworkForm",
+    "NodeTransitionForm",
     "SSHKeyForm",
     "UbuntuForm",
     "UIAdminNodeEditForm",
@@ -26,8 +28,14 @@
     UserChangeForm,
     UserCreationForm,
     )
-from django.contrib.auth.models import User
-from django.core.exceptions import ValidationError
+from django.contrib.auth.models import (
+    AnonymousUser,
+    User,
+    )
+from django.core.exceptions import (
+    PermissionDenied,
+    ValidationError,
+    )
 from django.core.validators import URLValidator
 from django.forms import (
     CharField,
@@ -184,6 +192,55 @@
         return node
 
 
+class NodeTransitionForm(forms.Form):
+    """A form used to perform a status change on a Node.
+
+    That form class should not be used directly but through subclasses
+    created using `get_transition_form`.
+    """
+
+    user = AnonymousUser()
+
+    # The name of the input button used with this form.
+    input_name = 'node_transition'
+
+    def __init__(self, instance, *args, **kwargs):
+        super(NodeTransitionForm, self).__init__(*args, **kwargs)
+        self.node = instance
+        self.transitions = self.node.available_transition_methods(
+            self.user)
+        self.transition_dict = dict(
+            [(transition['name'],
+             (transition['method'], transition['permission']))
+            for transition in self.transitions])
+
+    def save(self):
+        transition_name = self.data.get(self.input_name)
+        method_name, permission = self.transition_dict.get(
+            transition_name, (None, None))
+        if method_name is not None:
+            if not self.user.has_perm(permission, self.node):
+                raise PermissionDenied()
+            method = getattr(self.node, method_name)
+            method()
+        else:
+            raise PermissionDenied()
+
+
+def get_transition_form(user):
+    """Return a class derived from NodeTransitionForm for a specific user.
+
+    :param user: The user for which to build a form derived from
+        NodeTransitionForm.
+    :type user: :class:`django.contrib.auth.models.User`
+    :return: A form class derived from NodeTransitionForm.
+    :rtype: class:`django.forms.Form`
+    """
+    return type(
+        str("SpecificNodeTransitionForm"), (NodeTransitionForm,),
+        {'user': user})
+
+
 class ProfileForm(ModelForm):
     # We use the field 'last_name' to store the user's full name (and
     # don't display Django's 'first_name' field).

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-03-21 18:53:44 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-03-30 16:19:33 +0000
@@ -12,7 +12,19 @@
       <a href="{% url 'node-edit' node.id %}" class="button secondary">
         Edit node
       </a>
-    {% endif%}
+    {% endif %}
+    {% if form.transitions %}
+      <form id="node_actions" method="post" action=".">
+      {% for transition in form.transitions %}
+        {% if forloop.first %}<br /><br /><h4>Actions</h4>{% endif %}
+        <input class="secondary"
+               type="submit"
+               name={{ form.input_name }}
+               value="{{ transition.name }}" />
+        {% if not forloop.last %}<br /><br />{% endif %}
+      {% endfor %}
+      </form>
+    {% endif %}
   </div>
 {% endblock %}
 

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2012-03-22 17:34:36 +0000
+++ src/maasserver/tests/test_forms.py	2012-03-30 16:19:33 +0000
@@ -14,13 +14,18 @@
 import random
 
 from django import forms
-from django.core.exceptions import ValidationError
+from django.core.exceptions import (
+    PermissionDenied,
+    ValidationError,
+    )
 from django.http import QueryDict
 from maasserver.forms import (
     ConfigForm,
     EditUserForm,
+    get_transition_form,
     HostnameFormField,
     NewUserCreationForm,
+    NodeTransitionForm,
     NodeWithMACAddressesForm,
     ProfileForm,
     UIAdminNodeEditForm,
@@ -32,6 +37,7 @@
     Config,
     DEFAULT_CONFIG,
     NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
+    NODE_STATUS,
     POWER_TYPE_CHOICES,
     )
 from maasserver.testing.factory import factory
@@ -251,6 +257,66 @@
         self.assertItemsEqual(['', user.id], user_ids)
 
 
+class TestNodeTransitionForm(TestCase):
+
+    def test_get_transition_form_creates_form_class_with_attributes(self):
+        user = factory.make_admin()
+        form_class = get_transition_form(user)
+
+        self.assertEqual(user, form_class.user)
+
+    def test_get_transition_form_creates_form_instance(self):
+        user = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(user)(node)
+
+        self.assertIsInstance(form, NodeTransitionForm)
+        self.assertEqual(node, form.node)
+
+    def test_get_transition_form_for_admin(self):
+        admin = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(admin)(node)
+
+        self.assertItemsEqual(
+            {"Enlist node": ('accept_enlistment', 'admin')},
+            form.transition_dict)
+
+    def test_get_transition_form_for_user(self):
+        user = factory.make_user()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(user)(node)
+
+        self.assertIsInstance(form, NodeTransitionForm)
+        self.assertEqual(node, form.node)
+        self.assertItemsEqual({}, form.transition_dict)
+
+    def test_get_transition_form_node_for_admin_save(self):
+        admin = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(admin)(
+            node, {NodeTransitionForm.input_name: "Enlist node"})
+        form.save()
+
+        self.assertEqual(NODE_STATUS.READY, node.status)
+
+    def test_get_transition_form_for_user_save(self):
+        user = factory.make_user()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(user)(
+            node, {NodeTransitionForm.input_name: "Enlist node"})
+
+        self.assertRaises(PermissionDenied, form.save)
+
+    def test_get_transition_form_for_user_save_unknown_trans(self):
+        user = factory.make_user()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        form = get_transition_form(user)(
+            node, {NodeTransitionForm.input_name: factory.getRandomString()})
+
+        self.assertRaises(PermissionDenied, form.save)
+
+
 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-29 11:40:39 +0000
+++ src/maasserver/tests/test_views.py	2012-03-30 16:19:33 +0000
@@ -27,9 +27,11 @@
     views,
     )
 from maasserver.exceptions import NoRabbit
+from maasserver.forms import NodeTransitionForm
 from maasserver.models import (
     Config,
     NODE_AFTER_COMMISSIONING_ACTION,
+    NODE_STATUS,
     POWER_TYPE_CHOICES,
     SSHKey,
     UserProfile,
@@ -528,6 +530,44 @@
         self.assertEqual(
             after_commissioning_action, node.after_commissioning_action)
 
+    def test_view_node_admin_has_button_to_accept_enlistement(self):
+        self.logged_in_user.is_superuser = True
+        self.logged_in_user.save()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        node_link = reverse('node-view', args=[node.id])
+        response = self.client.get(node_link)
+        doc = fromstring(response.content)
+        inputs = [
+            input for input in doc.cssselect('form#node_actions input')
+            if input.name == NodeTransitionForm.input_name]
+
+        self.assertSequenceEqual(
+            ['Enlist node'], [input.value for input in inputs])
+
+    def test_view_node_POST_admin_can_enlist_node(self):
+        self.logged_in_user.is_superuser = True
+        self.logged_in_user.save()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        node_link = reverse('node-view', args=[node.id])
+        response = self.client.post(
+            node_link,
+            data={
+                NodeTransitionForm.input_name: 'Enlist node',
+            })
+
+        self.assertEqual(httplib.FOUND, response.status_code)
+        self.assertEqual(
+            NODE_STATUS.READY, reload_object(node).status)
+
+    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)
+        node_link = reverse('node-view', args=[node.id])
+        response = self.client.get(node_link)
+        doc = fromstring(response.content)
+
+        self.assertEqual(0, len(doc.cssselect('form#node_actions input')))
+
 
 class AdminNodeViewsTest(AdminLoggedInTestCase):
 

=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py	2012-03-28 16:13:04 +0000
+++ src/maasserver/views.py	2012-03-30 16:19:33 +0000
@@ -72,6 +72,7 @@
     AddArchiveForm,
     CommissioningForm,
     EditUserForm,
+    get_transition_form,
     MAASAndNetworkForm,
     NewUserCreationForm,
     ProfileForm,
@@ -101,7 +102,7 @@
     return dj_logout(request, next_page=reverse('login'))
 
 
-class NodeView(DetailView):
+class NodeView(UpdateView):
 
     template_name = 'maasserver/node_view.html'
 
@@ -111,12 +112,18 @@
         id = self.kwargs.get('id', None)
         return get_object_or_404(Node, id=id)
 
+    def get_form_class(self):
+        return get_transition_form(self.request.user)
+
     def get_context_data(self, **kwargs):
         context = super(NodeView, self).get_context_data(**kwargs)
         node = self.get_object()
         context['can_edit'] = self.request.user.has_perm('edit', node)
         return context
 
+    def get_success_url(self):
+        return reverse('node-view', args=[self.get_object().id])
+
 
 class NodeEdit(UpdateView):