Jeroen T. Vermeulen has proposed merging
lp:~jtv/maas/pre-987585-cosmetics-and-test-helper into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #987585 in MAAS: "Start Node button does not allocate node"
https://bugs.launchpad.net/maas/+bug/987585
For more details, see:
https://code.launchpad.net/~jtv/maas/pre-987585-cosmetics-and-test-helper/+merge/103329
This is still to keep the diff for my main branch small and focused. In the
diff you'll find:
* Removal of an unnecessary export.
* Some docstrings that were previously missing.
* A bit of cosmetic rearrangement of code: remove unnecessary negation /
flatten nesting structure.
* A genuine bug fix for a test (the test for an Allocated node used a Ready
one instead).
* Improvements to a test helper that POSTs to a page and then GETs the result.
* Some changes in wording: keep sentences short / don't use commas to
concatenate what could be stand-alone sentences / use dashes to disambiguate
nontrivial adjective-noun sequences / don't say “details” when offering more
generic information rather than details about the situation at hand.
The test helper deserves some explanation. It would simulate a form submission
from the user's point of view: a POST is answered with a redirect to a GET.
The helper did not verify the response from the POST, thus hiding errors during
testing. The client requests are not transactional, so changes saved in an
abortive POST would still show up in the database!
Jeroen
--
https://code.launchpad.net/~jtv/maas/pre-987585-cosmetics-and-test-helper/+merge/103329
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/pre-987585-cosmetics-and-test-helper into lp:maas.
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-04-23 05:57:31 +0000
+++ src/maasserver/forms.py 2012-04-24 17:17:20 +0000
@@ -17,7 +17,6 @@
"NodeForm",
"MACAddressForm",
"MAASAndNetworkForm",
- "NodeActionForm",
"SSHKeyForm",
"UbuntuForm",
"UIAdminNodeEditForm",
@@ -334,20 +333,21 @@
if user.has_perm(action['permission'], node)]
def display_message(self, message):
+ """Show `message` as feedback after performing an action."""
if self.request is not None:
messages.add_message(self.request, messages.INFO, message)
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 not None:
- if not self.user.has_perm(permission, self.node):
- raise PermissionDenied()
- execute(self.node, self.user)
- self.display_message(message)
- else:
- raise PermissionDenied()
+ if execute is None:
+ raise PermissionDenied()
+ if not self.user.has_perm(permission, self.node):
+ raise PermissionDenied()
+ execute(self.node, self.user)
+ self.display_message(message)
def get_action_form(user, request=None):
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-04-23 05:57:31 +0000
+++ src/maasserver/tests/test_forms.py 2012-04-24 17:17:20 +0000
@@ -351,7 +351,7 @@
def test_start_action_starts_allocated_node_for_owner(self):
node = factory.make_node(
- status=NODE_STATUS.READY, owner=factory.make_user())
+ status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
form = get_action_form(node.owner)(
node, {NodeActionForm.input_name: "Start node"})
form.save()
=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py 2012-04-23 14:02:51 +0000
+++ src/maasserver/tests/test_views_nodes.py 2012-04-24 17:17:20 +0000
@@ -13,6 +13,7 @@
__all__ = []
import httplib
+from urlparse import urlparse
from django.conf import settings
from django.core.urlresolvers import reverse
@@ -264,16 +265,24 @@
NODE_STATUS.COMMISSIONING, reload_object(node).status)
def perform_action_and_get_node_page(self, node, action_name):
+ """POST to perform a node action, then load the resulting page."""
node_link = reverse('node-view', args=[node.system_id])
- self.client.post(
+ response = self.client.post(
node_link,
data={
NodeActionForm.input_name: action_name,
})
- response = self.client.get(node_link)
- return response
+ if response.status_code != httplib.FOUND:
+ self.fail(
+ "POST failed with code %d: '%s'"
+ % (response.status_code, response.content))
+ redirect = urlparse(response['Location']).path
+ if redirect != node_link:
+ self.fail(
+ "Odd: POST on %s redirected to %s." % (node_link, redirect))
+ return self.client.get(node_link)
- def test_start_commisionning_displays_message(self):
+ def test_start_commisioning_displays_message(self):
self.become_admin()
node = factory.make_node(status=NODE_STATUS.DECLARED)
response = self.perform_action_and_get_node_page(
=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py 2012-04-23 10:57:09 +0000
+++ src/maasserver/views/nodes.py 2012-04-24 17:17:20 +0000
@@ -76,11 +76,10 @@
# Info message displayed on the node page for COMMISSIONING
# or READY nodes.
NODE_BOOT_INFO = mark_safe("""
-You can boot this node using Avahi enabled boot media or an
-adequately configured dhcp server, see
+You can boot this node using Avahi-enabled boot media or an adequately
+configured dhcp server. See
<a href="https://wiki.ubuntu.com/ServerTeam/MAAS/AvahiBoot";>
-https://wiki.ubuntu.com/ServerTeam/MAAS/AvahiBoot</a> for
-details.
+https://wiki.ubuntu.com/ServerTeam/MAAS/AvahiBoot</a> for instructions.
""")
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
