launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-965037 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #965037 in MAAS: "Node page says "allocated" but not to whom"
  https://bugs.launchpad.net/maas/+bug/965037

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-965037/+merge/99303

When showing the status of an allocated node, the UI currently says “Allocated.”  This branch makes it say “Allocated to <user>.”

Django does not appear to have the notion of a user's displayname, only a full name composed of first and last name — but both are optional, and besides of course not everybody on this planet has that kind of name.

The advanced thing to do would have been to use the full name if available, and resort to username if not.  But full names tend to be longer, and that would have stretched the display in a column that wasn't really designed for it.  Usernames tend to be nice and short, so this works well visually.
-- 
https://code.launchpad.net/~jtv/maas/bug-965037/+merge/99303
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-965037 into lp:maas.

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-03-26 05:07:12 +0000
+++ src/maasserver/models.py	2012-03-26 12:18:31 +0000
@@ -119,6 +119,9 @@
     RETIRED = 7
 
 
+# Django choices for NODE_STATUS: sequence of tuples (key, UI
+# representation).  Limited parameterization of the representation is
+# allowed; see Node.display_status.
 NODE_STATUS_CHOICES = (
     (NODE_STATUS.DECLARED, "Declared"),
     (NODE_STATUS.COMMISSIONING, "Commissioning"),
@@ -126,7 +129,7 @@
     (NODE_STATUS.MISSING, "Missing"),
     (NODE_STATUS.READY, "Ready"),
     (NODE_STATUS.RESERVED, "Reserved"),
-    (NODE_STATUS.ALLOCATED, "Allocated"),
+    (NODE_STATUS.ALLOCATED, "Allocated to %(owner)s"),
     (NODE_STATUS.RETIRED, "Retired"),
 )
 
@@ -400,7 +403,19 @@
             return self.system_id
 
     def display_status(self):
-        return NODE_STATUS_CHOICES_DICT[self.status]
+        """Resturn status text as displayed to the user.
+
+        The UI representation is taken from NODE_STATUS_CHOICES_DICT and may
+        interpolate the variable "owner" to reflect the username of the node's
+        current owner, if any.
+        """
+        params = {
+            # The User reference is represented as the user's username.
+            # Don't just say self.owner.username here, or there will be
+            # trouble with unowned nodes!
+            'owner': self.owner,
+        }
+        return NODE_STATUS_CHOICES_DICT[self.status] % params
 
     def add_mac_address(self, mac_address):
         """Add a new MAC Address to this `Node`.

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-03-26 05:07:12 +0000
+++ src/maasserver/tests/test_models.py	2012-03-26 12:18:31 +0000
@@ -69,12 +69,17 @@
         self.assertEqual(len(node.system_id), 41)
         self.assertTrue(node.system_id.startswith('node-'))
 
-    def test_display_status(self):
+    def test_display_status_shows_default_status(self):
         node = factory.make_node()
         self.assertEqual(
-            NODE_STATUS_CHOICES_DICT[NODE_STATUS.DEFAULT_STATUS],
+            NODE_STATUS_CHOICES_DICT[node.status],
             node.display_status())
 
+    def test_display_status_for_allocated_node_shows_owner(self):
+        node = factory.make_node(
+            owner=factory.make_user(), status=NODE_STATUS.ALLOCATED)
+        self.assertIn(node.owner.username, node.display_status())
+
     def test_add_node_with_token(self):
         user = factory.make_user()
         token = create_auth_token(user)