launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06329
[Merge] lp:~sinzui/launchpad/apply-commercial-subscription into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/apply-commercial-subscription into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #929569 in Launchpad itself: "cannot apply a commercial subscription to projects without a proprietary license"
https://bugs.launchpad.net/launchpad/+bug/929569
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/apply-commercial-subscription/+merge/92547
Allow anyone to apply a commercial subscription to any active project.
Pre-implementation: wgrant, wallyworld
We want to permit commercial projects to configure sharing, bug
tracking, and code hosting. Commercial projects are identified by a
current commercial subscription. There are about 30 projects that have
commercial featured enabled but do not have a commercial subscription.
Lp will not let me setup a commercial subscription for them because the
projects do not have a proprietary license. The special features will
disappear from these project pages
--------------------------------------------------------------------
RULES
* Remove the constraint that checks for a proprietary license.
The constraint is the CommercialProjects projects vocabulary
used by the view. Lp has tests that verify that calling
redeemSubscriptionVoucher() works with any project.
* The constraints are in CommercialProjectsVocabulary
* WTF: The vocabulary uses launchpad.Moderate.
but PersonVouchersView is using launchpad.Commercial.
launchpad.Commercial is correct. launchpad.Moderate does not
let the user redeem a voucher.
* WTF, I see that the __contains__ checks permit
deactivated and unmaintained projects for most users.
* WTF: the filter rule is re-sorting the sorted data at the
cost of instantiating ever project. The sort rules are identical.
Out of scope
* The vocab filters non-proprietary projects and creates terms
that provide expiration information. The terms.tokens conflict
with picker presentation rules and look like a Launchpad-Id
This is out of scope fo this branch. I will report a bug suggesting
that the picker entry rules include commercial information.
* I reported bug 930309.
Addendum
* The Cancel button on the form does not conform to Lp form
layout. Cancel should be a link.
* The page title does not conform to 3.0 rules. It contains
redundant user information.
* The breadcrumbs are truncated because the page_title is too long.
QA
* On qastaging verify you can apply a commercial subscription to
any of these projects http://pastebin.ubuntu.com/834571/
LINT
lib/lp/registry/vocabularies.py
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_commercialsubscription.py
lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt
lib/lp/registry/templates/person-vouchers.pt
lib/lp/registry/tests/test_commercialprojects_vocabularies.py
TEST
./bin/test -vvc lp.registry.tests.test_commercialprojects_vocabularies
./bin/test -vvc lp.registry.browser.tests.test_commercialsubscription
./bin/test -vvc -t xx-voucher-redemption lp.registry.tests.test_doc
IMPLEMENTATION
I updated the existing test to use lp.testing.testcase. I added tests for
all the methods that the vocabulary implements. This rewrite is difficult
to read in the diff :(. I fixed the __contains__ method. I removed
_filter_projs() because it just duplicated sorting since we do not want
to filter on proprietary licenses. I changed the search method to use
ProductSet.Search because it is faster and it caches the commercial
subscription information. I replace the launchpad.Moderate checks with
launchpad.Commercial because those are the only users who can work
with commercial subscriptions. I replaced the use of product.title with
displayname becase the former was deprecated in 2007. I rewrote
"unsubscribed" because Lp often uses the term with in the verb form, but it
is not possible to unsubscribe form a commercial subscription. I reported
bug 930309 to remind the purple squad to include commercial information
in the the picker details.
lib/lp/registry/vocabularies.py
lib/lp/registry/tests/test_commercialprojects_vocabularies.py
I revised the page_title to provide only the unique information about the
vouchers page. I Replaced the cancel action with a cancel_url, then removed
the form exceptions from the page template. I added a test for the views
properties. I Removed an unneeded test.
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_commercialsubscription.py
lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt
lib/lp/registry/templates/person-vouchers.pt
--
https://code.launchpad.net/~sinzui/launchpad/apply-commercial-subscription/+merge/92547
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/apply-commercial-subscription into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-02-03 22:03:43 +0000
+++ lib/lp/registry/browser/person.py 2012-02-10 18:16:23 +0000
@@ -2084,8 +2084,12 @@
@property
def page_title(self):
- return ('Commercial subscription vouchers for %s'
- % self.context.displayname)
+ return 'Commercial subscription vouchers'
+
+ @property
+ def cancel_url(self):
+ """See `LaunchpadFormView`."""
+ return canonical_url(self.context)
def setUpFields(self):
"""Set up the fields for this view."""
@@ -2153,12 +2157,6 @@
"CommercialProjects")
return len(vocabulary) > 0
- @action(_("Cancel"), name="cancel",
- validator='validate_cancel')
- def cancel_action(self, action, data):
- """Simply redirect to the user's page."""
- self.next_url = canonical_url(self.context)
-
@action(_("Redeem"), name="redeem")
def redeem_action(self, action, data):
salesforce_proxy = getUtility(ISalesforceVoucherProxy)
=== added file 'lib/lp/registry/browser/tests/test_commercialsubscription.py'
--- lib/lp/registry/browser/tests/test_commercialsubscription.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_commercialsubscription.py 2012-02-10 18:16:23 +0000
@@ -0,0 +1,24 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test views that manage commercial subscriptions."""
+
+__metaclass__ = type
+
+from lp.services.webapp.publisher import canonical_url
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.views import create_initialized_view
+
+
+class PersonVouchersViewTestCase(TestCaseWithFactory):
+ layer = DatabaseFunctionalLayer
+
+ def test_init(self):
+ user = self.factory.makePerson()
+ self.factory.makeProduct(owner=user)
+ user_url = canonical_url(user)
+ view = create_initialized_view(user, '+vouchers')
+ self.assertEqual('Commercial subscription vouchers', view.page_title)
+ self.assertEqual(user_url, view.cancel_url)
+ self.assertIs(None, view.next_url)
=== modified file 'lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt'
--- lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt 2012-01-15 11:06:57 +0000
+++ lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt 2012-02-10 18:16:23 +0000
@@ -190,14 +190,3 @@
Reset the class variable to disable forced faults.
>>> SalesforceXMLRPCTestTransport.forced_fault = None
-
-
-Canceling the request
----------------------
-
-If the 'Cancel' button is selected the person's overview page is shown.
-
- >>> browser.open('http://launchpad.dev/~bac/+vouchers')
- >>> browser.getControl('Cancel').click()
- >>> print browser.url
- http://launchpad.dev/~bac
=== modified file 'lib/lp/registry/templates/person-vouchers.pt'
--- lib/lp/registry/templates/person-vouchers.pt 2011-12-29 05:29:36 +0000
+++ lib/lp/registry/templates/person-vouchers.pt 2012-02-10 18:16:23 +0000
@@ -42,16 +42,8 @@
<tal:widget define="widget nocall:view/widgets/voucher">
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
- <tr>
- <td></td>
- <td>
- <input tal:replace="structure view/redeem_action/render" />
- <input tal:replace="structure view/cancel_action/render" />
- </td>
- </tr>
</table>
</metal:widgets>
- <metal:widgets fill-slot="buttons" />
</div>
</tal:has_voucher>
</tal:define_vouchers>
=== modified file 'lib/lp/registry/tests/test_commercialprojects_vocabularies.py'
--- lib/lp/registry/tests/test_commercialprojects_vocabularies.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_commercialprojects_vocabularies.py 2012-02-10 18:16:23 +0000
@@ -5,82 +5,70 @@
__metaclass__ = type
-from unittest import TestCase
-
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
+from lp.app.browser.tales import DateTimeFormatterAPI
from lp.registry.interfaces.product import (
- IProductSet,
License,
)
from lp.registry.vocabularies import CommercialProjectsVocabulary
from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
from lp.testing import (
- ANONYMOUS,
- login,
- logout,
+ celebrity_logged_in,
+ login_celebrity,
+ TestCaseWithFactory,
)
-from lp.testing.factory import LaunchpadObjectFactory
-from lp.testing.layers import LaunchpadFunctionalLayer
-
-
-class TestCommProjVocabulary(TestCase):
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestCommProjVocabulary(TestCaseWithFactory):
"""Test that the CommercialProjectsVocabulary behaves as expected."""
- layer = LaunchpadFunctionalLayer
+ layer = DatabaseFunctionalLayer
def setUp(self):
- login(ANONYMOUS)
+ super(TestCommProjVocabulary, self).setUp()
+ self.owner = self.factory.makePerson(
+ email_address_status=EmailAddressStatus.VALIDATED)
self._createProjects()
self.vocab = CommercialProjectsVocabulary(context=self.owner)
- def tearDown(self):
- logout()
-
def _createProjects(self):
- """Create a proprietary projects."""
- factory = LaunchpadObjectFactory()
- # Create a person to own the projects.
- self.owner = factory.makePerson(
- email_address_status=EmailAddressStatus.VALIDATED)
-
- # Create some proprietary projects.
+ """Create maintained projects."""
+ # Create 5 proprietary projects.
self.num_proprietary = 5
for i in range(self.num_proprietary):
- widget = factory.makeProduct(name='widget%d' % i,
- licenses=[License.OTHER_PROPRIETARY])
- naked_widget = removeSecurityProxy(widget)
- naked_widget.owner = self.owner
- # Create an open source project with a GNU license.
- widget = factory.makeProduct(name='openwidget',
- licenses=[License.GNU_GPL_V3])
- naked_widget = removeSecurityProxy(widget)
- naked_widget.owner = self.owner
-
- def test_emptySearch(self):
- """An empty search should return all commercial projects."""
+ self.factory.makeProduct(
+ name='widget%d' % i, owner=self.owner,
+ licenses=[License.OTHER_PROPRIETARY])
+ # Create an open source project.
+ self.num_commercial = self.num_proprietary + 1
+ self.maintained_project = self.factory.makeProduct(
+ name='open-widget', owner=self.owner,
+ licenses=[License.GNU_GPL_V3])
+ # Create a deactivated open source project.
+ with celebrity_logged_in('admin'):
+ self.deactivated_project = self.factory.makeProduct(
+ name='norwegian-blue-widget', owner=self.owner,
+ licenses=[License.GNU_GPL_V3])
+ self.deactivated_project.active = False
+
+ def test_attributes(self):
+ self.assertEqual('Select a commercial project', self.vocab.displayname)
+ self.assertEqual('Search', self.vocab.step_title)
+ self.assertEqual('displayname', self.vocab._orderBy)
+
+ def test_searchForTerms_empty(self):
+ # An empty search will return all active maintained projects.
results = self.vocab.searchForTerms('')
self.assertEqual(
- self.num_proprietary, len(results),
+ self.num_commercial, len(results),
"Expected %d results but got %d." % (self.num_proprietary,
len(results)))
- def test_SuccessfulSearch(self):
- """Search for a project name that exists."""
- # All of our commercial projects are named 'widgetn' where n in 0..4.
- # So searching for 'widget' should return the all of the commercial
- # projects. The open source project 'openwidget' will match the
- # search too, but be filtered out.
+ def test_searchForTerms_success(self):
+ # Search for for active maintained projects success.
results = self.vocab.searchForTerms('widget')
self.assertEqual(
- self.num_proprietary, len(results),
- "Expected %d results but got %d." % (self.num_proprietary,
- len(results)))
- # Searching on a subset of 'widget' should work also.
- results = self.vocab.searchForTerms('idge')
- self.assertEqual(
- self.num_proprietary, len(results),
+ self.num_commercial, len(results),
"Expected %d results but got %d." % (self.num_proprietary,
len(results)))
# Ensure we get only those that match by searching for a single
@@ -89,9 +77,9 @@
self.assertEqual(1, len(results),
"Expected %d result but got %d." % (1, len(results)))
- def test_FailedSearch(self):
- """Search for projects that are not commercial projects we own."""
- results = self.vocab.searchForTerms('openwidget')
+ def test_searchForTerms_fail(self):
+ # Search for deactivate or non-maintained projects fails.
+ results = self.vocab.searchForTerms('norwegian-blue-widget')
self.assertEqual(0, len(results),
"Expected %d results but got %d." %
(0, len(results)))
@@ -101,21 +89,63 @@
"Expected %d results but got %d." %
(0, len(results)))
- def test_TransitionedProjectsSearch(self):
- """Search for a project that changes from commercial to open."""
- # The project is commercial so the search succeeds.
- project_name = 'widget1'
- results = self.vocab.searchForTerms(project_name)
- self.assertEqual(1, len(results),
- "Expected %d result but got %d." % (1, len(results)))
-
- # Now change the license for the widget.
- widget = getUtility(IProductSet).getByName(project_name)
- naked_widget = removeSecurityProxy(widget)
- naked_widget.licenses = [License.GNU_GPL_V3]
-
- # The project is no longer commercial so it is not found.
- results = self.vocab.searchForTerms(project_name)
- self.assertEqual(0, len(results),
- "Expected %d results but got %d." %
- (0, len(results)))
+ def test_searchForTerms_commercial_admin(self):
+ # Users with launchpad.Commercial can search for any active project.
+ expert = login_celebrity('commercial_admin')
+ self.vocab = CommercialProjectsVocabulary(context=expert)
+ self.assertEqual(
+ 1, len(self.vocab.searchForTerms('open-widget')))
+ self.assertEqual(
+ 0, len(self.vocab.searchForTerms('norwegian-blue-widget')))
+
+ def test_toTerm_no_subscription(self):
+ # Commercial project terms contain subscription information.
+ term = self.vocab.toTerm(self.maintained_project)
+ self.assertEqual(self.maintained_project, term.value)
+ self.assertEqual('open-widget', term.token)
+ self.assertEqual('Open-widget (no subscription)', term.title)
+
+ def test_toTerm_with_subscription(self):
+ # Commercial project terms contain subscription information.
+ self.factory.makeCommercialSubscription(self.maintained_project)
+ cs = self.maintained_project.commercial_subscription
+ expiration_date = DateTimeFormatterAPI(cs.date_expires).displaydate()
+ term = self.vocab.toTerm(self.maintained_project)
+ self.assertEqual(self.maintained_project, term.value)
+ self.assertEqual('open-widget', term.token)
+ self.assertEqual(
+ 'Open-widget (expires %s)' % expiration_date, term.title)
+
+ def test_getTermByToken(self):
+ # The term for a token in the vocabulary is returned.
+ token = self.vocab.getTermByToken('open-widget')
+ self.assertEqual(self.maintained_project, token.value)
+
+ def test_getTermByToken_error(self):
+ # A LookupError is raised if the token is not in the vocabulary.
+ self.assertRaises(
+ LookupError, self.vocab.getTermByToken, 'norwegian-blue-widget')
+
+ def test_iter(self):
+ # The vocabulary can be iterated and the order is by displayname.
+ displaynames = [p.value.displayname for p in self.vocab]
+ self.assertEqual(
+ ['Open-widget', 'Widget0', 'Widget1', 'Widget2', 'Widget3',
+ 'Widget4'],
+ displaynames)
+
+ def test_contains_maintainer(self):
+ # The vocabulary only contains active projects the user maintains.
+ other_project = self.factory.makeProduct()
+ self.assertIs(False, other_project in self.vocab)
+ self.assertIs(False, self.deactivated_project in self.vocab)
+ self.assertIs(True, self.maintained_project in self.vocab)
+
+ def test_contains_commercial_admin(self):
+ # The vocabulary contains all active projects for commercial.
+ other_project = self.factory.makeProduct()
+ expert = login_celebrity('commercial_admin')
+ self.vocab = CommercialProjectsVocabulary(context=expert)
+ self.assertIs(True, other_project in self.vocab)
+ self.assertIs(False, self.deactivated_project in self.vocab)
+ self.assertIs(True, self.maintained_project in self.vocab)
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2012-02-07 14:53:38 +0000
+++ lib/lp/registry/vocabularies.py 2012-02-10 18:16:23 +0000
@@ -135,7 +135,6 @@
from lp.registry.interfaces.product import (
IProduct,
IProductSet,
- License,
)
from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.projectgroup import IProjectGroup
@@ -1496,10 +1495,10 @@
class CommercialProjectsVocabulary(NamedSQLObjectVocabulary):
"""List all commercial projects.
- A commercial project is one that does not qualify for free hosting. For
- normal users only commercial projects for which the user is the
- maintainer, or in the maintainers team, will be listed. For users with
- launchpad.Moderate permission, all commercial projects are returned.
+ A commercial project is an active project that can have a commercial
+ subscription to grant access to proprietary features. The vocabulary
+ contains the active projects user maintains, or all active project
+ if the user is a registry expert.
"""
implements(IHugeVocabulary)
@@ -1513,12 +1512,14 @@
"""The vocabulary's display nane."""
return 'Select a commercial project'
- def _filter_projs(self, projects):
- """Filter the list of all projects to just the commercial ones."""
- return [
- project for project in sorted(projects,
- key=attrgetter('displayname'))
- if not project.qualifies_for_free_hosting]
+ @cachedproperty
+ def product_set(self):
+ return getUtility(IProductSet)
+
+ @cachedproperty
+ def is_commercial_admin(self):
+ """Is the user a commercial admin?"""
+ return check_permission('launchpad.Commercial', self.product_set)
def _doSearch(self, query=None):
"""Return terms where query is in the text of name
@@ -1527,27 +1528,23 @@
user = self.context
if user is None:
return self.emptySelectResults()
- product_set = getUtility(IProductSet)
- if check_permission('launchpad.Moderate', product_set):
- projects = product_set.forReview(
- search_text=query, licenses=[License.OTHER_PROPRIETARY],
- active=True)
+ if self.is_commercial_admin:
+ projects = self.product_set.search(query)
else:
projects = user.getOwnedProjects(match_name=query)
- projects = self._filter_projs(projects)
return projects
def toTerm(self, project):
"""Return the term for this object."""
if project.commercial_subscription is None:
- sub_status = "(unsubscribed)"
+ sub_status = "(no subscription)"
else:
date_formatter = DateTimeFormatterAPI(
project.commercial_subscription.date_expires)
sub_status = "(expires %s)" % date_formatter.displaydate()
return SimpleTerm(project,
project.name,
- '%s %s' % (project.title, sub_status))
+ '%s %s' % (project.displayname, sub_status))
def getTermByToken(self, token):
"""Return the term for the given token."""
@@ -1560,24 +1557,25 @@
def searchForTerms(self, query=None, vocab_filter=None):
"""See `SQLObjectVocabularyBase`."""
results = self._doSearch(query)
- if type(results) is list:
- num = len(results)
- else:
- num = results.count()
+ num = results.count()
return CountableIterator(num, results, self.toTerm)
def _commercial_projects(self):
"""Return the list of commercial projects owned by this user."""
- return self._filter_projs(self._doSearch())
+ return self._doSearch()
def __iter__(self):
"""See `IVocabulary`."""
for proj in self._commercial_projects():
yield self.toTerm(proj)
- def __contains__(self, obj):
+ def __contains__(self, project):
"""See `IVocabulary`."""
- return obj in self._filter_projs([obj])
+ if not project.active:
+ return False
+ if self.is_commercial_admin:
+ return True
+ return self.context.inTeam(project.owner)
class DistributionVocabulary(NamedSQLObjectVocabulary):
Follow ups
