Merge lp:~sinzui/launchpad/hide-license-info into lp:launchpad

[Curtis Hovey]

Hide license_info when Other/* licenses are not selected.

    Launchpad bug: https://bugs.launchpad.net/bugs/490148
    Pre-implementation: no one
    Test command: ./bin/test -vv -t TestProductView

The Other/Open Source and Other/Proprietary license info is shown even
when Other/Open Source and Other/Proprietary are not selected. The
information in the field is often historical, and may contradict the
current license. Some users do not know that they need to manually delete
the text when editing the licenses, and they may not see the field once
they have unselected the license. Some users do not want to delete the
historical info, they want it hidden.

I have spent several hours this year helping users with this. I do not
want to explain this again. I can fix the display issue in less time that
it takes to help two more users.

--------------------------------------------------------------------

RULES

    * Add a rule to the view to tell the template to show the field
      when Other/Open Source or Other/Proprietary is selected.

QA

    * Visit https://launchpad.net/javaframework
    * Verify it does not state it is both GPL and CC-NC.

LINT

    lib/lp/registry/browser/product.py
    lib/lp/registry/browser/tests/product-views.txt
    lib/lp/registry/browser/tests/test_product.py
    lib/lp/registry/templates/product-index.pt

IMPLEMENTATION

Replaced an doctest for the ProductView with a unittest.
    lib/lp/registry/browser/tests/product-views.txt
    lib/lp/registry/browser/tests/test_product.py

Added a property to the ProductView to tell the template when to show
the license_info field.
    lib/lp/registry/browser/product.py
    lib/lp/registry/browser/tests/test_product.py

Updated the template to use the new property. Fixed a subtle issue with
validating script tag...a html-aware checker fails the template because
the script looks empty, which is invalid HTML markup.
    lib/lp/registry/templates/product-index.pt

[Benji York (benji)]

Looks good. Straightforward and well tested.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2011-02-05 06:11:48 +0000
+++ lib/lp/registry/browser/product.py	2011-03-07 20:29:17 +0000
@@ -42,7 +42,6 @@
     ]
 
 
-from cgi import escape
 from datetime import (
     datetime,
     timedelta,
@@ -1110,6 +1109,13 @@
         return (check_permission('launchpad.Edit', self.context) or
                 check_permission('launchpad.Commercial', self.context))
 
+    @cachedproperty
+    def show_license_info(self):
+        """Should the view show the extra license information."""
+        return (
+            License.OTHER_OPEN_SOURCE in self.context.licenses
+            or License.OTHER_PROPRIETARY in self.context.licenses)
+
 
 class ProductPackagesView(LaunchpadView):
     """View for displaying product packaging"""

=== modified file 'lib/lp/registry/browser/tests/product-views.txt'
--- lib/lp/registry/browser/tests/product-views.txt	2010-10-29 15:44:25 +0000
+++ lib/lp/registry/browser/tests/product-views.txt	2011-03-07 20:29:17 +0000
@@ -441,37 +441,6 @@
 Product index view
 ==================
 
-Programming languages
----------------------
-
-When the product has no programming languages specified, nothing is displayed
-on the index page.
-
-    >>> login('[e-mail]')
-    >>> view = create_initialized_view(firefox, name='+index')
-    >>> view.show_programming_languages
-    False
-
-But if the project owner logs in, they will see the programming language
-details, so that they can edit them.
-
-    >>> login_person(firefox.owner)
-    >>> view = create_initialized_view(firefox, name='+index')
-    >>> view.show_programming_languages
-    True
-
-Once the programming languages are specified though, even users who aren't the
-owner will see the list.
-
-    >>> firefox.programminglang = 'C++'
-    >>> transaction.commit()
-
-    >>> login('[e-mail]')
-    >>> view = create_initialized_view(firefox, name='+index')
-    >>> view.show_programming_languages
-    True
-
-
 +index portlets
 ---------------
 

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2010-11-10 22:04:20 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2011-03-07 20:29:17 +0000
@@ -23,6 +23,7 @@
     )
 from lp.testing import (
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.mail_helpers import pop_notifications
@@ -169,3 +170,51 @@
         view = create_initialized_view(self.product_set, '+new')
         message = find_tag_by_id(view.render(), 'staging-message')
         self.assertEqual(None, message)
+
+
+class TestProductView(TestCaseWithFactory):
+    """Tests the ProductView."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestProductView, self).setUp()
+        self.product = self.factory.makeProduct()
+
+    def test_show_programming_languages_without_languages(self):
+        # show_programming_languages is false when there are no programming
+        # languages set.
+        view = create_initialized_view(self.product, '+index')
+        self.assertEqual(None, self.product.programminglang)
+        self.assertFalse(view.show_programming_languages)
+
+    def test_show_programming_languages_with_languages(self):
+        # show_programming_languages is true when programming languages
+        # are set.
+        with person_logged_in(self.product.owner):
+            self.product.programminglang = 'C++'
+        view = create_initialized_view(self.product, '+index')
+        self.assertTrue(view.show_programming_languages)
+
+    def test_show_license_info_without_other_license(self):
+        # show_license_info is false when one of the "other" licenses is
+        # not selected.
+        view = create_initialized_view(self.product, '+index')
+        self.assertEqual((License.GNU_GPL_V2, ), self.product.licenses)
+        self.assertFalse(view.show_license_info)
+
+    def test_show_license_info_with_other_open_source_license(self):
+        # show_license_info is true when the Other/Open Source license is
+        # selected.
+        view = create_initialized_view(self.product, '+index')
+        with person_logged_in(self.product.owner):
+            self.product.licenses = [License.OTHER_OPEN_SOURCE]
+        self.assertTrue(view.show_license_info)
+
+    def test_show_license_info_with_other_open_proprietary_license(self):
+        # show_license_info is true when the Other/Proprietary license is
+        # selected.
+        view = create_initialized_view(self.product, '+index')
+        with person_logged_in(self.product.owner):
+            self.product.licenses = [License.OTHER_PROPRIETARY]
+        self.assertTrue(view.show_license_info)

=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt	2010-11-10 15:33:47 +0000
+++ lib/lp/registry/templates/product-index.pt	2011-03-07 20:29:17 +0000
@@ -41,7 +41,7 @@
                   },
                   window);
           });
-        "/>
+        "></script>
   </tal:head-epilogue>
 </head>
 
@@ -162,9 +162,9 @@
                         condition="not:repeat/license/end">,</tal:comma>
                   </tal:licenses>
                   <tal:none condition="not:context/licenses">None specified</tal:none>
-                  <tal:has_description condition="context/license_info">
-                    (<tal:description replace="context/license_info" />)
-                  </tal:has_description>
+                  <tal:has_license_info condition="view/show_license_info">
+                    (<tal:license_info replace="context/license_info" />)
+                  </tal:has_license_info>
                 </dd>
                 <dd id="commercial_subscription"
                   tal:condition="context/commercial_subscription">