Merge lp:~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page into lp:launchpad
- bug-602385-register-project-from-sourcepackage-page
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Edwin Grubbs |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11326 |
Proposed branch: | lp:~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page |
Merge into: | lp:launchpad |
Diff against target: |
695 lines (+401/-13) 10 files modified
lib/canonical/launchpad/browser/multistep.py (+19/-0) lib/canonical/launchpad/webapp/launchpadform.py (+3/-0) lib/lp/registry/browser/product.py (+30/-3) lib/lp/registry/browser/sourcepackage.py (+47/-4) lib/lp/registry/browser/tests/sourcepackage-views.txt (+14/-3) lib/lp/registry/browser/tests/test_sourcepackage_views.py (+114/-0) lib/lp/registry/model/sourcepackage.py (+0/-2) lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt (+89/-1) lib/lp/registry/templates/sourcepackage-edit-packaging.pt (+20/-0) lib/lp/soyuz/tests/test_publishing.py (+65/-0) |
To merge this branch: | bzr merge lp:~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jelmer Vernooij (community) | Approve | ||
Registry Administrators | Pending | ||
Review via email: mp+31856@code.launchpad.net |
Commit message
Description of the change
Summary
-------
This branch makes it easy to register an upstream project from a source
package by prefilling the project registration form with data from the
source package.
Implementation details
-------
Added link to $sourcepackage/
$sourcepackage/
lib/
lib/
lib/
lib/
The makeBinaryPacka
binarypackagere
lib/
When +edit-packaging became a MultiStepView, some of the tests
quit exercising the view completely, since no errors were expected, and
the view had just stopped doing anything. I added a check inside the
MultiStepView to help prevent this from happening.
lib/
lib/
lib/
Drive-by lint fixes.
lib/
Tests
-----
./bin/test -vv -t '/sourcepackage
Demo and Q/A
------------
The summary field is not always populated, since the
only info a source package has is the summaries from its binary
packages that might not exist yet.
* Open http://
* Select "Register the upstream project" radio button.
* Click the "Link to Upstream Project" button.
* The form should be prefilled.
* Enter the license.
* Submit the form.
* You should now be redirected to the source package page.
* Open http://
* Click the "Register the upstream project" link.
* The form should be prefilled.
* Enter the license.
* Submit the form.
* You should now be redirected to the +edit-packaging page.
Jelmer Vernooij (jelmer) wrote : | # |
Jelmer Vernooij (jelmer) wrote : | # |
The summary seems to be "<packagename>: summary" now, should the packagename perhaps be stripped out ?
It would also be nice to fill in the description field from the multi-line Debian description, or perhaps that is something that can be addressed later.
Jelmer Vernooij (jelmer) : | # |
Edwin Grubbs (edwin-grubbs) wrote : | # |
Hi Jelmer,
Thanks for the review.
> It would be nice to have some tests for get_register_
> sure dashes are converted properly, etc.
The method uses urllib.urlencode(), which is reliable at encoding url parameters. I can add a unit test for that if you still want one, but we generally haven't tested that.
> get_register_
> None - when does that situation occur?
The SourcePackage.
> Since it's likely that the user doesn't want to create a project for their own
> page this way, it might be a good idea to enable the "I do not want to
> maintain this project" checkbox when registering a project this way.
This was commented on in bug 602385, and we thought that the user should make the decision not to own a project explicitly.
> The summary seems to be "<packagename>: summary" now, should the packagename
> perhaps be stripped out ?
The summary is definitely the part that I'm least happy about. The source package may or may not have any summary info depending on whether it has binary packages. If there are multiple binary packages, it will be even uglier, looking like this:
<packagename>: summary
<packagename>: summary2
Do you think I could stop populating the summary this way and just make the user enter it?
> It would also be nice to fill in the description field from the multi-line
> Debian description, or perhaps that is something that can be addressed later.
I think this would have to be done in a follow up branch. As bug 602385 indicates, it's also desired to automatically connect the source package to the new project instead of just redirecting back to the source package. Do you think the Debian description or some other field would be good for filling in the summary field also?
Jelmer Vernooij (jelmer) wrote : | # |
On Thu, 2010-08-05 at 18:04 +0000, Edwin Grubbs wrote:
> Thanks for the review.
>
> > It would be nice to have some tests for get_register_
> > sure dashes are converted properly, etc.
> The method uses urllib.urlencode(), which is reliable at encoding url parameters. I can add a unit test for that if you still want one, but we generally haven't tested that.
I'm not concerned about the URL encoding, but about e.g. replacing
dashes in the source package name with spaces in the project title (as
is done on the first line of get_register_
since this function might get more complex in the future as more fields
are pre-filled-in.
> > Since it's likely that the user doesn't want to create a project for their own
> > page this way, it might be a good idea to enable the "I do not want to
> > maintain this project" checkbox when registering a project this way.
> This was commented on in bug 602385, and we thought that the user should make the decision not to own a project explicitly.
Fair enough.
> > The summary seems to be "<packagename>: summary" now, should the packagename
> > perhaps be stripped out ?
> The summary is definitely the part that I'm least happy about. The source package may or may not have any summary info depending on whether it has binary packages. If there are multiple binary packages, it will be even uglier, looking like this:
> <packagename>: summary
> <packagename>: summary2
> Do you think I could stop populating the summary this way and just make the user enter it?
I hadn't considered that we are basing the new project on a source
package and not on a binary package. As an optimization, I think it
would still be nice to strip out the "packagename:" bit if there is only
a single binary package for the source package, but I don't see a good
alternative in the other situation.
Certainly, having this field pre-filled-in, even when there are multiple
binary packages, is better than having an empty field imho.
> > It would also be nice to fill in the description field from the
> multi-line > Debian description, or perhaps that is something that can
> be addressed later. I think this would have to be done in a follow up
> branch. As bug 602385 indicates, it's also desired to automatically
> connect the source package to the new project instead of just
> redirecting back to the source package. Do you think the Debian
> description or some other field would be good for filling in the
> summary field also?
Using the Debian description to fill in the description field would also
be useful, but the same problem as above applies here as well
unfortunately as there can be multiple binary packages, each with their
own description.
Thanks for following up, nice work.
review approve
Cheers,
Jelmer
Edwin Grubbs (edwin-grubbs) wrote : | # |
> On Thu, 2010-08-05 at 18:04 +0000, Edwin Grubbs wrote:
> > Thanks for the review.
> >
> > > It would be nice to have some tests for get_register_
> making
> > > sure dashes are converted properly, etc.
>
> > The method uses urllib.urlencode(), which is reliable at encoding url
> parameters. I can add a unit test for that if you still want one, but we
> generally haven't tested that.
> I'm not concerned about the URL encoding, but about e.g. replacing
> dashes in the source package name with spaces in the project title (as
> is done on the first line of get_register_
> since this function might get more complex in the future as more fields
> are pre-filled-in.
I added tests for get_register_
> > > Since it's likely that the user doesn't want to create a project for their
> own
> > > page this way, it might be a good idea to enable the "I do not want to
> > > maintain this project" checkbox when registering a project this way.
> > This was commented on in bug 602385, and we thought that the user should
> make the decision not to own a project explicitly.
> Fair enough.
>
> > > The summary seems to be "<packagename>: summary" now, should the
> packagename
> > > perhaps be stripped out ?
> > The summary is definitely the part that I'm least happy about. The source
> package may or may not have any summary info depending on whether it has
> binary packages. If there are multiple binary packages, it will be even
> uglier, looking like this:
> > <packagename>: summary
> > <packagename>: summary2
> > Do you think I could stop populating the summary this way and just make the
> user enter it?
> I hadn't considered that we are basing the new project on a source
> package and not on a binary package. As an optimization, I think it
> would still be nice to strip out the "packagename:" bit if there is only
> a single binary package for the source package, but I don't see a good
> alternative in the other situation.
>
> Certainly, having this field pre-filled-in, even when there are multiple
> binary packages, is better than having an empty field imho.
I removed the parts before the colon. I also eliminated duplicates, since it would look really ugly to have a summary like the one from this sourcepackage.
https:/
I added tests for this also.
> > > It would also be nice to fill in the description field from the
> > multi-line > Debian description, or perhaps that is something that can
> > be addressed later. I think this would have to be done in a follow up
> > branch. As bug 602385 indicates, it's also desired to automatically
> > connect the source package to the new project instead of just
> > redirecting back to the source package. Do you think the Debian
> > description or some other field would be good for filling in the
> > summary field also?
> Using the Debian description to fill in the description field would also
> be useful, but the same problem as above applies here as well
> unfortunately as there can be multiple binary packages, each with their
> own description.
That would be nice. I'll look into it and open a bug ...
Preview Diff
1 | === modified file 'lib/canonical/launchpad/browser/multistep.py' |
2 | --- lib/canonical/launchpad/browser/multistep.py 2010-05-12 19:06:17 +0000 |
3 | +++ lib/canonical/launchpad/browser/multistep.py 2010-08-09 01:33:51 +0000 |
4 | @@ -93,6 +93,14 @@ |
5 | view.total_steps = self.total_steps |
6 | view.is_step = self.getIsStepDict() |
7 | self.step_number += 1 |
8 | + |
9 | + action_required = None |
10 | + for name in self.request.form.keys(): |
11 | + if name.startswith('field.actions.'): |
12 | + action_required = (name, self.request.form[name]) |
13 | + break |
14 | + |
15 | + action_taken = view.action_taken |
16 | while view.next_step is not None: |
17 | view = view.next_step(self.context, self.request) |
18 | assert isinstance(view, StepView), 'Not a StepView: %s' % view |
19 | @@ -102,8 +110,19 @@ |
20 | view.is_step = self.getIsStepDict() |
21 | self.step_number += 1 |
22 | view.injectStepNameInRequest() |
23 | + if view.action_taken is not None: |
24 | + action_taken = view.action_taken |
25 | + |
26 | self.view = view |
27 | |
28 | + if action_required is not None and action_taken is None: |
29 | + # This is mostly useful for catching tests that pass |
30 | + # in invalid form data via a dictionary instead of |
31 | + # using a test browser. |
32 | + raise AssertionError( |
33 | + 'MultiStepView did not find action for %s=%r' |
34 | + % action_required) |
35 | + |
36 | def render(self): |
37 | return self.view.render() |
38 | |
39 | |
40 | === modified file 'lib/canonical/launchpad/webapp/launchpadform.py' |
41 | --- lib/canonical/launchpad/webapp/launchpadform.py 2010-06-23 23:07:10 +0000 |
42 | +++ lib/canonical/launchpad/webapp/launchpadform.py 2010-08-09 01:33:51 +0000 |
43 | @@ -74,6 +74,8 @@ |
44 | |
45 | actions = () |
46 | |
47 | + action_taken = None |
48 | + |
49 | render_context = False |
50 | |
51 | form_result = None |
52 | @@ -112,6 +114,7 @@ |
53 | self.form_result = action.success(data) |
54 | if self.next_url: |
55 | self.request.response.redirect(self.next_url) |
56 | + self.action_taken = action |
57 | |
58 | def render(self): |
59 | """Return the body of the response. |
60 | |
61 | === modified file 'lib/lp/registry/browser/product.py' |
62 | --- lib/lp/registry/browser/product.py 2010-08-04 04:07:21 +0000 |
63 | +++ lib/lp/registry/browser/product.py 2010-08-09 01:33:51 +0000 |
64 | @@ -120,7 +120,7 @@ |
65 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
66 | from canonical.launchpad.webapp.launchpadform import ( |
67 | action, custom_widget, LaunchpadEditFormView, LaunchpadFormView, |
68 | - ReturnToReferrerMixin) |
69 | + ReturnToReferrerMixin, safe_action) |
70 | from canonical.launchpad.webapp.menu import NavigationMenu |
71 | from canonical.launchpad.webapp.tales import MenuAPI |
72 | from canonical.widgets.popup import PersonPickerWidget |
73 | @@ -1850,6 +1850,16 @@ |
74 | search_results_count = 0 |
75 | |
76 | @property |
77 | + def _return_url(self): |
78 | + """This view is using the hidden _return_url field. |
79 | + |
80 | + It is not using the `ReturnToReferrerMixin`, since none |
81 | + of its other code is used, because multistep views can't |
82 | + have next_url set until the form submission succeeds. |
83 | + """ |
84 | + return self.request.form.get('_return_url') |
85 | + |
86 | + @property |
87 | def _next_step(self): |
88 | """Define the next step. |
89 | |
90 | @@ -1866,6 +1876,10 @@ |
91 | self.request.form['name'] = data['name'].lower() |
92 | self.request.form['summary'] = data['summary'] |
93 | |
94 | + # Make this a safe_action, so that the sourcepackage page can skip |
95 | + # the first step with a link (GET request) providing form values. |
96 | + continue_action = safe_action(StepView.continue_action) |
97 | + |
98 | |
99 | class ProjectAddStepTwo(StepView, ProductLicenseMixin, ReturnToReferrerMixin): |
100 | """Step 2 (of 2) in the +new project add wizard.""" |
101 | @@ -1887,6 +1901,16 @@ |
102 | custom_widget('license_info', GhostWidget) |
103 | |
104 | @property |
105 | + def _return_url(self): |
106 | + """This view is using the hidden _return_url field. |
107 | + |
108 | + It is not using the `ReturnToReferrerMixin`, since none |
109 | + of its other code is used, because multistep views can't |
110 | + have next_url set until the form submission succeeds. |
111 | + """ |
112 | + return self.request.form.get('_return_url') |
113 | + |
114 | + @property |
115 | def step_description(self): |
116 | """See `MultiStepView`.""" |
117 | if self.search_results_count > 0: |
118 | @@ -2001,7 +2025,10 @@ |
119 | self.product = self.create_product(data) |
120 | self.notifyCommercialMailingList() |
121 | notify(ObjectCreatedEvent(self.product)) |
122 | - self.next_url = canonical_url(self.product) |
123 | + if self._return_url is None: |
124 | + self.next_url = canonical_url(self.product) |
125 | + else: |
126 | + self.next_url = self._return_url |
127 | |
128 | |
129 | class ProductAddView(MultiStepView): |
130 | @@ -2027,7 +2054,7 @@ |
131 | |
132 | driver = copy_field(IProduct['driver']) |
133 | |
134 | - transfer_to_registry = Bool( |
135 | + transfer_to_registry = Bool( |
136 | title=_("I do not want to maintain this project"), |
137 | required=False, |
138 | description=_( |
139 | |
140 | === modified file 'lib/lp/registry/browser/sourcepackage.py' |
141 | --- lib/lp/registry/browser/sourcepackage.py 2010-07-02 14:34:58 +0000 |
142 | +++ lib/lp/registry/browser/sourcepackage.py 2010-08-09 01:33:51 +0000 |
143 | @@ -19,6 +19,8 @@ |
144 | |
145 | from apt_pkg import ParseSrcDepends |
146 | from cgi import escape |
147 | +import string |
148 | +import urllib |
149 | from z3c.ptcompat import ViewPageTemplateFile |
150 | from zope.app.form.browser import DropdownWidget |
151 | from zope.app.form.interfaces import IInputWidget |
152 | @@ -35,12 +37,14 @@ |
153 | |
154 | from canonical.launchpad import helpers |
155 | from canonical.launchpad.browser.multistep import MultiStepView, StepView |
156 | + |
157 | from lp.bugs.browser.bugtask import BugTargetTraversalMixin |
158 | from canonical.launchpad.browser.packagerelationship import ( |
159 | relationship_builder) |
160 | from lp.answers.browser.questiontarget import ( |
161 | QuestionTargetFacetMixin, QuestionTargetAnswersMenu) |
162 | from lp.services.worlddata.interfaces.country import ICountry |
163 | +from lp.registry.browser.product import ProjectAddStepOne |
164 | from lp.registry.interfaces.packaging import IPackaging, IPackagingUtil |
165 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
166 | from lp.registry.interfaces.product import IProductSet |
167 | @@ -62,6 +66,30 @@ |
168 | from canonical.lazr.utils import smartquote |
169 | |
170 | |
171 | +def get_register_upstream_url(source_package, return_url): |
172 | + displayname = string.capwords(source_package.name.replace('-', ' ')) |
173 | + params = { |
174 | + '_return_url': return_url, |
175 | + 'field.name': source_package.name, |
176 | + 'field.displayname': displayname, |
177 | + 'field.title': displayname, |
178 | + 'field.__visited_steps__': ProjectAddStepOne.step_name, |
179 | + 'field.actions.continue': 'Continue', |
180 | + } |
181 | + if len(source_package.releases) == 0: |
182 | + params['field.summary'] = '' |
183 | + else: |
184 | + # This is based on the SourcePackageName.summary attribute, but |
185 | + # it eliminates the binary.name and duplicate summary lines. |
186 | + summary_set = set() |
187 | + for binary in source_package.releases[0].sample_binary_packages: |
188 | + summary_set.add(binary.summary) |
189 | + params['field.summary'] = '\n'.join(sorted(summary_set)) |
190 | + query_string = urllib.urlencode( |
191 | + sorted(params.items()), doseq=True) |
192 | + return '/projects/+new?%s' % query_string |
193 | + |
194 | + |
195 | class SourcePackageNavigation(GetitemNavigation, BugTargetTraversalMixin): |
196 | |
197 | usedfor = ISourcePackage |
198 | @@ -190,6 +218,11 @@ |
199 | self.next_step = SourcePackageChangeUpstreamStepTwo |
200 | self.request.form['product'] = data['product'] |
201 | |
202 | + @property |
203 | + def register_upstream_url(self): |
204 | + return get_register_upstream_url( |
205 | + self.context, return_url=self.request.getURL()) |
206 | + |
207 | |
208 | class SourcePackageChangeUpstreamStepTwo(ReturnToReferrerMixin, StepView): |
209 | """A view to set the `IProductSeries` of a sourcepackage.""" |
210 | @@ -345,7 +378,7 @@ |
211 | def processForm(self): |
212 | # look for an update to any of the things we track |
213 | form = self.request.form |
214 | - if form.has_key('packaging'): |
215 | + if 'packaging' in form: |
216 | if self.productseries_widget.hasValidInput(): |
217 | new_ps = self.productseries_widget.getInputValue() |
218 | # we need to create or update the packaging |
219 | @@ -445,6 +478,7 @@ |
220 | initial_focus_widget = None |
221 | max_suggestions = 9 |
222 | other_upstream = object() |
223 | + register_upstream = object() |
224 | |
225 | def setUpFields(self): |
226 | """See `LaunchpadFormView`.""" |
227 | @@ -467,9 +501,12 @@ |
228 | vocab_terms.append(SimpleTerm(product, product.name, description)) |
229 | # Add an option to represent the user's decision to choose a |
230 | # different project. Note that project names cannot be uppercase. |
231 | - description = 'Choose another upstream project' |
232 | - vocab_terms.append( |
233 | - SimpleTerm(self.other_upstream, 'OTHER_UPSTREAM', description)) |
234 | + vocab_terms.append( |
235 | + SimpleTerm(self.other_upstream, 'OTHER_UPSTREAM', |
236 | + 'Choose another upstream project')) |
237 | + vocab_terms.append( |
238 | + SimpleTerm(self.register_upstream, 'REGISTER_UPSTREAM', |
239 | + 'Register the upstream project')) |
240 | upstream_vocabulary = SimpleVocabulary(vocab_terms) |
241 | |
242 | self.form_fields = Fields( |
243 | @@ -487,6 +524,12 @@ |
244 | self.next_url = canonical_url( |
245 | self.context, view_name="+edit-packaging") |
246 | return |
247 | + elif upstream is self.register_upstream: |
248 | + # The user wants to create a new project. |
249 | + url = get_register_upstream_url( |
250 | + self.context, return_url=self.request.getURL()) |
251 | + self.request.response.redirect(url) |
252 | + return |
253 | self.context.setPackaging(upstream.development_focus, self.user) |
254 | self.request.response.addInfoNotification( |
255 | 'The project %s was linked to this source package.' % |
256 | |
257 | === modified file 'lib/lp/registry/browser/tests/sourcepackage-views.txt' |
258 | --- lib/lp/registry/browser/tests/sourcepackage-views.txt 2010-05-13 18:55:10 +0000 |
259 | +++ lib/lp/registry/browser/tests/sourcepackage-views.txt 2010-08-09 01:33:51 +0000 |
260 | @@ -119,6 +119,7 @@ |
261 | empty. |
262 | |
263 | >>> form = { |
264 | + ... 'field.__visited_steps__': 'sourcepackage_change_upstream_step1', |
265 | ... 'field.product': '', |
266 | ... 'field.actions.continue': 'Continue', |
267 | ... } |
268 | @@ -133,12 +134,18 @@ |
269 | but there is no notification message that the upstream link was updated. |
270 | |
271 | >>> form = { |
272 | - ... 'field.productseries': 'bonkers/crazy', |
273 | - ... 'field.actions.change': 'Change', |
274 | + ... 'field.__visited_steps__': 'sourcepackage_change_upstream_step2', |
275 | + ... 'field.product': 'bonkers', |
276 | + ... 'field.productseries': 'crazy', |
277 | + ... 'field.actions.continue': 'Continue', |
278 | ... } |
279 | >>> view = create_initialized_view( |
280 | ... package, name='+edit-packaging', form=form, |
281 | ... principal=product.owner) |
282 | + >>> print view.view |
283 | + <...SourcePackageChangeUpstreamStepTwo object...> |
284 | + >>> print view.view.next_url |
285 | + http://launchpad.dev/youbuntu/busy/+source/bonkers |
286 | >>> view.view.errors |
287 | [] |
288 | |
289 | @@ -199,6 +206,7 @@ |
290 | Registered upstream project: |
291 | Lernid |
292 | Choose another upstream project |
293 | + Register the upstream project |
294 | |
295 | The form does not steal focus because it is not the primary purpose of the |
296 | page. |
297 | @@ -229,6 +237,7 @@ |
298 | Lernid... |
299 | Lernid Dev... |
300 | Choose another upstream project |
301 | + Register the upstream project |
302 | |
303 | Choosing the "Choose another upstream project" option redirects the user |
304 | to the +edit-packaging page where the user can search for a project. |
305 | @@ -259,7 +268,8 @@ |
306 | ... name='stinkyseries', product=product) |
307 | >>> distroseries = factory.makeDistroRelease(name='wonky', |
308 | ... distribution=distribution) |
309 | - >>> sourcepackagename = factory.makeSourcePackageName(name='stinkypackage') |
310 | + >>> sourcepackagename = factory.makeSourcePackageName( |
311 | + ... name='stinkypackage') |
312 | >>> package = factory.makeSourcePackage( |
313 | ... sourcepackagename=sourcepackagename, distroseries=distroseries) |
314 | |
315 | @@ -360,3 +370,4 @@ |
316 | match for this source package. Can you help us find one? |
317 | Registered upstream project: |
318 | Choose another upstream project |
319 | + Register the upstream project |
320 | |
321 | === added file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py' |
322 | --- lib/lp/registry/browser/tests/test_sourcepackage_views.py 1970-01-01 00:00:00 +0000 |
323 | +++ lib/lp/registry/browser/tests/test_sourcepackage_views.py 2010-08-09 01:33:51 +0000 |
324 | @@ -0,0 +1,114 @@ |
325 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
326 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
327 | + |
328 | +"""Tests for SourcePackage view code.""" |
329 | + |
330 | +__metaclass__ = type |
331 | + |
332 | +import cgi |
333 | +import urllib |
334 | + |
335 | +from zope.component import getUtility |
336 | + |
337 | +from canonical.testing import DatabaseFunctionalLayer |
338 | + |
339 | +from lp.registry.browser.sourcepackage import get_register_upstream_url |
340 | +from lp.registry.interfaces.distroseries import IDistroSeriesSet |
341 | +from lp.soyuz.tests.test_publishing import SoyuzTestPublisher |
342 | +from lp.testing import TestCaseWithFactory |
343 | + |
344 | + |
345 | +class TestSourcePackageViewHelpers(TestCaseWithFactory): |
346 | + """Tests for SourcePackage view helper functions.""" |
347 | + |
348 | + layer = DatabaseFunctionalLayer |
349 | + |
350 | + def test_get_register_upstream_url_displayname(self): |
351 | + source_package = self.factory.makeSourcePackage( |
352 | + sourcepackagename='python-super-package') |
353 | + return_url = 'http://example.com/foo?a=b&c=d' |
354 | + url = get_register_upstream_url(source_package, return_url) |
355 | + expected_url = ( |
356 | + '/projects/+new?' |
357 | + '_return_url=' |
358 | + 'http%3A%2F%2Fexample.com%2Ffoo%3Fa%3Db%26c%3Dd' |
359 | + '&field.__visited_steps__=projectaddstep1' |
360 | + '&field.actions.continue=Continue' |
361 | + # The sourcepackagename 'python-super-package' is split on |
362 | + # the hyphens, and each word is capitalized. |
363 | + '&field.displayname=Python+Super+Package' |
364 | + '&field.name=python-super-package' |
365 | + # The summary is empty, since the source package doesn't |
366 | + # have a binary package release. |
367 | + '&field.summary=' |
368 | + '&field.title=Python+Super+Package') |
369 | + self.assertEqual(expected_url, url) |
370 | + |
371 | + def test_get_register_upstream_url_summary(self): |
372 | + test_publisher = SoyuzTestPublisher() |
373 | + test_data = test_publisher.makeSourcePackageWithBinaryPackageRelease() |
374 | + source_package_name = ( |
375 | + test_data['source_package'].sourcepackagename.name) |
376 | + distroseries_id = test_data['distroseries'].id |
377 | + test_publisher.updateDistroSeriesPackageCache( |
378 | + test_data['distroseries']) |
379 | + |
380 | + # updateDistroSeriesPackageCache reconnects the db, so the |
381 | + # objects need to be reloaded. |
382 | + distroseries = getUtility(IDistroSeriesSet).get(distroseries_id) |
383 | + source_package = distroseries.getSourcePackage(source_package_name) |
384 | + return_url = 'http://example.com/foo?a=b&c=d' |
385 | + url = get_register_upstream_url(source_package, return_url) |
386 | + expected_base = '/projects/+new' |
387 | + expected_params = [ |
388 | + ('_return_url', 'http://example.com/foo?a=b&c=d'), |
389 | + ('field.__visited_steps__', 'projectaddstep1'), |
390 | + ('field.actions.continue', 'Continue'), |
391 | + ('field.displayname', 'Bonkers'), |
392 | + ('field.name', 'bonkers'), |
393 | + ('field.summary', 'summary for flubber-bin\n' |
394 | + + 'summary for flubber-lib'), |
395 | + ('field.title', 'Bonkers'), |
396 | + ] |
397 | + base, query = urllib.splitquery(url) |
398 | + params = cgi.parse_qsl(query) |
399 | + self.assertEqual((expected_base, expected_params), |
400 | + (base, params)) |
401 | + |
402 | + def test_get_register_upstream_url_summary_duplicates(self): |
403 | + |
404 | + class FakeDistroSeriesBinaryPackage: |
405 | + def __init__(self, summary): |
406 | + self.summary = summary |
407 | + |
408 | + class FakeDistributionSourcePackageRelease: |
409 | + sample_binary_packages = [ |
410 | + FakeDistroSeriesBinaryPackage('summary for foo'), |
411 | + FakeDistroSeriesBinaryPackage('summary for bar'), |
412 | + FakeDistroSeriesBinaryPackage('summary for baz'), |
413 | + FakeDistroSeriesBinaryPackage('summary for baz'), |
414 | + ] |
415 | + |
416 | + class FakeSourcePackage: |
417 | + name = 'foo' |
418 | + releases = [FakeDistributionSourcePackageRelease()] |
419 | + |
420 | + source_package = FakeSourcePackage() |
421 | + return_url = 'http://example.com/foo?a=b&c=d' |
422 | + url = get_register_upstream_url(source_package, return_url) |
423 | + expected_base = '/projects/+new' |
424 | + expected_params = [ |
425 | + ('_return_url', 'http://example.com/foo?a=b&c=d'), |
426 | + ('field.__visited_steps__', 'projectaddstep1'), |
427 | + ('field.actions.continue', 'Continue'), |
428 | + ('field.displayname', 'Foo'), |
429 | + ('field.name', 'foo'), |
430 | + ('field.summary', 'summary for bar\n' |
431 | + + 'summary for baz\n' |
432 | + + 'summary for foo'), |
433 | + ('field.title', 'Foo'), |
434 | + ] |
435 | + base, query = urllib.splitquery(url) |
436 | + params = cgi.parse_qsl(query) |
437 | + self.assertEqual((expected_base, expected_params), |
438 | + (base, params)) |
439 | |
440 | === modified file 'lib/lp/registry/model/sourcepackage.py' |
441 | --- lib/lp/registry/model/sourcepackage.py 2010-08-02 21:38:00 +0000 |
442 | +++ lib/lp/registry/model/sourcepackage.py 2010-08-09 01:33:51 +0000 |
443 | @@ -39,7 +39,6 @@ |
444 | from lp.registry.model.packaging import Packaging |
445 | from lp.translations.model.potemplate import ( |
446 | HasTranslationTemplatesMixin, |
447 | - POTemplate, |
448 | TranslationTemplatesCollection) |
449 | from canonical.launchpad.interfaces.lpstorm import IStore |
450 | from lp.soyuz.model.publishing import ( |
451 | @@ -52,7 +51,6 @@ |
452 | SourcePackageRelease) |
453 | from lp.translations.model.translationimportqueue import ( |
454 | HasTranslationImportsMixin) |
455 | -from canonical.launchpad.helpers import shortlist |
456 | from lp.soyuz.interfaces.buildrecords import IHasBuildRecords |
457 | from lp.registry.interfaces.packaging import PackagingType |
458 | from lp.registry.interfaces.distribution import NoPartnerArchive |
459 | |
460 | === modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt' |
461 | --- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-05-18 17:05:29 +0000 |
462 | +++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-08-09 01:33:51 +0000 |
463 | @@ -1,4 +1,15 @@ |
464 | -= Packaging = |
465 | +Packaging |
466 | +========= |
467 | + |
468 | +Create test data. |
469 | + |
470 | + >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher |
471 | + >>> test_publisher = SoyuzTestPublisher() |
472 | + >>> login('admin@canonical.com') |
473 | + >>> test_data = test_publisher.makeSourcePackageWithBinaryPackageRelease() |
474 | + >>> test_publisher.updateDistroSeriesPackageCache( |
475 | + ... test_data['distroseries']) |
476 | + >>> logout() |
477 | |
478 | No Privileges Person visit the distroseries upstream links page for Hoary |
479 | and sees that pmount is not linked. |
480 | @@ -20,6 +31,7 @@ |
481 | match for this source package. Can you help us find one? |
482 | Registered upstream project: |
483 | Choose another upstream project |
484 | + Register the upstream project |
485 | |
486 | No Privileges Person knows that the pmount package comes from the thunderbird |
487 | project. He sets the upstream packaging link and sees that it is set. |
488 | @@ -58,3 +70,79 @@ |
489 | ... user_browser.contents, 'packages_list')) |
490 | The Hoary Hedgehog Release (active development) ... |
491 | 0.1-2 release (main) ... weeks ago |
492 | + |
493 | +Register a project from a source package |
494 | +---------------------------------------- |
495 | + |
496 | +If an upstream project doesn't already exist in Launchpad, it can |
497 | +be registered with data from the source package prefilling the first |
498 | +step of the multistep form. |
499 | + |
500 | + >>> user_browser.open( |
501 | + ... 'http://launchpad.dev/youbuntu/busy/+source/bonkers') |
502 | + >>> user_browser.getControl( |
503 | + ... 'Register the upstream project').selected = True |
504 | + >>> user_browser.getControl("Link to Upstream Project").click() |
505 | + >>> print user_browser.url.replace('&', '\n&') |
506 | + http://launchpad.dev/projects/+new?_return_url=http...%2Bindex |
507 | + &field.__visited_steps__=projectaddstep1 |
508 | + &field.actions.continue=Continue |
509 | + &field.displayname=Bonkers |
510 | + &field.name=bonkers |
511 | + &field.summary=summary+for+flubber-bin%0Asummary+for+flubber-lib |
512 | + &field.title=Bonkers |
513 | + >>> print user_browser.getControl(name='field.name').value |
514 | + bonkers |
515 | + >>> print user_browser.getControl(name='field.displayname').value |
516 | + Bonkers |
517 | + >>> print user_browser.getControl(name='field.title').value |
518 | + Bonkers |
519 | + >>> print user_browser.getControl(name='field.summary').value |
520 | + summary for flubber-bin |
521 | + summary for flubber-lib |
522 | + >>> print extract_text( |
523 | + ... find_tag_by_id(user_browser.contents, 'step-title')) |
524 | + Step 2 (of 2): Check for duplicate projects |
525 | + |
526 | +If the user selects "Choose another upstream project" and then finds out |
527 | +that the project doesn't exist, there is a also a link on the |
528 | ++edit-packaging page to register the project. |
529 | + |
530 | + >>> user_browser.open( |
531 | + ... 'http://launchpad.dev/youbuntu/busy/+source/bonkers/') |
532 | + >>> user_browser.getControl( |
533 | + ... 'Choose another upstream project').selected = True |
534 | + >>> user_browser.getControl("Link to Upstream Project").click() |
535 | + >>> print user_browser.url |
536 | + http://launchpad.dev/youbuntu/busy/+source/bonkers/+edit-packaging |
537 | + |
538 | + >>> user_browser.getLink("Register the upstream project").click() |
539 | + >>> print user_browser.url.replace('&', '\n&') |
540 | + http://launchpad.dev/projects/+new?_return_url=http...%2Bedit-packaging |
541 | + &field.__visited_steps__=projectaddstep1 |
542 | + &field.actions.continue=Continue |
543 | + &field.displayname=Bonkers |
544 | + &field.name=bonkers |
545 | + &field.summary=summary+for+flubber-bin%0Asummary+for+flubber-lib |
546 | + &field.title=Bonkers |
547 | + >>> print user_browser.getControl(name='field.name').value |
548 | + bonkers |
549 | + >>> print user_browser.getControl(name='field.displayname').value |
550 | + Bonkers |
551 | + >>> print user_browser.getControl(name='field.title').value |
552 | + Bonkers |
553 | + >>> print user_browser.getControl(name='field.summary').value |
554 | + summary for flubber-bin |
555 | + summary for flubber-lib |
556 | + >>> print extract_text( |
557 | + ... find_tag_by_id(user_browser.contents, 'step-title')) |
558 | + Step 2 (of 2): Check for duplicate projects |
559 | + |
560 | +If there are no problems with the prefilled data, then the license |
561 | +just needs to be selected. The user will then be redirected back |
562 | +to the source package page so that it can be linked. |
563 | + |
564 | + >>> user_browser.getControl(name='field.licenses').value = ['BSD'] |
565 | + >>> user_browser.getControl("Complete Registration").click() |
566 | + >>> print user_browser.url |
567 | + http://launchpad.dev/youbuntu/busy/+source/bonkers/+edit-packaging |
568 | |
569 | === modified file 'lib/lp/registry/templates/sourcepackage-edit-packaging.pt' |
570 | --- lib/lp/registry/templates/sourcepackage-edit-packaging.pt 2010-02-16 17:37:36 +0000 |
571 | +++ lib/lp/registry/templates/sourcepackage-edit-packaging.pt 2010-08-09 01:33:51 +0000 |
572 | @@ -27,6 +27,26 @@ |
573 | If you need a new series created, contact the owner of |
574 | <a tal:content="structure view/product/fmt:link"/>. |
575 | </div> |
576 | + |
577 | + <div metal:fill-slot="buttons"> |
578 | + <input tal:repeat="action view/actions" |
579 | + tal:replace="structure action/render" |
580 | + /> |
581 | + or |
582 | + <tal:comment condition="nothing"> |
583 | + This template is for a multistep view, and only the first |
584 | + step provides the register_upstream_url. |
585 | + </tal:comment> |
586 | + <a id="register-upstream-link" |
587 | + tal:condition="view/register_upstream_url | nothing" |
588 | + tal:attributes="href view/register_upstream_url"> |
589 | + Register the upstream project |
590 | + </a> |
591 | + <tal:has-cancel-link condition="view/cancel_url"> |
592 | + or |
593 | + <a tal:attributes="href view/cancel_url">Cancel</a> |
594 | + </tal:has-cancel-link> |
595 | + </div> |
596 | </div> |
597 | |
598 | </div> |
599 | |
600 | === modified file 'lib/lp/soyuz/tests/test_publishing.py' |
601 | --- lib/lp/soyuz/tests/test_publishing.py 2010-08-05 17:11:43 +0000 |
602 | +++ lib/lp/soyuz/tests/test_publishing.py 2010-08-09 01:33:51 +0000 |
603 | @@ -10,6 +10,7 @@ |
604 | from StringIO import StringIO |
605 | import tempfile |
606 | |
607 | +import transaction |
608 | import pytz |
609 | from zope.component import getUtility |
610 | from zope.security.proxy import removeSecurityProxy |
611 | @@ -18,13 +19,16 @@ |
612 | from canonical.database.constants import UTC_NOW |
613 | from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet |
614 | from canonical.launchpad.webapp.errorlog import ErrorReportingUtility |
615 | +from canonical.testing.layers import reconnect_stores |
616 | from canonical.testing import ( |
617 | DatabaseFunctionalLayer, LaunchpadZopelessLayer) |
618 | + |
619 | from lp.app.errors import NotFoundError |
620 | from lp.archivepublisher.config import Config |
621 | from lp.archivepublisher.diskpool import DiskPool |
622 | from lp.buildmaster.interfaces.buildbase import BuildStatus |
623 | from lp.registry.interfaces.distribution import IDistributionSet |
624 | +from lp.registry.interfaces.distroseries import IDistroSeriesSet |
625 | from lp.registry.interfaces.person import IPersonSet |
626 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
627 | from lp.registry.interfaces.sourcepackage import SourcePackageUrgency |
628 | @@ -460,6 +464,67 @@ |
629 | |
630 | return source |
631 | |
632 | + def makeSourcePackageWithBinaryPackageRelease(self): |
633 | + """Make test data for SourcePackage.summary. |
634 | + |
635 | + The distroseries that is returned from this method needs to be |
636 | + passed into updateDistroseriesPackageCache() so that |
637 | + SourcePackage.summary can be populated. |
638 | + """ |
639 | + distribution = self.factory.makeDistribution( |
640 | + name='youbuntu', displayname='Youbuntu') |
641 | + distroseries = self.factory.makeDistroRelease(name='busy', |
642 | + distribution=distribution) |
643 | + source_package_name = self.factory.makeSourcePackageName( |
644 | + name='bonkers') |
645 | + source_package = self.factory.makeSourcePackage( |
646 | + sourcepackagename=source_package_name, |
647 | + distroseries=distroseries) |
648 | + component = self.factory.makeComponent('multiverse') |
649 | + das = self.factory.makeDistroArchSeries( |
650 | + distroseries=distroseries) |
651 | + spph = self.factory.makeSourcePackagePublishingHistory( |
652 | + sourcepackagename=source_package_name, |
653 | + distroseries=distroseries, |
654 | + component=component) |
655 | + |
656 | + for name in ('flubber-bin', 'flubber-lib'): |
657 | + binary_package_name = self.factory.makeBinaryPackageName(name) |
658 | + build = self.factory.makeBinaryPackageBuild( |
659 | + source_package_release=spph.sourcepackagerelease, |
660 | + archive=self.factory.makeArchive(), |
661 | + distroarchseries=das) |
662 | + bpr = self.factory.makeBinaryPackageRelease( |
663 | + binarypackagename=binary_package_name, |
664 | + summary='summary for %s' % name, |
665 | + build=build, component=component) |
666 | + bpph = self.factory.makeBinaryPackagePublishingHistory( |
667 | + binarypackagerelease=bpr, distroarchseries=das) |
668 | + return dict( |
669 | + distroseries=distroseries, |
670 | + source_package=source_package) |
671 | + |
672 | + def updateDistroSeriesPackageCache( |
673 | + self, distroseries, restore_db_connection='launchpad'): |
674 | + # XXX: EdwinGrubbs 2010-08-04 bug=396419. Currently there is no |
675 | + # test api call to switchDbUser that works for non-zopeless layers. |
676 | + # When bug 396419 is fixed, we can instead use |
677 | + # DatabaseLayer.switchDbUser() instead of reconnect_stores() |
678 | + transaction.commit() |
679 | + reconnect_stores(config.statistician.dbuser) |
680 | + distroseries = getUtility(IDistroSeriesSet).get(distroseries.id) |
681 | + |
682 | + class TestLogger: |
683 | + # Silent logger. |
684 | + def debug(self, msg): |
685 | + pass |
686 | + distroseries.updateCompletePackageCache( |
687 | + archive=distroseries.distribution.main_archive, |
688 | + ztm=transaction, |
689 | + log=TestLogger()) |
690 | + transaction.commit() |
691 | + reconnect_stores(restore_db_connection) |
692 | + |
693 | |
694 | class TestNativePublishingBase(TestCaseWithFactory, SoyuzTestPublisher): |
695 | layer = LaunchpadZopelessLayer |
It would be nice to have some tests for get_register_ upstream_ url() - making sure dashes are converted properly, etc.
get_register_ upstream_ url() handles the case of source_ package. summary being None - when does that situation occur?
Since it's likely that the user doesn't want to create a project for their own page this way, it might be a good idea to enable the "I do not want to maintain this project" checkbox when registering a project this way.