launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/delete-failing-commissioning-node-bug-994781 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #994781 in MAAS: "removing a node when it has failed commissioning is not possible from the UI"
  https://bugs.launchpad.net/maas/+bug/994781

For more details, see:
https://code.launchpad.net/~allenap/maas/delete-failing-commissioning-node-bug-994781/+merge/105083

Jeroen mentioned basically what the problem was this morning during the call.
-- 
https://code.launchpad.net/~allenap/maas/delete-failing-commissioning-node-bug-994781/+merge/105083
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/delete-failing-commissioning-node-bug-994781 into lp:maas.

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2012-04-27 13:55:54 +0000
+++ src/maasserver/node_action.py	2012-05-08 16:24:22 +0000
@@ -129,7 +129,7 @@
     permission = NODE_PERMISSION.ADMIN
 
     def inhibit(self):
-        if self.node.owner is not None:
+        if self.node.status == NODE_STATUS.ALLOCATED:
             return "You cannot delete this node because it's in use."
         return None
 

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-04-25 16:58:45 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-05-08 16:24:22 +0000
@@ -17,7 +17,9 @@
     <form id="node_actions" method="post" action=".">
       {% for action in form.action_buttons %}
         <input
-          class="secondary {% if action.inhibition %}disabled{% endif %} {% if not forloop.first %}space-top{% endif %}"
+          class="secondary
+                 {% if action.inhibition %}disabled{% endif %}
+                 {% if not forloop.first %}space-top{% endif %}"
           type="submit"
           name="{{ form.input_name }}"
           value="{{ action.display }}"

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2012-04-27 13:55:54 +0000
+++ src/maasserver/tests/test_node_action.py	2012-05-08 16:24:22 +0000
@@ -162,20 +162,19 @@
         action.fake_inhibition = factory.getRandomString()
         self.assertIsNone(action.inhibition)
 
-    def test_Delete_inhibit_allows_if_node_has_no_owner(self):
-        unowned_node = factory.make_node(status=NODE_STATUS.READY)
-        self.assertIsNone(
-            Delete(unowned_node, factory.make_admin()).inhibit())
-
-    def test_Delete_inhibit_disallows_if_node_has_owner(self):
-        owned_node = factory.make_node(
-            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
-        action = Delete(owned_node, factory.make_admin())
+    def test_Delete_inhibit_when_node_is_allocated(self):
+        node = factory.make_node(status=NODE_STATUS.ALLOCATED)
+        action = Delete(node, factory.make_admin())
         inhibition = action.inhibit()
-        self.assertIsNotNone(inhibition)
         self.assertEqual(
             "You cannot delete this node because it's in use.", inhibition)
 
+    def test_Delete_does_not_inhibit_otherwise(self):
+        node = factory.make_node(status=NODE_STATUS.FAILED_TESTS)
+        action = Delete(node, factory.make_admin())
+        inhibition = action.inhibit()
+        self.assertIsNone(inhibition)
+
     def test_Delete_redirects_to_node_delete_view(self):
         node = factory.make_node()
         action = Delete(node, factory.make_admin())