Merge lp:~jtv/maas/bug-986185-conditional-node-actions into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Turn Delete Node into a regular node action.

Actually I split off two preparatory branches, which are independent of one another but both needed for this one. And an MP will only take one prerequisite branch, so review may have to wait until both the pre-986185-* branches are reviewed and landed.

The Delete Node button is special in many ways that set it apart from generic, easily defined “node actions”:
 - It can be visible-but-disabled if the user has the requisite permissions, but circumstances prevent the action.
 - It's actually a link, not a button, even though it's dressed up to look like a button.
 - It requires special treatment in the view in order to control its visibility and availability like a node action's.
 - It forwards to a confirmation page on a different view.

Nevertheless, this branch turns it into a regular node action. To that end I had to shave a few yaks:

1. Inhibiting node actions.

Every node action can now provide an “inhibit” function. This function looks for a reason to disable the action, but leave it visible. It returns None if there is no reason to disable it, or an explanatory text.

As it stands, all actions' inhibitions are calculated on every use of the view. This is actually a bit wasteful: on an action POST the only action that needs to be checked for inhibitions is the one that's being exercised. (The action does a double-check, so that for instance you don't accidentally delete a node that's been acquired since you loaded its overview page.) There shouldn't normally be too many actions though.

2. Compiling actions.

The node-action form had a list of references to NODE_ACTION entries, plus a more compact version of the same information. I unify them into “compiled” actions, which are like the applicable actions but any inhibitions are stored as an extra entry.

3. Disabled buttons.

A disabled action button is still a button, but with some different styling, plus a tooltip showing what's inhibiting the action, such as “you haven't registered an SSH key yet.”

4. Redirection.

The delete-node action returns a redirect to the confirmation page. This is how an action POST can support the same workflow as the first half of the old GET/POST combination. It's probably a bit less inefficient, but if that becomes a problem, we should probably be looking at mass-deletion UIs instead of shaving milliseconds off the individual operation.

In one of the prep branches I introduced a new exception type, Redirect, which conveys a temporary-redirect response to the exception middleware or, in this case, a wrapper in the view that catches MAASAPIExceptions and returns their HTTP responses. (This was already documented; the prep branch puts the logic for creating the response in the exception itself so that it's easily reusable.)

My final branch in this series will apply the new infrastructure to the Start Node button, so that you won't be able to acquire and start a node unless you have an SSH key registered to be installed on the node.

Jeroen

[Julian Edwards (julian-edwards)]

Thank you for this, it's quite a lot nicer! Just a few comments, and one thing that is a needs-fixing, but not your fault, you just get the straw that broke the reviewer's back I'm afraid. See below ...

-----

The DELETE_ACTION looks really disjoint against the other inline actions now, is there any more refactoring we can do with those so the NODE_ACTIONS stuff looks nicer?

26 + if node.owner is not None:
27 + return "You cannot delete this node because it's in use."
28 + else:
29 + return None
...
135 + if action is None:
136 + return False
137 + else:
138 + return self.user.has_perm(action['permission'], self.node)

You don't need the "else:" on these functions.

176 + def find_action(self, action_name):
177 + """Find the compiled action of the given name, or return None."""
178 + for action in self.action_buttons:
179 + if action['display'] == action_name:
180 + return action
181 + return None

Mostly thinking out loud here, but I wonder if we can turn action_buttons into a dict so you don't need to iterate here. Whenever I see code doing this it just makes me think it's using the wrong data types.

And here's the need-fixing thing: I remember raising this with rvb before and I'd forgotten about it, but these actions in the dicts appearing as strings is terrible, they really need to be enumerated values. Can you please see if you can sort it out in this branch since you're adding a new action? And damn, I wish we had a proper enum type :/

review: Needs Fixing

[Jeroen T. Vermeulen (jtv)]

> Thank you for this, it's quite a lot nicer! Just a few comments, and one
> thing that is a needs-fixing, but not your fault, you just get the straw that
> broke the reviewer's back I'm afraid. See below ...

Speaking of which, I'm sorry you had to review this before the prerequisite branches had landed. If it helps any, I positively agonized over which one to specify as the pre-req to minimize your pain!

(Yes, I could have made one of the prereqs artificially depend on the other, but that would have had its complications as well.)

> The DELETE_ACTION looks really disjoint against the other inline actions now,
> is there any more refactoring we can do with those so the NODE_ACTIONS stuff
> looks nicer?

That's only because I broke this job into separate branches. :) My other main branch for this bug adorns the “Start node” action in much the same way.

> 26 + if node.owner is not None:
> 27 + return "You cannot delete this node because it's in use."
> 28 + else:
> 29 + return None
> ...
> 135 + if action is None:
> 136 + return False
> 137 + else:
> 138 + return self.user.has_perm(action['permission'], self.node)
>
> You don't need the "else:" on these functions.

It hadn't escaped my notion! It's mostly a matter of taste. In this case I felt that to a human reader these code flows were better expressed as alternate code paths (if/else) than as an “early escape” followed by a longer “regular” code path (if-return). But for functions this short, who's to say? As a wise man said, long ago: is 0 octal or decimal?

> 176 + def find_action(self, action_name):
> 177 + """Find the compiled action of the given name, or return None."""
> 178 + for action in self.action_buttons:
> 179 + if action['display'] == action_name:
> 180 + return action
> 181 + return None
>
> Mostly thinking out loud here, but I wonder if we can turn action_buttons into
> a dict so you don't need to iterate here. Whenever I see code doing this it
> just makes me think it's using the wrong data types.

Absolutely. It started out as a dict plus a separate list, which bogged down my sweeping changes so I unified them into an OrderedDict. It was the data structure that had everything we needed. But that still complicated things like usage from the template. And with only one lookup ever happening anywhere anyway, it just didn't seem worth the effect it had on the choice of data structure.

If you really like, I could make another attempt for an OrderedDict. But that doesn't seem quite ambitious enough given the following:

> And here's the need-fixing thing: I remember raising this with rvb before and
> I'd forgotten about it, but these actions in the dicts appearing as strings is
> terrible, they really need to be enumerated values. Can you please see if you
> can sort it out in this branch since you're adding a new action? And damn, I
> wish we had a proper enum type :/

Maybe we should nail down the typing of NODE_ACTIONS a bit more. For example, we could compile it out of a single flat list of action classes, relational-style. The current structure w...

[Julian Edwards (julian-edwards)]

On Thursday 26 April 2012 06:41:17 you wrote:
> Speaking of which, I'm sorry you had to review this before the prerequisite
> branches had landed. If it helps any, I positively agonized over which one
> to specify as the pre-req to minimize your pain!

No worries, it was a small pre-req!

> That's only because I broke this job into separate branches. :) My other
> main branch for this bug adorns the “Start node” action in much the same
> way.

Ah! I have the memory of a goldfish.

> Maybe we should nail down the typing of NODE_ACTIONS a bit more. For
> example, we could compile it out of a single flat list of action classes,
> relational-style. The current structure would get compiled out of that
> single list. Each action class would have a list of node states it applied
> to, and get inserted into each. That would make it easier and cleaner to
> add mappings such as an enum-to-action mapping.
>
> (This approach would also have the side effect that any two actions that can
> both appear on the same page will always appear there in the same order;
> theoretically there are cases where you might want to design it differently
> but I suspect it's what we want anyway.)
>
> Every action in NODE_ACTIONS would be a class, not a dict, so we don't need
> to test against mis-spelled keys and such. Instead of “compiling” a node
> action we'd instantiate it, passing the state (user, node, request) to its
> initializer. The “execute” and “inhibit” items would become methods, which
> would have access to self.user, self.node etc. Adding more items to the
> signature would affect the base-class initializer only.
>
> I really don't think it's as much work as it sounds, but if you agree, I'd
> prefer to do it in a separate branch.

Yes yes yes, this is massively better! Even though I can't *quite* visualise
it without an example I think I get the gist.

Separate branch is fine, thank you!

[Julian Edwards (julian-edwards)]

review: Approve

[MAAS Lander (maas-lander)]

Attempt to merge into lp:maas failed due to conflicts:

text conflict in src/maasserver/forms.py
text conflict in src/maasserver/tests/test_forms.py

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-04-26 03:27:34 +0000
+++ src/maasserver/forms.py	2012-04-26 07:54:18 +0000
@@ -37,6 +37,7 @@
     PermissionDenied,
     ValidationError,
     )
+from django.core.urlresolvers import reverse
 from django.core.validators import URLValidator
 from django.forms import (
     CharField,
@@ -51,6 +52,7 @@
     NODE_PERMISSION,
     NODE_STATUS,
     )
+from maasserver.exceptions import Redirect
 from maasserver.fields import MACAddressFormField
 from maasserver.models import (
     Config,
@@ -246,6 +248,23 @@
     node.start_commissioning(user)
 
 
+def inhibit_delete(node, user, request=None):
+    """Give me one reason why `user` can't delete `node`."""
+    if node.owner is not None:
+        return "You cannot delete this node because it's in use."
+    else:
+        return None
+
+
+def delete_node(node, user, request=None):
+    """Delete-a-node action.
+
+    Permissions and inhibitions have already been checked at this point.  We
+    redirect to the deletion view, which asks the user if they're sure.
+    """
+    raise Redirect(reverse('node-delete', args=[node.system_id]))
+
+
 # Node actions per status.
 #
 # This maps each NODE_STATUS to a list of actions applicable to a node
@@ -276,8 +295,18 @@
 #                    node, favourite_colour(user)),
 # }
 #
+DELETE_ACTION = {
+    'display': "Delete node",
+    'permission': NODE_PERMISSION.ADMIN,
+    'execute': delete_node,
+    'message': "Node deleted.",
+    'inhibit': inhibit_delete,
+}
+
+
 NODE_ACTIONS = {
     NODE_STATUS.DECLARED: [
+        DELETE_ACTION,
         {
             'display': "Accept & commission",
             'permission': NODE_PERMISSION.ADMIN,
@@ -286,6 +315,7 @@
         },
     ],
     NODE_STATUS.FAILED_TESTS: [
+        DELETE_ACTION,
         {
             'display': "Retry commissioning",
             'permission': NODE_PERMISSION.ADMIN,
@@ -293,7 +323,14 @@
             'message': "Started a new attempt to commission this node.",
         },
     ],
+    NODE_STATUS.COMMISSIONING: [
+        DELETE_ACTION,
+    ],
+    NODE_STATUS.MISSING: [
+        DELETE_ACTION,
+    ],
     NODE_STATUS.READY: [
+        DELETE_ACTION,
         {
             'display': "Start node",
             'permission': NODE_PERMISSION.VIEW,
@@ -303,6 +340,15 @@
                 "It has been asked to start up."),
         },
     ],
+    NODE_STATUS.RESERVED: [
+        DELETE_ACTION,
+    ],
+    NODE_STATUS.ALLOCATED: [
+        DELETE_ACTION,
+    ],
+    NODE_STATUS.RETIRED: [
+        DELETE_ACTION,
+    ],
 }
 
 
@@ -322,30 +368,62 @@
     def __init__(self, instance, *args, **kwargs):
         super(NodeActionForm, self).__init__(*args, **kwargs)
         self.node = instance
-        self.action_buttons = self.available_action_methods(
-            self.node, self.user)
-        # Create a convenient dict to fetch the action's name and
-        # the permission to be checked from the button name.
-        self.action_dict = {
-            action['display']: (
-                action['permission'], action['execute'], action['message'])
-            for action in self.action_buttons
-        }
-
-    def available_action_methods(self, node, user):
+        self.action_buttons = self.compile_actions()
+
+    def have_permission(self, action=None):
+        """Does the current user have the permission required by `action`?
+
+        The permission is relative to `self.node`.
+
+        :param action: Action to check.  If no action is given, the answer is
+            always False.
+        :return: Does the user have the required permission to perform
+            `action` on `self.node`?
+        """
+        if action is None:
+            return False
+        else:
+            return self.user.has_perm(action['permission'], self.node)
+
+    def compile_action(self, action):
+        """Return a copy of `action`, but add "inhibition" entry.
+
+        The key "inhibition" maps to a unicode string explaining why the
+        action is not currently available for `self.node`, or None if the
+        action can be executed.
+        """
+        compiled_action = action.copy()
+        inhibit = action.get('inhibit', lambda node, user, request: None)
+        compiled_action['inhibition'] = inhibit(
+                node=self.node, user=self.user, request=self.request)
+        return compiled_action
+
+    def compile_actions(self):
         """Return the actions that this user is allowed to perform on a node.
 
+        Each action will be "filled out," i.e. it will have an "inhibit"
+        callable even if the original action in NODE_ACTIONS did not.
+
         :param node: The node for which the check should be performed.
         :type node: :class:`maasserver.models.Node`
         :param user: The user who would be performing the action.  Only the
             actions available to this user will be returned.
         :type user: :class:`django.contrib.auth.models.User`
-        :return: Any applicable action dicts, as found in NODE_ACTIONS.
+        :return: Any applicable action dicts, as compiled based on
+            NODE_ACTIONS and the current user and node.
         :rtype: Sequence
         """
         return [
-            action for action in NODE_ACTIONS.get(node.status, ())
-            if user.has_perm(action['permission'], node)]
+            self.compile_action(action)
+            for action in NODE_ACTIONS.get(self.node.status, [])
+                if self.have_permission(action)]
+
+    def find_action(self, action_name):
+        """Find the compiled action of the given name, or return None."""
+        for action in self.action_buttons:
+            if action['display'] == action_name:
+                return action
+        return None
 
     def display_message(self, message):
         """Show `message` as feedback after performing an action."""
@@ -355,14 +433,17 @@
     def save(self):
         """An action was requested.  Perform it."""
         action_name = self.data.get(self.input_name)
-        permission, execute, message = (
-            self.action_dict.get(action_name, (None, None, None)))
-        if execute is None:
-            raise PermissionDenied()
-        if not self.user.has_perm(permission, self.node):
-            raise PermissionDenied()
-        execute(node=self.node, user=self.user, request=self.request)
-        self.display_message(message)
+        action = self.find_action(action_name)
+
+        if not self.have_permission(action):
+            raise PermissionDenied("Not a permitted action: %s" % action_name)
+
+        if action['inhibition'] is not None:
+            raise PermissionDenied(action['inhibition'])
+
+        action['execute'](
+            node=self.node, user=self.user, request=self.request)
+        self.display_message(action.get('message'))
 
 
 def get_action_form(user, request=None):

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-04-16 16:22:32 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-04-26 07:54:18 +0000
@@ -12,30 +12,20 @@
       Edit node
     </a>
   {% endif %}
-  {% if form.action_buttons or can_delete %}
+  {% if form.action_buttons %}
     <h4>Actions</h4>
-  {% endif %}
-  {% if can_delete %}
-    {% if node.owner %}
-      <a href="#"
-         title="You cannot delete this node because it's in use."
-         class="disabled button">
-        Delete node
-      </a>
-    {% else %}
-      <a href="{% url 'node-delete' node.system_id %}"
-         class="button secondary">
-        Delete node
-      </a>
-    {% endif %}
-  {% endif %}
-  {% if form.action_buttons %}
     <form id="node_actions" method="post" action=".">
       {% for action in form.action_buttons %}
-        <input class="secondary {% if not forloop.first or can_delete %}space-top{% endif %}"
-               type="submit"
-               name="{{ form.input_name }}"
-               value="{{ action.display }}" />
+        <input
+          class="secondary {% if action.inhibition %}disabled{% endif %} {% if not forloop.first %}space-top{% endif %}"
+          type="submit"
+          name="{{ form.input_name }}"
+          value="{{ action.display }}"
+          {% if action.inhibition %}}
+            title="{{ action.inhibition }}"
+            disabled="disabled"
+          {% endif %}
+          />
       {% endfor %}
     </form>
   {% endif %}

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2012-04-26 06:00:47 +0000
+++ src/maasserver/tests/test_forms.py	2012-04-26 07:54:18 +0000
@@ -12,11 +12,15 @@
 __metaclass__ = type
 __all__ = []
 
+import httplib
+from urlparse import urlparse
+
 from django import forms
 from django.core.exceptions import (
     PermissionDenied,
     ValidationError,
     )
+from django.core.urlresolvers import reverse
 from django.http import QueryDict
 import maasserver.api
 from maasserver.enum import (
@@ -26,11 +30,14 @@
     NODE_STATUS,
     NODE_STATUS_CHOICES_DICT,
     )
+from maasserver.exceptions import MAASAPIException
 from maasserver.forms import (
     ConfigForm,
+    delete_node,
     EditUserForm,
     get_action_form,
     HostnameFormField,
+    inhibit_delete,
     NewUserCreationForm,
     NODE_ACTIONS,
     NodeActionForm,
@@ -48,6 +55,7 @@
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from provisioningserver.enum import POWER_TYPE_CHOICES
+from testtools.testcase import ExpectedException
 
 
 class NodeWithMACAddressesFormTest(TestCase):
@@ -244,7 +252,6 @@
 
     def test_NODE_ACTIONS_initial_states(self):
         allowed_states = set(NODE_STATUS_CHOICES_DICT.keys() + [None])
-
         self.assertTrue(set(NODE_ACTIONS.keys()) <= allowed_states)
 
     def test_NODE_ACTIONS_dict_contains_only_accepted_keys(self):
@@ -263,28 +270,101 @@
 
 class TestNodeActionForm(TestCase):
 
-    def test_available_action_methods_for_declared_node_admin(self):
+    def test_inhibit_delete_allows_deletion_of_unowned_node(self):
+        admin = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        self.assertIsNone(inhibit_delete(node, admin))
+
+    def test_inhibit_delete_inhibits_deletion_of_owned_node(self):
+        admin = factory.make_admin()
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
+        self.assertEqual(
+            "You cannot delete this node because it's in use.",
+            inhibit_delete(node, admin))
+
+    def test_delete_node_redirects_to_confirmation_page(self):
+        admin = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        try:
+            delete_node(node, admin)
+        except MAASAPIException as e:
+            pass
+        else:
+            self.fail("delete_node should have raised a Redirect.")
+        response = e.make_http_response()
+        self.assertEqual(httplib.FOUND, response.status_code)
+        self.assertEqual(
+            reverse('node-delete', args=[node.system_id]),
+            urlparse(response['Location']).path)
+
+    def have_permission_returns_False_if_action_is_None(self):
+        admin = factory.make_admin()
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        self.assertFalse(get_action_form(admin)(node).have_permission(None))
+
+    def have_permission_returns_False_if_lacking_permission(self):
+        user = factory.make_user()
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
+        action = {
+            'permission': NODE_PERMISSION.EDIT,
+        }
+        self.assertFalse(get_action_form(user)(node).have_permission(action))
+
+    def have_permission_returns_True_given_permission(self):
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
+        action = {
+            'permission': NODE_PERMISSION.EDIT,
+        }
+        form = get_action_form(node.owner)(node)
+        self.assertTrue(form.have_permission(action))
+
+    def test_compile_action_records_inhibition(self):
+        node = factory.make_node()
+        user = factory.make_user()
+        form = get_action_form(user)(node)
+        inhibition = factory.getRandomString()
+
+        def inhibit(*args, **kwargs):
+            return inhibition
+
+        compiled_action = form.compile_action({'inhibit': inhibit})
+        self.assertEqual(inhibition, compiled_action['inhibition'])
+
+    def test_compile_action_records_None_if_no_inhibition(self):
+        node = factory.make_node()
+        user = factory.make_user()
+        form = get_action_form(user)(node)
+
+        def inhibit(*args, **kwargs):
+            return None
+
+        compiled_action = form.compile_action({'inhibit': inhibit})
+        self.assertIsNone(compiled_action['inhibition'])
+
+    def test_compile_actions_for_declared_node_admin(self):
         # Check which transitions are available for an admin on a
         # 'Declared' node.
         admin = factory.make_admin()
         node = factory.make_node(status=NODE_STATUS.DECLARED)
         form = get_action_form(admin)(node)
-        actions = form.available_action_methods(node, admin)
-        self.assertEqual(
-            ["Accept & commission"],
+        actions = form.compile_actions()
+        self.assertItemsEqual(
+            ["Accept & commission", "Delete node"],
             [action['display'] for action in actions])
         # All permissions should be ADMIN.
         self.assertEqual(
             [NODE_PERMISSION.ADMIN] * len(actions),
             [action['permission'] for actions in actions])
 
-    def test_available_action_methods_for_declared_node_simple_user(self):
+    def test_compile_actions_for_declared_node_simple_user(self):
         # A simple user sees no actions for a 'Declared' node.
         user = factory.make_user()
         node = factory.make_node(status=NODE_STATUS.DECLARED)
         form = get_action_form(user)(node)
-        self.assertItemsEqual(
-            [], form.available_action_methods(node, user))
+        self.assertItemsEqual([], form.compile_actions())
 
     def test_get_action_form_creates_form_class_with_attributes(self):
         user = factory.make_admin()
@@ -306,8 +386,8 @@
         form = get_action_form(admin)(node)
 
         self.assertItemsEqual(
-            ["Accept & commission"],
-            form.action_dict)
+            ["Accept & commission", "Delete node"],
+            [action['display'] for action in form.action_buttons])
 
     def test_get_action_form_for_user(self):
         user = factory.make_user()
@@ -343,6 +423,15 @@
 
         self.assertRaises(PermissionDenied, form.save)
 
+    def test_save_double_checks_for_inhibitions(self):
+        admin = factory.make_admin()
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
+        form = get_action_form(admin)(
+            node, {NodeActionForm.input_name: "Delete node"})
+        with ExpectedException(PermissionDenied, "You cannot delete.*"):
+            form.save()
+
     def test_start_action_acquires_and_starts_ready_node_for_user(self):
         node = factory.make_node(status=NODE_STATUS.READY)
         user = factory.make_user()

=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py	2012-04-26 06:00:47 +0000
+++ src/maasserver/tests/test_views_nodes.py	2012-04-26 07:54:18 +0000
@@ -129,14 +129,6 @@
             else:
                 self.assertNotIn(help_link, links)
 
-    def test_view_node_shows_link_to_delete_node_for_admin(self):
-        self.become_admin()
-        node = factory.make_node()
-        node_link = reverse('node-view', args=[node.system_id])
-        response = self.client.get(node_link)
-        node_delete_link = reverse('node-delete', args=[node.system_id])
-        self.assertIn(node_delete_link, get_content_links(response))
-
     def test_admin_can_delete_nodes(self):
         self.become_admin()
         node = factory.make_node()
@@ -256,6 +248,26 @@
         content_text = doc.cssselect('#content')[0].text_content()
         self.assertNotIn("Error output", content_text)
 
+    def test_view_node_POST_admin_can_delete_unused_node(self):
+        self.become_admin()
+        node = factory.make_node(status=NODE_STATUS.READY)
+        response = self.client.post(
+            reverse('node-view', args=[node.system_id]),
+            data={NodeActionForm.input_name: "Delete node"})
+        self.assertEqual(httplib.FOUND, response.status_code)
+        self.assertEqual(
+            reverse('node-delete', args=[node.system_id]),
+            urlparse(response['Location']).path)
+
+    def test_view_node_POST_admin_cannot_delete_used_node(self):
+        self.become_admin()
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
+        response = self.client.post(
+            reverse('node-view', args=[node.system_id]),
+            data={NodeActionForm.input_name: "Delete node"})
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
     def test_view_node_POST_admin_can_start_commissioning_node(self):
         self.become_admin()
         node = factory.make_node(status=NODE_STATUS.DECLARED)

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2012-04-25 16:41:15 +0000
+++ src/maasserver/views/nodes.py	2012-04-26 07:54:18 +0000
@@ -107,8 +107,6 @@
         node = self.get_object()
         context['can_edit'] = self.request.user.has_perm(
             NODE_PERMISSION.EDIT, node)
-        context['can_delete'] = self.request.user.has_perm(
-            NODE_PERMISSION.ADMIN, node)
         if node.status in (NODE_STATUS.COMMISSIONING, NODE_STATUS.READY):
             messages.info(self.request, NODE_BOOT_INFO)
         context['error_text'] = (