Merge lp:~sinzui/launchpad/commercial-project-picker into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 14909
Proposed branch: lp:~sinzui/launchpad/commercial-project-picker
Merge into: lp:launchpad
Diff against target: 173 lines (+55/-29)
5 files modified
lib/lp/app/browser/tests/test_vocabulary.py (+26/-0)
lib/lp/app/browser/vocabulary.py (+19/-1)
lib/lp/registry/interfaces/product.py (+3/-0)
lib/lp/registry/tests/test_commercialprojects_vocabularies.py (+6/-18)
lib/lp/registry/vocabularies.py (+1/-10)
To merge this branch: bzr merge lp:~sinzui/launchpad/commercial-project-picker
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+95970@code.launchpad.net

Commit message

[r=gmb][bug=930309] Include commercial subscription in the picker details.

Description of the change

Include commercial subscription in the picker details.

    Launchpad bug: https://bugs.launchpad.net/bugs/930309
    Pre-implementation: jcsackett

The CommercialProjects vocabulary appends parenthetical subscription
information to the project displayname. This looks very in the picker,
and can be confused with the Launchpad-Id. The information often
overruns the space for the entry and is not shown.

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

RULES

    * If the project has a commercial subscription, append it to the
      picker entry details.
      * This could be a performance issue.
        Checking commercial_subscriptionID should mitigate the cost of
        adding the subscription information. Only projects with a commercial
        subscription will get an additional DB check.
        Maybe the commercial subscription information can be gotten in bulk.
      * Oh, commercial_subscription isn't in the schema; it is a cached attr
        on Product.
    * There are three states to present in the picker
      A. commercial subscription: None
      B. Commercial subscription: Active
      C. Commercial subscription: Expired

QA

    * Visit a bug on qastaging.
    * Choose the retarget it.
    * Search for a commercial project.
    * Expand the some of the entries to see the details.
    * Verify the entry shows commercial subscription information.
      Verify the commercial project entry states that it has an active
      commercial subscription.

LINT

    lib/lp/app/browser/vocabulary.py
    lib/lp/app/browser/tests/test_vocabulary.py
    lib/lp/registry/vocabularies.py
    lib/lp/registry/interfaces/product.py
    lib/lp/registry/tests/test_commercialprojects_vocabularies.py

TEST

    ./bin/test -vvc lp.app.browser.tests.test_vocabulary
    ./bin/test -vvc lp.registry.tests.test_commercialprojects_vocabularies

IMPLEMENTATION

Added getCommercialSubscription() to TargetPickerEntrySourceAdapter that
returns None for all target types. Added getCommercialSubscription to
the IProduct subclass to summarise the commercial subscription info.
Removed a duplicate call to getMaintainer() in the base class. Add
has_current_commercial_subscription to IProduct because the model method
was private.
    lib/lp/app/browser/vocabulary.py
    lib/lp/app/browser/tests/test_vocabulary.py
    lib/lp/registry/interfaces/product.py

Removed the overloading the of project's displayname that looked like a
Launchpad-Id and could overrun the picker overlay..
    lib/lp/registry/tests/test_commercialprojects_vocabularies.py
    lib/lp/registry/vocabularies.py

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

Hi Curtis,

Nice branch; r=me

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/tests/test_vocabulary.py'
2--- lib/lp/app/browser/tests/test_vocabulary.py 2012-01-01 02:58:52 +0000
3+++ lib/lp/app/browser/tests/test_vocabulary.py 2012-03-05 18:47:22 +0000
4@@ -35,6 +35,7 @@
5 VocabularyFilter,
6 )
7 from lp.testing import (
8+ celebrity_logged_in,
9 login_person,
10 TestCaseWithFactory,
11 )
12@@ -318,6 +319,31 @@
13 'http://launchpad.dev/fnord',
14 self.getPickerEntry(product).alt_title_link)
15
16+ def test_provides_commercial_subscription_none(self):
17+ product = self.factory.makeProduct(name='fnord')
18+ self.assertEqual(
19+ 'Commercial Subscription: None',
20+ self.getPickerEntry(product).details[1])
21+
22+ def test_provides_commercial_subscription_active(self):
23+ product = self.factory.makeProduct(name='fnord')
24+ self.factory.makeCommercialSubscription(product)
25+ self.assertEqual(
26+ 'Commercial Subscription: Active',
27+ self.getPickerEntry(product).details[1])
28+
29+ def test_provides_commercial_subscription_expired(self):
30+ product = self.factory.makeProduct(name='fnord')
31+ self.factory.makeCommercialSubscription(product)
32+ import datetime
33+ import pytz
34+ then = datetime.datetime(2005, 6, 15, 0, 0, 0, 0, pytz.UTC)
35+ with celebrity_logged_in('admin'):
36+ product.commercial_subscription.date_expires = then
37+ self.assertEqual(
38+ 'Commercial Subscription: Expired',
39+ self.getPickerEntry(product).details[1])
40+
41
42 class TestProjectGroupPickerEntrySourceAdapter(TestCaseWithFactory):
43
44
45=== modified file 'lib/lp/app/browser/vocabulary.py'
46--- lib/lp/app/browser/vocabulary.py 2012-02-22 05:22:13 +0000
47+++ lib/lp/app/browser/vocabulary.py 2012-03-05 18:47:22 +0000
48@@ -235,6 +235,10 @@
49 """Gets the maintainer information for the target picker entry."""
50 raise NotImplemented
51
52+ def getCommercialSubscription(self, target):
53+ """Gets the commercial subscription details for the target."""
54+ return None
55+
56 def getPickerEntries(self, term_values, context_object, **kwarg):
57 """See `IPickerEntrySource`"""
58 entries = (
59@@ -265,7 +269,11 @@
60 maintainer = self.getMaintainer(target)
61 if maintainer is not None:
62 picker_entry.details.append(
63- 'Maintainer: %s' % self.getMaintainer(target))
64+ 'Maintainer: %s' % maintainer)
65+ commercial_subscription = self.getCommercialSubscription(target)
66+ if commercial_subscription is not None:
67+ picker_entry.details.append(
68+ 'Commercial Subscription: %s' % commercial_subscription)
69 return entries
70
71
72@@ -342,6 +350,16 @@
73 """See `TargetPickerEntrySource`"""
74 return target.summary
75
76+ def getCommercialSubscription(self, target):
77+ """See `TargetPickerEntrySource`"""
78+ if target.commercial_subscription:
79+ if target.has_current_commercial_subscription:
80+ return 'Active'
81+ else:
82+ return 'Expired'
83+ else:
84+ return 'None'
85+
86
87 @adapter(IDistribution)
88 class DistributionPickerEntrySourceAdapter(TargetPickerEntrySourceAdapter):
89
90=== modified file 'lib/lp/registry/interfaces/product.py'
91--- lib/lp/registry/interfaces/product.py 2012-02-14 07:06:37 +0000
92+++ lib/lp/registry/interfaces/product.py 2012-03-05 18:47:22 +0000
93@@ -729,6 +729,9 @@
94 "Whether the project's licensing requires a new "
95 "commercial subscription to use launchpad.")))
96
97+ has_current_commercial_subscription = Attribute("""
98+ Whether the project has a current commercial subscription.""")
99+
100 license_status = Attribute("""
101 Whether the license is OPENSOURCE, UNREVIEWED, or PROPRIETARY.""")
102
103
104=== modified file 'lib/lp/registry/tests/test_commercialprojects_vocabularies.py'
105--- lib/lp/registry/tests/test_commercialprojects_vocabularies.py 2012-02-28 04:24:19 +0000
106+++ lib/lp/registry/tests/test_commercialprojects_vocabularies.py 2012-03-05 18:47:22 +0000
107@@ -5,7 +5,6 @@
108
109 __metaclass__ = type
110
111-from lp.app.browser.tales import DateTimeFormatterAPI
112 from lp.registry.interfaces.product import License
113 from lp.registry.vocabularies import CommercialProjectsVocabulary
114 from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
115@@ -96,23 +95,12 @@
116 self.assertEqual(
117 0, len(self.vocab.searchForTerms('norwegian-blue-widget')))
118
119- def test_toTerm_no_subscription(self):
120- # Commercial project terms contain subscription information.
121- term = self.vocab.toTerm(self.maintained_project)
122- self.assertEqual(self.maintained_project, term.value)
123- self.assertEqual('open-widget', term.token)
124- self.assertEqual('Open-widget (no subscription)', term.title)
125-
126- def test_toTerm_with_subscription(self):
127- # Commercial project terms contain subscription information.
128- self.factory.makeCommercialSubscription(self.maintained_project)
129- cs = self.maintained_project.commercial_subscription
130- expiration_date = DateTimeFormatterAPI(cs.date_expires).displaydate()
131- term = self.vocab.toTerm(self.maintained_project)
132- self.assertEqual(self.maintained_project, term.value)
133- self.assertEqual('open-widget', term.token)
134- self.assertEqual(
135- 'Open-widget (expires %s)' % expiration_date, term.title)
136+ def test_toTerm(self):
137+ # Commercial project terms contain subscription information.
138+ term = self.vocab.toTerm(self.maintained_project)
139+ self.assertEqual(self.maintained_project, term.value)
140+ self.assertEqual('open-widget', term.token)
141+ self.assertEqual('Open-widget', term.title)
142
143 def test_getTermByToken_user(self):
144 # The term for a token in the vocabulary is returned for maintained
145
146=== modified file 'lib/lp/registry/vocabularies.py'
147--- lib/lp/registry/vocabularies.py 2012-02-13 21:01:51 +0000
148+++ lib/lp/registry/vocabularies.py 2012-03-05 18:47:22 +0000
149@@ -98,7 +98,6 @@
150 removeSecurityProxy,
151 )
152
153-from lp.app.browser.tales import DateTimeFormatterAPI
154 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
155 from lp.blueprints.interfaces.specification import ISpecification
156 from lp.bugs.interfaces.bugtask import IBugTask
157@@ -1536,15 +1535,7 @@
158
159 def toTerm(self, project):
160 """Return the term for this object."""
161- if project.commercial_subscription is None:
162- sub_status = "(no subscription)"
163- else:
164- date_formatter = DateTimeFormatterAPI(
165- project.commercial_subscription.date_expires)
166- sub_status = "(expires %s)" % date_formatter.displaydate()
167- return SimpleTerm(project,
168- project.name,
169- '%s %s' % (project.displayname, sub_status))
170+ return SimpleTerm(project, project.name, project.displayname)
171
172 def getTermByToken(self, token):
173 """Return the term for the given token."""