Merge lp:~jtv/maas/multi-nodegroupformfield-label into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Support multiple managed interfaces in NodeGroupFormField.label_from_instance.

It's hard to test explicitly for the case where a nodegroup has multiple managed interfaces, because we don't actually allow that situation yet. But luckily the difference is entirely hidden way inside a unicode.join() call — as far as our code is concerned there is no difference now between one managed interface or several.

I considered adding tests for the fact that unmanaged interfaces are ignored, but I'm not actually sure it's all that important. it seems fairly arbitrary and there were no tests for it before.

Jeroen

[Raphaël Badin (rvb)]

[0]

52 + for interface in nodegroup.nodegroupinterface_set.all():
53 + interface.delete()

nodegroup.nodegroupinterface_set.all().delete() is more concise.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Ah of course! I've been doing that elsewhere as well. Thanks.

=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py	2013-10-07 09:41:03 +0000
+++ src/maasserver/fields.py	2014-01-27 12:38:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Custom model fields."""
@@ -88,11 +88,12 @@
 
     def label_from_instance(self, nodegroup):
         """Django method: get human-readable choice label for nodegroup."""
-        interface = nodegroup.get_managed_interface()
-        if interface is None:
+        interfaces = sorted(
+            interface.ip for interface in nodegroup.get_managed_interfaces())
+        if len(interfaces) > 0:
+            return "%s: %s" % (nodegroup.name, ', '.join(interfaces))
+        else:
             return nodegroup.name
-        else:
-            return "%s: %s" % (nodegroup.name, interface.ip)
 
     def clean(self, value):
         """Django method: provide expected output for various inputs.

=== modified file 'src/maasserver/tests/test_fields.py'
--- src/maasserver/tests/test_fields.py	2013-10-21 05:31:09 +0000
+++ src/maasserver/tests/test_fields.py	2014-01-27 12:38:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test custom model fields."""
@@ -31,7 +31,6 @@
 from maasserver.models import (
     MACAddress,
     NodeGroup,
-    NodeGroupInterface,
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -47,17 +46,16 @@
 
     def test_label_from_instance_tolerates_missing_interface(self):
         nodegroup = factory.make_node_group()
-        interface = nodegroup.get_managed_interface()
-        if interface is not None:
-            NodeGroupInterface.objects.filter(id=interface.id).delete()
+        nodegroup.nodegroupinterface_set.all().delete()
         self.assertEqual(
             nodegroup.name,
             NodeGroupFormField().label_from_instance(nodegroup))
 
     def test_label_from_instance_shows_name_and_address(self):
         nodegroup = factory.make_node_group()
+        [interface] = nodegroup.get_managed_interfaces()
         self.assertEqual(
-            '%s: %s' % (nodegroup.name, nodegroup.get_managed_interface().ip),
+            '%s: %s' % (nodegroup.name, interface.ip),
             NodeGroupFormField().label_from_instance(nodegroup))
 
     def test_clean_defaults_to_master(self):