launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05340
[Merge] lp:~sinzui/launchpad/project-review-3 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/project-review-3 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/project-review-3/+merge/80739
Remove old license search rules so that commercial review takes less time.
Launchpad bug:
https://bugs.launchpad.net/bugs/882714
https://bugs.launchpad.net/bugs/721461
https://bugs.launchpad.net/bugs/873155
Pre-implementation: no one
Bug 882714 "Remove the missing license use case from project license code"
All project now have licenses. The code that search with the zero
license case can be removed.
Bug 721461 "project review search does not help reviewers"
* The text field does not search the whiteboard or license info field
* "Description of additional licenses" is useless
* I cannot search for proprietary projects without a subscription.
Bug 873155 "Gibberish on project review page"
s/need to without approval/need to withhold approval/
--------------------------------------------------------------------
RULES
Bug 882714 "Remove the missing license use case from project license code"
* Remove the code the deals with the missing license case (zero_licenses)
Bug 721461 "project review search does not help reviewers"
* Remove "Description of additional licenses" field
* Update the search_text to search the whiteboard and license info fields
* Add a field that permits me to search for other/proprietary projects
without a commercial subscription.
QA
Bug 882714 "Remove the missing license use case from project license code"
* Visit https://qastaging.launchpad.net/projects/+review-licenses
* Verify there is no section called
"Or has no license specified:"
* Verify you can search for with no licenses specified.
* Verify you can search for I Don't Know licenses.
Bug 721461 "project review search does not help reviewers"
* Visit https://qastaging.launchpad.net/projects/+review-licenses
* Verify there is not section called:
"Description of additional licenses"
* Verify you can search for 'zlib' to locate projects with
other/open source.
* Verify you can search for 'discriminates' to locate projects we
recorded that they discriminate in the whiteboard.
* Verify you can search for other/proprietary projects without a
commercial subscription.
Bug 873155 "Gibberish on project review page"
* Visit https://qastaging.launchpad.net/launchpad/+review-license
* Verify the second paragraph reads:
"You may need to withhold approval for these common reasons:"
LINT
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/test_product.py
lib/lp/registry/doc/commercialsubscription.txt
lib/lp/registry/interfaces/product.py
lib/lp/registry/model/product.py
TEST
./bin/test -vvc -t ProductSetReviewLicensesViewTestCase \
lp.registry.browser.tests.test_product
./bin/test -vvc -t doc/commercialsubscription \
lp.registry.tests.test_doc
IMPLEMENTATION
Updated IProductReviewSearch and IProductSet.forReview() to remove the
license_info_is_empty and zero_licenses fields which are not used. Updated
the search_text clause to also search the license_info and reviewer_whiteboard
fields. Added has_subscription to permit explicit search for projects
without a commercial subscription (The True case was implicitly handled by
the needs_join case in the forReview()) The complex ORing code was not needed
after I removed the unused fields.
lib/lp/registry/interfaces/product.py
lib/lp/registry/model/product.py
lib/lp/registry/doc/commercialsubscription.txt
Removed the unused fields. Added has_subscription and used the custom_widget
to ensure the default value is None so that the reviewers can ignore the
field most of the time.
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/test_product.py
--
https://code.launchpad.net/~sinzui/launchpad/project-review-3/+merge/80739
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/project-review-3 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2011-09-19 12:41:37 +0000
+++ lib/lp/registry/browser/product.py 2011-10-29 18:27:26 +0000
@@ -1871,9 +1871,8 @@
'active',
'project_reviewed',
'license_approved',
- 'license_info_is_empty',
'licenses',
- 'has_zero_licenses',
+ 'has_subscription',
]
side_by_side_field_names = [
@@ -1891,9 +1890,7 @@
_messageNoValue="(do not filter)")
custom_widget('license_approved', LaunchpadRadioWidget,
_messageNoValue="(do not filter)")
- custom_widget('license_info_is_empty', LaunchpadRadioWidget,
- _messageNoValue="(do not filter)")
- custom_widget('has_zero_licenses', LaunchpadRadioWidget,
+ custom_widget('has_subscription', LaunchpadRadioWidget,
_messageNoValue="(do not filter)")
custom_widget('created_after', DateWidget)
custom_widget('created_before', DateWidget)
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2011-05-13 15:20:50 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2011-10-29 18:27:26 +0000
@@ -321,8 +321,7 @@
'license_approved': False,
'search_text': None,
'licenses': set(),
- 'has_zero_licenses': None,
- 'license_info_is_empty': None,
+ 'has_subscription': None,
'created_after': None,
'created_before': None,
'subscription_expires_after': None,
=== modified file 'lib/lp/registry/doc/commercialsubscription.txt'
--- lib/lp/registry/doc/commercialsubscription.txt 2011-10-02 07:02:37 +0000
+++ lib/lp/registry/doc/commercialsubscription.txt 2011-10-29 18:27:26 +0000
@@ -383,6 +383,23 @@
Gnome Applets
gnomebaker
+The license_info field is also searched for matching search_text:
+
+ >>> bzr.license_info = 'Code in /contrib is under a mit-like license.'
+ >>> for product in product_set.forReview(search_text='mit'):
+ ... print product.name
+ bzr
+
+The whiteboard field is also searched for matching search_text:
+
+ >>> from lp.testing import celebrity_logged_in
+ >>> with celebrity_logged_in('registry_experts'):
+ ... bzr.reviewer_whiteboard = (
+ ... 'cc-nc discriminates against commercial uses.')
+ >>> for product in product_set.forReview(search_text='cc-nc'):
+ ... print product.name
+ bzr
+
You can search for whether the product is active or not.
>>> for product in product_set.forReview(active=False):
@@ -411,42 +428,6 @@
... print product.name
bzr
-You can also search for projects that have zero licenses. This is also
-convenient for searching for projects with at least one license.
-
- >>> for product in product_set.forReview(
- ... has_zero_licenses=True, search_text='firefox|a52dec|bzr|derby'):
- ... print product.name
- firefox
- a52dec
- >>> for product in product_set.forReview(
- ... has_zero_licenses=False, search_text='firefox|a52dec|bzr|derby'):
- ... print product.name
- bzr
- derby
-
-You can search for products whose license_info field is empty or not empty.
-This OR the licenses search field must match.
-
- >>> for product in product_set.forReview(license_info_is_empty=True):
- ... print product.name
- tomcat
- python-gnome2-dev
- unassigned
- arch-mirrors
- firefox
- ...
- >>> for product in product_set.forReview(license_info_is_empty=False):
- ... print product.name
- mega-money-maker
- obsolete-junk
- >>> for product in product_set.forReview(license_info_is_empty=False,
- ... licenses=[License.GNU_GPL_V2]):
- ... print product.name
- bzr
- mega-money-maker
- obsolete-junk
-
It is possible to search for problem project that have been reviewed, but
not approved
@@ -511,6 +492,13 @@
... print product.name
bzr
+A reviewer can search for projects without a commercial subscription.
+
+ >>> for product in product_set.forReview(
+ ... has_subscription=False, licenses=[License.OTHER_PROPRIETARY]):
+ ... print product.name
+ mega-money-maker
+
You can search for products based on the date of that
its commercial subscription was modified, which only
happens when a voucher is redeemed.
@@ -534,13 +522,6 @@
>>> product_set.forReview().count() == Product.select().count()
True
-The license_info_is_empty parameter only accepts True, False, or None.
-
- >>> product_set.forReview(license_info_is_empty='foo').count()
- Traceback (most recent call last):
- ...
- AssertionError...
-
The full text search will not match strings with dots in their name
but a clause is included to search specifically for the name.
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2011-08-23 04:38:33 +0000
+++ lib/lp/registry/interfaces/product.py 2011-10-29 18:27:26 +0000
@@ -917,10 +917,9 @@
project_reviewed=Bool(title=_("Is the project license reviewed")),
licenses=Set(title=_('Licenses'),
value_type=Choice(vocabulary=License)),
- license_info_is_empty=Bool(title=_("License info is empty")),
- has_zero_licenses=Bool(title=_("Has zero licenses")),
created_after=Date(title=_("Created after date")),
created_before=Date(title=_("Created before date")),
+ has_subscription=Bool(title=_("Has a commercial subscription")),
subscription_expires_after=Date(
title=_("Subscription expires after")),
subscription_expires_before=Date(
@@ -936,10 +935,9 @@
active=None,
project_reviewed=None,
licenses=None,
- license_info_is_empty=None,
- has_zero_licenses=None,
created_after=None,
created_before=None,
+ has_subscription=None,
subscription_expires_after=None,
subscription_expires_before=None,
subscription_modified_after=None,
@@ -1064,20 +1062,14 @@
title=_('Project Approved'), values=[True, False],
required=False, default=False)
- license_info_is_empty = Choice(
- title=_('Description of additional licenses'),
- description=_('Either this field or any one of the selected licenses'
- ' must match.'),
- vocabulary=emptiness_vocabulary, required=False, default=None)
-
licenses = Set(
title=_('Licenses'),
value_type=Choice(vocabulary=License),
required=False,
default=set())
- has_zero_licenses = Choice(
- title=_('Or has no license specified'),
+ has_subscription = Choice(
+ title=_('Has Commercial Subscription'),
values=[True, False], required=False)
created_after = Date(title=_("Created between"), required=False)
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2011-10-07 16:20:20 +0000
+++ lib/lp/registry/model/product.py 2011-10-29 18:27:26 +0000
@@ -1450,9 +1450,8 @@
def forReview(self, search_text=None, active=None,
project_reviewed=None, license_approved=None, licenses=None,
- license_info_is_empty=None,
- has_zero_licenses=None,
created_after=None, created_before=None,
+ has_subscription=None,
subscription_expires_after=None,
subscription_expires_before=None,
subscription_modified_after=None,
@@ -1473,8 +1472,10 @@
if search_text is not None and search_text.strip() != '':
conditions.append(SQL('''
Product.fti @@ ftq(%(text)s) OR
- Product.name = lower(%(text)s)
- ''' % sqlvalues(text=search_text)))
+ Product.name = %(text)s OR
+ strpos(lower(Product.license_info), %(text)s) > 0 OR
+ strpos(lower(Product.reviewer_whiteboard), %(text)s) > 0
+ ''' % sqlvalues(text=search_text.lower())))
def dateToDatetime(date):
"""Convert a datetime.date to a datetime.datetime
@@ -1536,58 +1537,28 @@
subscription_modified_before)
needs_join = True
- if needs_join:
+ if needs_join or has_subscription:
conditions.append(
CommercialSubscription.productID == Product.id)
- or_conditions = []
- if license_info_is_empty is True:
- # Match products whose license_info doesn't contain
- # any non-space characters.
- or_conditions.append("Product.license_info IS NULL")
- or_conditions.append(r"Product.license_info ~ E'^\\s*$'")
- elif license_info_is_empty is False:
- # license_info contains something besides spaces.
- or_conditions.append(r"Product.license_info ~ E'[^\\s]'")
- elif license_info_is_empty is None:
- # Don't restrict result if license_info_is_empty is None.
- pass
- else:
- raise AssertionError('license_info_is_empty invalid: %r'
- % license_info_is_empty)
-
- has_license_subquery = '''%s (
- SELECT 1
- FROM ProductLicense
- WHERE ProductLicense.product = Product.id
- LIMIT 1
- )
- '''
- if has_zero_licenses is True:
- # The subquery finds zero rows.
- or_conditions.append(has_license_subquery % 'NOT EXISTS')
- elif has_zero_licenses is False:
- # The subquery finds at least one row.
- or_conditions.append(has_license_subquery % 'EXISTS')
- elif has_zero_licenses is None:
- # Don't restrict results if has_zero_licenses is None.
- pass
- else:
- raise AssertionError('has_zero_licenses is invalid: %r'
- % has_zero_licenses)
+ if has_subscription is False:
+ conditions.append(SQL('''
+ NOT EXISTS (
+ SELECT 1
+ FROM CommercialSubscription
+ WHERE CommercialSubscription.product = Product.id
+ LIMIT 1)
+ '''))
if licenses is not None and len(licenses) > 0:
- or_conditions.append('''EXISTS (
+ conditions.append(SQL('''EXISTS (
SELECT 1
FROM ProductLicense
WHERE ProductLicense.product = Product.id
AND license IN %s
LIMIT 1
)
- ''' % sqlvalues(tuple(licenses)))
-
- if len(or_conditions) != 0:
- conditions.append(SQL('(%s)' % '\nOR '.join(or_conditions)))
+ ''' % sqlvalues(tuple(licenses))))
result = IStore(Product).find(
Product, *conditions).config(
=== modified file 'lib/lp/registry/templates/product-review-license.pt'
--- lib/lp/registry/templates/product-review-license.pt 2011-05-10 20:29:52 +0000
+++ lib/lp/registry/templates/product-review-license.pt 2011-10-29 18:27:26 +0000
@@ -19,7 +19,7 @@
</p>
<p>
- You may need to without approval for these common reasons:
+ You may need to withhold approval for these common reasons:
</p>
<ul class="bulleted">
