launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02883
[Merge] lp:~sinzui/launchpad/hide-license-info into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/hide-license-info into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#490148 License details for "Other" are displayed even if not selected
https://bugs.launchpad.net/bugs/490148
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/hide-license-info/+merge/52467
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
--
https://code.launchpad.net/~sinzui/launchpad/hide-license-info/+merge/52467
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/hide-license-info into lp:launchpad.
=== 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:27:35 +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:27:35 +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('no-priv@xxxxxxxxxxxxx')
- >>> 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('no-priv@xxxxxxxxxxxxx')
- >>> 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:27:35 +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:27:35 +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">
