launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/maas/maas-delete-node-bug-968280 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-delete-node-bug-968280/+merge/100574

This branch adds a link to delete a node on the "node view" page.  This is only visible/accessible to admin users.
-- 
https://code.launchpad.net/~rvb/maas/maas-delete-node-bug-968280/+merge/100574
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-delete-node-bug-968280 into lp:maas.

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-04-03 06:42:29 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-04-03 10:02:19 +0000
@@ -14,19 +14,25 @@
       </a>
     </div>
   {% endif %}
-  {% if form.transition_buttons %}
+  {% if form.transition_buttons or can_delete %}
+    <div class="block size3">
+    <h4>Actions</h4>
+    {% if can_delete %}
+      <a href="{% url 'node-delete' node.system_id %}" class="button secondary">
+        Delete node
+      </a>
+    {% endif %}
     {% for transition in form.transition_buttons %}
       {% if forloop.first %}
-      <div class="block size3">
-        <h4>Actions</h4>
         <form id="node_actions" method="post" action=".">
       {% endif %}
-      <input class="secondary {% if not forloop.first %}space-top{% endif %}"
+      <input class="secondary {% if not forloop.first or can_delete %}space-top{% endif %}"
              type="submit"
              name="{{ form.input_name }}"
              value="{{ transition.display }}" />
-      {% if forloop.last %}</form></div>{% endif %}
+      {% if forloop.last %}</form>{% endif %}
     {% endfor %}
+    </div>
   {% endif %}
 {% endblock %}
 

=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py	2012-04-03 09:27:09 +0000
+++ src/maasserver/tests/test_views.py	2012-04-03 10:02:19 +0000
@@ -501,6 +501,36 @@
         node_edit_link = reverse('node-edit', args=[node.system_id])
         self.assertIn(node_edit_link, get_content_links(response))
 
+    def test_view_node_does_not_show_link_to_delete_node(self):
+        # Only admin users can delete nodes.
+        node = factory.make_node(owner=self.logged_in_user)
+        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.assertNotIn(node_delete_link, get_content_links(response))
+
+    def test_user_cannot_delete_node(self):
+        node = factory.make_node(owner=self.logged_in_user)
+        node_delete_link = reverse('node-delete', args=[node.system_id])
+        response = self.client.get(node_delete_link)
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_view_node_shows_link_to_delete_node_for_admin(self):
+        self.become_admin()
+        node = factory.make_node(owner=factory.make_user())
+        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(owner=factory.make_user())
+        node_delete_link = reverse('node-delete', args=[node.system_id])
+        response = self.client.post(node_delete_link, {'post': 'yes'})
+        self.assertEqual(httplib.FOUND, response.status_code)
+        self.assertFalse(User.objects.filter(id=node.id).exists())
+
     def test_user_cannot_view_someone_elses_node(self):
         node = factory.make_node(owner=factory.make_user())
         node_view_link = reverse('node-view', args=[node.system_id])

=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py	2012-04-02 13:43:04 +0000
+++ src/maasserver/urls.py	2012-04-03 10:02:19 +0000
@@ -34,6 +34,7 @@
     combo_view,
     login,
     logout,
+    NodeDelete,
     NodeEdit,
     NodeListView,
     NodesCreateView,
@@ -89,6 +90,9 @@
         r'^nodes/(?P<system_id>[\w\-]+)/edit/$', NodeEdit.as_view(),
         name='node-edit'),
     url(
+        r'^nodes/(?P<system_id>[\w\-]+)/delete/$', NodeDelete.as_view(),
+        name='node-delete'),
+     url(
         r'^nodes/create/$', NodesCreateView.as_view(), name='node-create'),
 )
 

=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py	2012-04-03 09:40:38 +0000
+++ src/maasserver/views.py	2012-04-03 10:02:19 +0000
@@ -129,6 +129,8 @@
         node = self.get_object()
         context['can_edit'] = self.request.user.has_perm(
             NODE_PERMISSIONS.EDIT, node)
+        context['can_delete'] = self.request.user.has_perm(
+            NODE_PERMISSIONS.ADMIN, node)
         return context
 
     def get_success_url(self):
@@ -156,6 +158,29 @@
         return reverse('node-view', args=[self.get_object().system_id])
 
 
+class NodeDelete(DeleteView):
+
+    template_name = 'maasserver/node_confirm_delete.html'
+
+    context_object_name = 'node_to_delete'
+
+    def get_object(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_PERMISSIONS.ADMIN)
+        return node
+
+    def get_next_url(self):
+        return reverse('node-list')
+
+    def delete(self, request, *args, **kwargs):
+        node = self.get_object()
+        node.delete()
+        messages.info(request, "Node %s deleted." % node.system_id)
+        return HttpResponseRedirect(self.get_next_url())
+
+
 def get_longpoll_context():
     if messaging is not None and django_settings.LONGPOLL_PATH is not None:
         try: