Merge lp:~rvb/maas/bug-1413030 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Also hide the labels when the fields themselves are hidden in the add_node HTML snippet.

The testing for this isn't pretty, I'll admit to that; this is kind of an emergency fix. As part of the current JS-rework, I think we should re-think the usage of snippets.html.

Now, instead of hiding the fields and the label, maybe we should investigate now displaying them at all but the change will be bigger and this is an emergency fix before releasing rc3.

[Gavin Panella (allenap)]

+ self.assertEqual(
+ ['hidden', 'hidden'],
+ [os_system_class, distro_series_class])

Do this in two assertions, something like:

        os_system_class = add_node_snippet.cssselect(
            "[for=id_osystem]")[0].attrib['class']
        self.expectThat(os_system_class, Equals('hidden'))

        distro_series_class = add_node_snippet.cssselect(
            "[for=id_distro_series]")[0].attrib['class']
        self.expectThat(distro_series_class, Equals('hidden'))

Better still, express what you're trying to test:

        get_class = lambda node: node.attrib['class']
        node_is_hidden = AfterPreprocessing(get_class, Equals('hidden'))

        self.expectThat(
            add_node_snippet.cssselect("[for=id_osystem]")[0],
            node_is_hidden)
        self.expectThat(
            add_node_snippet.cssselect("[for=id_distro_series]")[0],
            node_is_hidden)

Or even:

        self.expectThat(
            add_node_snippet.cssselect(".hidden [for=id_osystem]"),
            Not(HasLength(0)))
        self.expectThat(
            add_node_snippet.cssselect(".hidden [for=id_distro_series]"),
            Not(HasLength(0)))

review: Approve

[Raphaël Badin (rvb)]

All right, thanks for the review. I've switched to using two self.expectThat(...) expressions.

=== modified file 'src/maasserver/templates/maasserver/snippets.html'
--- src/maasserver/templates/maasserver/snippets.html	2015-01-20 14:38:01 +0000
+++ src/maasserver/templates/maasserver/snippets.html	2015-01-21 16:07:13 +0000
@@ -19,11 +19,17 @@
       </span>
     </li>
     <li>
-      <label for="id_osystem">OS</label>
+      <label
+        for="id_osystem"
+        class="{% if node_form.osystem.is_hidden %}hidden{% endif %}">
+      OS</label>
       {{ node_form.osystem }}
     </li>
     <li>
-      <label for="id_distro_series">Release</label>
+      <label
+        for="id_distro_series"
+        class="{% if node_form.distro_series.is_hidden %}hidden{% endif %}">
+      Release</label>
       {{ node_form.distro_series }}
     </li>
     <li class="license_key hidden">

=== added file 'src/maasserver/views/tests/test_snippets.py'
--- src/maasserver/views/tests/test_snippets.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/views/tests/test_snippets.py	2015-01-21 16:07:13 +0000
@@ -0,0 +1,47 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test maasserver snippets."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+
+from django.core.urlresolvers import reverse
+from lxml.html import fromstring
+from maasserver.testing.testcase import MAASServerTestCase
+from testtools.matchers import (
+    HasLength,
+    Not,
+    )
+
+
+class SnippetsTest(MAASServerTestCase):
+
+    def test_index_page_containts_add_node_snippet(self):
+        self.client_log_in()
+        index_page = self.client.get(reverse('index'))
+        doc = fromstring(index_page.content)
+        self.assertEqual(
+            'text/x-template', doc.cssselect('#add-node')[0].attrib['type'])
+
+    def test_add_node_snippet_hides_osystem_distro_series_labels(self):
+        self.client_log_in()
+        index_page = self.client.get(reverse('index'))
+        doc = fromstring(index_page.content)
+        content_text = doc.cssselect('#add-node')[0].text_content()
+        add_node_snippet = fromstring(content_text)
+        self.expectThat(
+            add_node_snippet.cssselect("label[for=id_osystem].hidden"),
+            Not(HasLength(0)), "No hidden id_osystem label")
+        self.expectThat(
+            add_node_snippet.cssselect("label[for=id_distro_series].hidden"),
+            Not(HasLength(0)), "No hidden id_distro_series label")