Merge lp://qastaging/~sinzui/launchpad/package-link-validation-2 into lp://qastaging/launchpad
- package-link-validation-2
- Merge into devel
Proposed by
Curtis Hovey
| Status: | Merged | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Merged at revision: | not available | ||||||||||||
| Proposed branch: | lp://qastaging/~sinzui/launchpad/package-link-validation-2 | ||||||||||||
| Merge into: | lp://qastaging/launchpad | ||||||||||||
| Diff against target: |
712 lines 8 files modified
lib/lp/bugs/stories/guided-filebug/xx-project-guided-filebug.txt (+1/-1) lib/lp/registry/browser/configure.zcml (+1/-1) lib/lp/registry/browser/packaging.py (+8/-5) lib/lp/registry/browser/productseries.py (+57/-81) lib/lp/registry/browser/tests/productseries-views.txt (+132/-0) lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt (+0/-45) lib/lp/registry/stories/packaging/xx-ubuntu-pkging.txt (+0/-136) lib/lp/registry/templates/productseries-ubuntupkg.pt (+41/-46) |
||||||||||||
| To merge this branch: | bzr merge lp://qastaging/~sinzui/launchpad/package-link-validation-2 | ||||||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Edwin Grubbs (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
| Curtis Hovey (sinzui) wrote : | # |
Revision history for this message
| Edwin Grubbs (edwin-grubbs) wrote : | # |
Download full text (6.2 KiB)
Hi Curtis,
This is a huge improvement over the old form. Please try to use setFieldError()
if default_
There are a few minor comments below.
merge-conditional
-Edwin
>=== modified file 'lib/lp/
>--- lib/lp/
>+++ lib/lp/
>@@ -35,10 +35,19 @@
>
> cancel_url = next_url
>
>+ @property
>+ def default_
>+ """The default distroseries for this view; None.
>+
>+ The vocabulary guarantees a distroseries, but in subclasses that is
>+ not true.
>+ """
>+ return None
Couldn't this just be:
default_
>
> def validate(self, data):
> productseries = self.context
> sourcepackagename = data.get(
>- distroseries = data['distroser
>+ distroseries = data.get(
> if sourcepackagename is None:
> message = "You must choose the source package name."
> self.setFieldEr
>
>=== modified file 'lib/lp/
>--- lib/lp/
>+++ lib/lp/
>@@ -11,59 +11,45 @@
> <div class="
> <p>
> This page is a quick way for you to link the
>- <span tal:replace=
>- "<span tal:replace=
>- the current development series of Ubuntu. We will use this
>- information to improve the flow of bug fixes and translations
>- from Ubuntu to the
>+ <tal:series replace=
>+ the <strong>current development series of Ubuntu</strong>. We will
>+ use this information to improve the flow of bug fixes and
>+ translations from Ubuntu to the
> <span tal:replace=
> </p>
>-
>- <p class="
>- You must log in to change the mapping from this project branch to its
>- Ubuntu package.
>+
Trailing white space.
>+ <p>
>+ Use
Trailing white space.
>+ <a tal:replace=
>+ to link this series to packages in other distribution series.
> </p>
> </div>
>
> <div class="portlet">
>- <form method="POST"
>- tal:condition=
>- tal:attributes=
>-
>- <h2>
>- Package in
>- <span tal:replace=
>- The Hoary Hedgehog
>- </span>
>- </h2>
>-
>-
>- <div class="field">
>- <div>
>- <label>Source package:</label>
>-...
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/bugs/stories/guided-filebug/xx-project-guided-filebug.txt' |
| 2 | --- lib/lp/bugs/stories/guided-filebug/xx-project-guided-filebug.txt 2009-09-15 09:49:49 +0000 |
| 3 | +++ lib/lp/bugs/stories/guided-filebug/xx-project-guided-filebug.txt 2009-10-22 01:43:16 +0000 |
| 4 | @@ -230,7 +230,7 @@ |
| 5 | Finally, we add Testy Product to a source package: |
| 6 | |
| 7 | >>> admin_browser.open('http://bugs.launchpad.dev/testy/trunk/+ubuntupkg') |
| 8 | - >>> admin_browser.getControl(name='ubuntupkg').value = 'thunderbird' |
| 9 | + >>> admin_browser.getControl('Source Package Name').value = 'thunderbird' |
| 10 | >>> admin_browser.getControl('Update').click() |
| 11 | |
| 12 | And this is also shown on the filebug page: |
| 13 | |
| 14 | === modified file 'lib/lp/registry/browser/configure.zcml' |
| 15 | --- lib/lp/registry/browser/configure.zcml 2009-10-17 00:04:32 +0000 |
| 16 | +++ lib/lp/registry/browser/configure.zcml 2009-10-22 01:43:16 +0000 |
| 17 | @@ -1663,7 +1663,7 @@ |
| 18 | template="../templates/productseries-ubuntupkg.pt" |
| 19 | for="lp.registry.interfaces.productseries.IProductSeries" |
| 20 | class="lp.registry.browser.productseries.ProductSeriesUbuntuPackagingView" |
| 21 | - permission="zope.Public"/> |
| 22 | + permission="launchpad.View"/> |
| 23 | <browser:pages |
| 24 | for="lp.registry.interfaces.productseries.IProductSeries" |
| 25 | class="lp.registry.browser.productseries.ProductSeriesView" |
| 26 | |
| 27 | === modified file 'lib/lp/registry/browser/packaging.py' |
| 28 | --- lib/lp/registry/browser/packaging.py 2009-10-19 16:29:09 +0000 |
| 29 | +++ lib/lp/registry/browser/packaging.py 2009-10-22 01:43:16 +0000 |
| 30 | @@ -20,6 +20,7 @@ |
| 31 | class PackagingAddView(LaunchpadFormView): |
| 32 | schema = IPackaging |
| 33 | field_names = ['distroseries', 'sourcepackagename', 'packaging'] |
| 34 | + default_distroseries = None |
| 35 | |
| 36 | @property |
| 37 | def label(self): |
| 38 | @@ -38,7 +39,7 @@ |
| 39 | def validate(self, data): |
| 40 | productseries = self.context |
| 41 | sourcepackagename = data.get('sourcepackagename', None) |
| 42 | - distroseries = data['distroseries'] |
| 43 | + distroseries = data.get('distroseries', self.default_distroseries) |
| 44 | if sourcepackagename is None: |
| 45 | message = "You must choose the source package name." |
| 46 | self.setFieldError('sourcepackagename', message) |
| 47 | @@ -48,21 +49,23 @@ |
| 48 | sourcepackagename=sourcepackagename, |
| 49 | distroseries=distroseries): |
| 50 | # The series packaging conflicts with itself. |
| 51 | - self.addError(_( |
| 52 | + message = _( |
| 53 | "This series is already packaged in %s." % |
| 54 | - distroseries.displayname)) |
| 55 | + distroseries.displayname) |
| 56 | + self.setFieldError('sourcepackagename', message) |
| 57 | elif packaging_util.packagingEntryExists( |
| 58 | sourcepackagename=sourcepackagename, |
| 59 | distroseries=distroseries): |
| 60 | # The series package conflicts with another series. |
| 61 | sourcepackage = distroseries.getSourcePackage( |
| 62 | sourcepackagename.name) |
| 63 | - self.addError(structured( |
| 64 | + message = structured( |
| 65 | 'The <a href="%s">%s</a> package in %s is already linked to ' |
| 66 | 'another series.' % |
| 67 | (canonical_url(sourcepackage), |
| 68 | sourcepackagename.name, |
| 69 | - distroseries.displayname))) |
| 70 | + distroseries.displayname)) |
| 71 | + self.setFieldError('sourcepackagename', message) |
| 72 | else: |
| 73 | # The distroseries and sourcepackagename are not already linked |
| 74 | # to this series, or any other series. |
| 75 | |
| 76 | === modified file 'lib/lp/registry/browser/productseries.py' |
| 77 | --- lib/lp/registry/browser/productseries.py 2009-10-20 16:45:23 +0000 |
| 78 | +++ lib/lp/registry/browser/productseries.py 2009-10-22 01:43:16 +0000 |
| 79 | @@ -73,10 +73,9 @@ |
| 80 | |
| 81 | from lp.registry.browser import ( |
| 82 | MilestoneOverlayMixin, RegistryDeleteViewMixin) |
| 83 | +from lp.registry.browser.packaging import PackagingAddView |
| 84 | from lp.registry.interfaces.distroseries import DistroSeriesStatus |
| 85 | from lp.registry.interfaces.productseries import IProductSeries |
| 86 | -from lp.registry.interfaces.sourcepackagename import ( |
| 87 | - ISourcePackageNameSet) |
| 88 | |
| 89 | def quote(text): |
| 90 | """Escape and quite text.""" |
| 91 | @@ -288,88 +287,14 @@ |
| 92 | return None |
| 93 | |
| 94 | |
| 95 | -# A View Class for ProductSeries |
| 96 | -# |
| 97 | -# XXX: StuartBishop 2005-05-02: |
| 98 | -# We should be using autogenerated add forms and edit forms so that |
| 99 | -# this becomes maintainable and form validation handled for us. |
| 100 | -# Currently, the pages just return 'System Error' as they trigger database |
| 101 | -# constraints. |
| 102 | class ProductSeriesView(LaunchpadView, MilestoneOverlayMixin): |
| 103 | """A view to show a series with translations.""" |
| 104 | - def initialize(self): |
| 105 | - """See `LaunchpadFormView`.""" |
| 106 | - self.form = self.request.form |
| 107 | - self.has_errors = False |
| 108 | - |
| 109 | - # Let's find out what source package is associated with this |
| 110 | - # productseries in the current release of ubuntu. |
| 111 | - ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
| 112 | - self.curr_ubuntu_series = ubuntu.currentseries |
| 113 | - self.setUpPackaging() |
| 114 | - |
| 115 | - # Check the form submission. |
| 116 | - self.processForm() |
| 117 | |
| 118 | @property |
| 119 | def page_title(self): |
| 120 | """Return the HTML page title.""" |
| 121 | return self.context.title |
| 122 | |
| 123 | - def processForm(self): |
| 124 | - """Process a form if it was submitted.""" |
| 125 | - if not self.request.method == "POST": |
| 126 | - # The form was not posted, we don't do anything. |
| 127 | - return |
| 128 | - assert 'set_ubuntu_pkg' in self.form, ( |
| 129 | - "This can handle POST requests only for 'set_ubuntu_pkg' form.") |
| 130 | - self.setCurrentUbuntuPackage() |
| 131 | - |
| 132 | - def setUpPackaging(self): |
| 133 | - """Ensure that the View class correctly reflects the packaging of |
| 134 | - its product series context.""" |
| 135 | - self.curr_ubuntu_package = None |
| 136 | - self.curr_ubuntu_pkgname = '' |
| 137 | - try: |
| 138 | - cr = self.curr_ubuntu_series |
| 139 | - self.curr_ubuntu_package = self.context.getPackage(cr) |
| 140 | - cp = self.curr_ubuntu_package |
| 141 | - self.curr_ubuntu_pkgname = cp.sourcepackagename.name |
| 142 | - except NotFoundError: |
| 143 | - pass |
| 144 | - ubuntu = self.curr_ubuntu_series.distribution |
| 145 | - self.ubuntu_history = self.context.getPackagingInDistribution(ubuntu) |
| 146 | - |
| 147 | - def setCurrentUbuntuPackage(self): |
| 148 | - """Set the Packaging record for this product series in the current |
| 149 | - Ubuntu distroseries to be for the source package name that is given |
| 150 | - in the form. |
| 151 | - """ |
| 152 | - ubuntupkg = self.form.get('ubuntupkg', '') |
| 153 | - if ubuntupkg == '': |
| 154 | - # No package was selected. |
| 155 | - self.request.response.addWarningNotification( |
| 156 | - 'Request ignored. You need to select a source package.') |
| 157 | - return |
| 158 | - # make sure we have a person to work with |
| 159 | - if self.user is None: |
| 160 | - self.request.response.addErrorNotification('Please log in first!') |
| 161 | - self.has_errors = True |
| 162 | - return |
| 163 | - # see if the name that is given is a real source package name |
| 164 | - spns = getUtility(ISourcePackageNameSet) |
| 165 | - try: |
| 166 | - spn = spns[ubuntupkg] |
| 167 | - except NotFoundError: |
| 168 | - self.request.response.addErrorNotification( |
| 169 | - 'Invalid source package name %s' % ubuntupkg) |
| 170 | - self.has_errors = True |
| 171 | - return |
| 172 | - # set the packaging record for this productseries in the current |
| 173 | - # ubuntu series. if none exists, one will be created |
| 174 | - self.context.setPackaging(self.curr_ubuntu_series, spn, self.user) |
| 175 | - self.setUpPackaging() |
| 176 | - |
| 177 | def requestCountry(self): |
| 178 | """The country associated with the IP of the request.""" |
| 179 | return ICountry(self.request, None) |
| 180 | @@ -426,7 +351,6 @@ |
| 181 | for status in sorted(status_counts, |
| 182 | key=attrgetter('sortkey'))] |
| 183 | |
| 184 | - |
| 185 | @property |
| 186 | def latest_release_with_download_files(self): |
| 187 | for release in self.context.releases: |
| 188 | @@ -435,10 +359,62 @@ |
| 189 | return None |
| 190 | |
| 191 | |
| 192 | -class ProductSeriesUbuntuPackagingView(ProductSeriesView): |
| 193 | - """A view to show series package in Ubuntu.""" |
| 194 | - |
| 195 | - label = 'Ubuntu source packaging' |
| 196 | +class ProductSeriesUbuntuPackagingView(PackagingAddView): |
| 197 | + |
| 198 | + field_names = ['sourcepackagename'] |
| 199 | + page_title = 'Ubuntu source packaging' |
| 200 | + label = page_title |
| 201 | + |
| 202 | + def __init__(self, context, request): |
| 203 | + """Set the staic packaging information for this series.""" |
| 204 | + super(ProductSeriesUbuntuPackagingView, self).__init__( |
| 205 | + context, request) |
| 206 | + ubuntu = getUtility(ILaunchpadCelebrities).ubuntu |
| 207 | + self._ubuntu_series = ubuntu.currentseries |
| 208 | + try: |
| 209 | + package = self.context.getPackage(self._ubuntu_series) |
| 210 | + self.default_sourcepackagename = package.sourcepackagename |
| 211 | + except NotFoundError: |
| 212 | + # The package has never been set. |
| 213 | + self.default_sourcepackagename = None |
| 214 | + |
| 215 | + def setUpWidgets(self): |
| 216 | + """See `LaunchpadFormView`. |
| 217 | + |
| 218 | + Set the current `ISourcePackageName` as the default value. |
| 219 | + """ |
| 220 | + super(ProductSeriesUbuntuPackagingView, self).setUpWidgets() |
| 221 | + if self.default_sourcepackagename is not None: |
| 222 | + widget = self.widgets.get('sourcepackagename') |
| 223 | + widget.setRenderedValue(self.default_sourcepackagename) |
| 224 | + |
| 225 | + @property |
| 226 | + def default_distroseries(self): |
| 227 | + """The current Ubuntu distroseries""" |
| 228 | + return self._ubuntu_series |
| 229 | + |
| 230 | + @property |
| 231 | + def ubuntu_history(self): |
| 232 | + return self.context.getPackagingInDistribution( |
| 233 | + self.default_distroseries.distribution) |
| 234 | + |
| 235 | + def validate(self, data): |
| 236 | + sourcepackagename = data.get('sourcepackagename', None) |
| 237 | + if sourcepackagename == self.default_sourcepackagename: |
| 238 | + # The data has not changed, so nothing else needs to be done. |
| 239 | + return |
| 240 | + super(ProductSeriesUbuntuPackagingView, self).validate(data) |
| 241 | + |
| 242 | + @action('Update', name='continue') |
| 243 | + def continue_action(self, action, data): |
| 244 | + # set the packaging record for this productseries in the current |
| 245 | + # ubuntu series. if none exists, one will be created |
| 246 | + sourcepackagename = data['sourcepackagename'] |
| 247 | + if self.default_sourcepackagename == sourcepackagename: |
| 248 | + # There is no change. |
| 249 | + return |
| 250 | + self.context.setPackaging( |
| 251 | + self.default_distroseries, sourcepackagename, self.user) |
| 252 | |
| 253 | |
| 254 | class ProductSeriesEditView(LaunchpadEditFormView): |
| 255 | |
| 256 | === modified file 'lib/lp/registry/browser/tests/productseries-views.txt' |
| 257 | --- lib/lp/registry/browser/tests/productseries-views.txt 2009-10-19 14:52:30 +0000 |
| 258 | +++ lib/lp/registry/browser/tests/productseries-views.txt 2009-10-22 01:43:16 +0000 |
| 259 | @@ -420,3 +420,135 @@ |
| 260 | ... print error |
| 261 | ('sourcepackagename', u'Source Package Name', RequiredMissing()) |
| 262 | You must choose the source package name. |
| 263 | + |
| 264 | +The +addpackage view provides the default_distroseries property. It is None |
| 265 | +by default, but subclasses may change it. |
| 266 | + |
| 267 | + >>> print view.default_distroseries |
| 268 | + None |
| 269 | + |
| 270 | + |
| 271 | +Linking Ubuntu packages |
| 272 | +----------------------- |
| 273 | + |
| 274 | +The +ubuntupkg named view is a subclass of the +addpackage named view. It |
| 275 | +allows the user to update the current linked Ubuntu package. |
| 276 | + |
| 277 | + >>> from lp.registry.browser.packaging import PackagingAddView |
| 278 | + |
| 279 | + >>> view = create_initialized_view(productseries, '+ubuntupkg') |
| 280 | + >>> isinstance(view, PackagingAddView) |
| 281 | + True |
| 282 | + |
| 283 | + >>> print view.label |
| 284 | + Ubuntu source packaging |
| 285 | + |
| 286 | + >>> print view.page_title |
| 287 | + Ubuntu source packaging |
| 288 | + |
| 289 | + >>> print view.field_names |
| 290 | + ['sourcepackagename'] |
| 291 | + |
| 292 | + >>> print view.cancel_url |
| 293 | + http://launchpad.dev/hot/hotter |
| 294 | + |
| 295 | +The view restricts the packaging to the current Ubuntu series. |
| 296 | + |
| 297 | + >>> print view.default_distroseries.name |
| 298 | + hoary |
| 299 | + |
| 300 | +The sourcepackagename is None if the package link was never set. The view's |
| 301 | +packaging history is empty, and the sourcepackagename widget is empty. |
| 302 | + |
| 303 | + >>> new_productseries = factory.makeProductSeries( |
| 304 | + ... product=product, name='cold') |
| 305 | + >>> view = create_initialized_view(new_productseries, '+ubuntupkg') |
| 306 | + |
| 307 | + >>> print view.default_sourcepackagename |
| 308 | + None |
| 309 | + |
| 310 | + >>> print view.widgets.get('sourcepackagename')._getFormValue() |
| 311 | + <BLANKLINE> |
| 312 | + |
| 313 | + >>> print view.ubuntu_history |
| 314 | + [] |
| 315 | + |
| 316 | +Series have been packaged in Ubuntu do have the current information and |
| 317 | +a history. |
| 318 | + |
| 319 | + >>> view = create_initialized_view(productseries, '+ubuntupkg') |
| 320 | + >>> print view.default_sourcepackagename.name |
| 321 | + hot |
| 322 | + |
| 323 | + >>> print view.widgets.get('sourcepackagename')._getFormValue().name |
| 324 | + hot |
| 325 | + |
| 326 | + >>> for packaging in view.ubuntu_history: |
| 327 | + ... print packaging.distroseries.name |
| 328 | + ... print packaging.sourcepackagename.name |
| 329 | + hoary hot |
| 330 | + |
| 331 | +The package in the current Ubuntu series can be updated. |
| 332 | + |
| 333 | + >>> form = { |
| 334 | + ... 'field.sourcepackagename': 'thunderbird', |
| 335 | + ... 'field.actions.continue': 'Update', |
| 336 | + ... } |
| 337 | + >>> view = create_initialized_view( |
| 338 | + ... productseries, '+ubuntupkg', form=form) |
| 339 | + >>> view.errors |
| 340 | + [] |
| 341 | + |
| 342 | + >>> for packaging in view.ubuntu_history: |
| 343 | + ... print packaging.distroseries.name |
| 344 | + ... print packaging.sourcepackagename.name |
| 345 | + hoary thunderbird |
| 346 | + |
| 347 | +It is not an error to submit the same sourcepackagename information, the |
| 348 | +action is ignored because there is no change |
| 349 | + |
| 350 | + >>> form = { |
| 351 | + ... 'field.sourcepackagename': 'thunderbird', |
| 352 | + ... 'field.actions.continue': 'Update', |
| 353 | + ... } |
| 354 | + >>> view = create_initialized_view( |
| 355 | + ... productseries, '+ubuntupkg', form=form) |
| 356 | + >>> view.errors |
| 357 | + [] |
| 358 | + |
| 359 | + >>> for packaging in view.ubuntu_history: |
| 360 | + ... print packaging.distroseries.name |
| 361 | + ... print packaging.sourcepackagename.name |
| 362 | + hoary thunderbird |
| 363 | + |
| 364 | +When the current Ubuntu series changes, the sourcepackagename is not known, |
| 365 | +and a new entry can be added to the packaging history. |
| 366 | + |
| 367 | + >>> from lp.registry.interfaces.distroseries import DistroSeriesStatus |
| 368 | + |
| 369 | + >>> login('admin@canonical.com') |
| 370 | + >>> hoary.status = DistroSeriesStatus.CURRENT |
| 371 | + >>> grumpy_series = ubuntu.getSeries('grumpy') |
| 372 | + >>> grumpy_series.status = DistroSeriesStatus.FROZEN |
| 373 | + |
| 374 | + >>> login_person(a_user) |
| 375 | + >>> form = { |
| 376 | + ... 'field.sourcepackagename': 'hot', |
| 377 | + ... 'field.actions.continue': 'Update', |
| 378 | + ... } |
| 379 | + >>> view = create_initialized_view( |
| 380 | + ... productseries, '+ubuntupkg', form=form) |
| 381 | + >>> view.errors |
| 382 | + [] |
| 383 | + |
| 384 | + >>> print view.default_distroseries.name |
| 385 | + grumpy |
| 386 | + |
| 387 | + >>> print view.default_sourcepackagename |
| 388 | + None |
| 389 | + |
| 390 | + >>> for packaging in view.ubuntu_history: |
| 391 | + ... print packaging.distroseries.name |
| 392 | + ... print packaging.sourcepackagename.name |
| 393 | + grumpy hot |
| 394 | + hoary thunderbird |
| 395 | |
| 396 | === modified file 'lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt' |
| 397 | --- lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt 2009-08-27 20:42:12 +0000 |
| 398 | +++ lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt 2009-10-22 01:43:16 +0000 |
| 399 | @@ -52,48 +52,3 @@ |
| 400 | The Warty Warthog Release (current stable release) Set upstream link |
| 401 | 1.0.8-1ubuntu1 release (main) ... weeks ago |
| 402 | 1.0.9a-4 release (main) ... weeks ago |
| 403 | - |
| 404 | - |
| 405 | -== Multiple package links == |
| 406 | - |
| 407 | -It is possible for multiple product series to be linked to the same |
| 408 | -sourcepackage. When this is the case, only the latest is displayed and |
| 409 | -will be deleted. |
| 410 | - |
| 411 | -(Link evolution source package to a new series.) |
| 412 | - |
| 413 | - >>> admin_browser.open('http://launchpad.dev/evolution/+addseries') |
| 414 | - >>> admin_browser.getControl('Name').value = 'no-crashes' |
| 415 | - >>> admin_browser.getControl('Summary').value = ( |
| 416 | - ... 'The series that everybody is waiting for!') |
| 417 | - >>> admin_browser.getControl('Register Series').click() |
| 418 | - |
| 419 | - >>> user_browser.open( |
| 420 | - ... 'http://launchpad.dev/evolution/no-crashes/+ubuntupkg') |
| 421 | - >>> user_browser.getControl(name='ubuntupkg').value = 'evolution' |
| 422 | - >>> user_browser.getControl('Update').click() |
| 423 | - |
| 424 | - |
| 425 | -When the user visits the source package page, only the latest link is |
| 426 | -displayed: |
| 427 | - |
| 428 | - >>> user_browser.open( |
| 429 | - ... 'http://launchpad.dev/ubuntu/+source/evolution') |
| 430 | - >>> print extract_text(find_tag_by_id( |
| 431 | - ... user_browser.contents, 'packages_list')) |
| 432 | - The Hoary Hedgehog Release (active development) Evolution no-crashes series |
| 433 | - 1.0 release (main) ... weeks ago |
| 434 | - |
| 435 | - |
| 436 | -Once he deletes the link, the older link shows: |
| 437 | - |
| 438 | - >>> form = user_browser.getForm("delete_hoary_evolution_no-crashes") |
| 439 | - >>> form.getControl("Delete Link").click() |
| 440 | - >>> for message in get_feedback_messages(user_browser.contents): |
| 441 | - ... print message |
| 442 | - Removed upstream association between Evolution no-crashes and Hoary. |
| 443 | - |
| 444 | - >>> print extract_text( |
| 445 | - ... find_tag_by_id(user_browser.contents, 'packages_list')) |
| 446 | - The Hoary Hedgehog Release (active development) Evolution trunk series |
| 447 | - 1.0 release (main) ... weeks ago |
| 448 | |
| 449 | === removed file 'lib/lp/registry/stories/packaging/xx-ubuntu-pkging.txt' |
| 450 | --- lib/lp/registry/stories/packaging/xx-ubuntu-pkging.txt 2009-10-16 15:00:55 +0000 |
| 451 | +++ lib/lp/registry/stories/packaging/xx-ubuntu-pkging.txt 1970-01-01 00:00:00 +0000 |
| 452 | @@ -1,136 +0,0 @@ |
| 453 | -Ubuntu packaging |
| 454 | -================ |
| 455 | - |
| 456 | -We want to be able to edit the ubuntu packaging for a product series, |
| 457 | -and we also want to be able to create it where there was none |
| 458 | -previously. |
| 459 | - |
| 460 | -first, make sure we can see a page for this on the trunk series of |
| 461 | -evolution. The form field shows that it is currently mapped to |
| 462 | -evolution. |
| 463 | - |
| 464 | - >>> print http(r""" |
| 465 | - ... GET /evolution/trunk/+ubuntupkg HTTP/1.1 |
| 466 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 467 | - ... """) |
| 468 | - HTTP/1.1 200 Ok |
| 469 | - ... |
| 470 | - <h1>Ubuntu source packaging</h1> |
| 471 | - ... |
| 472 | - <form method="POST" |
| 473 | - action="http://localhost/evolution/trunk/+ubuntupkg"> |
| 474 | - ... |
| 475 | - <input type="text" name="ubuntupkg" size="30" |
| 476 | - value="evolution" /> |
| 477 | - ... |
| 478 | - |
| 479 | -Now let's POST to that form, we'll say we are linking it to package |
| 480 | -pmount, which is clearly not correct! In the resulting page, you can see |
| 481 | -that the form now says pmount, and there is a new row in the history |
| 482 | -that includes it. |
| 483 | - |
| 484 | - >>> print http(r""" |
| 485 | - ... POST /evolution/trunk/+ubuntupkg HTTP/1.1 |
| 486 | - ... Content-Length: 36 |
| 487 | - ... Content-Type: application/x-www-form-urlencoded |
| 488 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 489 | - ... |
| 490 | - ... ubuntupkg=pmount&set_ubuntu_pkg=Update""") |
| 491 | - HTTP/1.1 200 Ok |
| 492 | - Content-Length: ... |
| 493 | - Content-Type: text/html;charset=utf-8 |
| 494 | - <BLANKLINE> |
| 495 | - ... |
| 496 | - <input type="text" name="ubuntupkg" size="30" |
| 497 | - value="pmount" /> |
| 498 | - ... |
| 499 | - |
| 500 | -OK, if that worked, we better set it back to where it was so we restore |
| 501 | -balance to the Force... |
| 502 | - |
| 503 | - >>> print http(r""" |
| 504 | - ... POST /evolution/trunk/+ubuntupkg HTTP/1.1 |
| 505 | - ... Content-Length: 39 |
| 506 | - ... Content-Type: application/x-www-form-urlencoded |
| 507 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 508 | - ... |
| 509 | - ... ubuntupkg=evolution&set_ubuntu_pkg=Update""") |
| 510 | - HTTP/1.1 200 Ok |
| 511 | - Content-Length: ... |
| 512 | - Content-Type: text/html;charset=utf-8 |
| 513 | - <BLANKLINE> |
| 514 | - ... |
| 515 | - <input type="text" name="ubuntupkg" size="30" |
| 516 | - value="evolution" /> |
| 517 | - ... |
| 518 | - |
| 519 | -Now lets make sure we can create a new ubuntu package relationship where |
| 520 | -none existed previously. First make sure none exists, by viewing the |
| 521 | -form, it should have an empty field for ubuntupkg. |
| 522 | - |
| 523 | - >>> print http(r""" |
| 524 | - ... GET /thunderbird/trunk/+ubuntupkg HTTP/1.1 |
| 525 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 526 | - ... """) |
| 527 | - HTTP/1.1 200 Ok |
| 528 | - Content-Length: ... |
| 529 | - Content-Type: text/html;charset=utf-8 |
| 530 | - <BLANKLINE> |
| 531 | - ... |
| 532 | - <input type="text" name="ubuntupkg" size="30" |
| 533 | - value="" /> |
| 534 | - ... |
| 535 | - |
| 536 | -Trying to add a Source Package with an non-existent name should fail |
| 537 | -graciously. |
| 538 | - |
| 539 | - >>> print http(r""" |
| 540 | - ... POST /thunderbird/trunk/+ubuntupkg HTTP/1.1 |
| 541 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 542 | - ... Content-Length: 37 |
| 543 | - ... Content-Type: application/x-www-form-urlencoded |
| 544 | - ... |
| 545 | - ... ubuntupkg=moz&set_ubuntu_pkg=Update""") |
| 546 | - HTTP/1.1 200 Ok |
| 547 | - ... |
| 548 | - ...<div class="error message">Invalid source package name moz</div>... |
| 549 | - ... |
| 550 | - <input type="text" name="ubuntupkg" size="30" |
| 551 | - value="" /> |
| 552 | - ... |
| 553 | - |
| 554 | -Now let's post to it. We'll lie and say it's the mozilla source package |
| 555 | -in the current ubuntu distroseries. |
| 556 | - |
| 557 | - >>> print http(r""" |
| 558 | - ... POST /thunderbird/trunk/+ubuntupkg HTTP/1.1 |
| 559 | - ... Authorization: Basic Y2FybG9zQGNhbm9uaWNhbC5jb206dGVzdA== |
| 560 | - ... Content-Length: 37 |
| 561 | - ... Content-Type: application/x-www-form-urlencoded |
| 562 | - ... |
| 563 | - ... ubuntupkg=mozilla&set_ubuntu_pkg=Update""") |
| 564 | - HTTP/1.1 200 Ok |
| 565 | - Content-Length: ... |
| 566 | - Content-Type: text/html;charset=utf-8 |
| 567 | - <BLANKLINE> |
| 568 | - ... |
| 569 | - <input type="text" name="ubuntupkg" size="30" |
| 570 | - value="mozilla" /> |
| 571 | - ... |
| 572 | - |
| 573 | -Markup in the source package name is escaped in error notices to |
| 574 | -prevent XSS markup injection. No Privileges Person cannot insert an |
| 575 | -executable script into the page using an invalid source package name. |
| 576 | - |
| 577 | - >>> user_browser.open('http://launchpad.dev/bzr/trunk/') |
| 578 | - >>> user_browser.getLink('Link to Ubuntu package').click() |
| 579 | - >>> print user_browser.title |
| 580 | - Bazaar trunk... |
| 581 | - |
| 582 | - >>> user_browser.getControl(name='ubuntupkg').value = ( |
| 583 | - ... "bzr<script>window.alert('XSS')</script>") |
| 584 | - >>> user_browser.getControl('Update').click() |
| 585 | - >>> for message in get_feedback_messages(user_browser.contents): |
| 586 | - ... print message |
| 587 | - Invalid source package name |
| 588 | - bzr<script>window.alert('XSS')</script> |
| 589 | |
| 590 | === modified file 'lib/lp/registry/templates/productseries-ubuntupkg.pt' |
| 591 | --- lib/lp/registry/templates/productseries-ubuntupkg.pt 2009-09-15 02:16:19 +0000 |
| 592 | +++ lib/lp/registry/templates/productseries-ubuntupkg.pt 2009-10-22 01:43:16 +0000 |
| 593 | @@ -11,59 +11,45 @@ |
| 594 | <div class="top-portlet"> |
| 595 | <p> |
| 596 | This page is a quick way for you to link the |
| 597 | - <span tal:replace="context/product/displayname">Firefox</span> |
| 598 | - "<span tal:replace="context/name">1.0</span>" series to a package in |
| 599 | - the current development series of Ubuntu. We will use this |
| 600 | - information to improve the flow of bug fixes and translations |
| 601 | - from Ubuntu to the |
| 602 | + <tal:series replace="context/title" /> to a package in |
| 603 | + the <strong>current development series of Ubuntu</strong>. We will |
| 604 | + use this information to improve the flow of bug fixes and |
| 605 | + translations from Ubuntu to the |
| 606 | <span tal:replace="context/product/displayname">Firefox</span> team. |
| 607 | </p> |
| 608 | |
| 609 | - <p class="informational message" tal:condition="not:request/lp:person"> |
| 610 | - You must log in to change the mapping from this project branch to its |
| 611 | - Ubuntu package. |
| 612 | + <p> |
| 613 | + Use |
| 614 | + <a tal:replace="structure context/menu:overview/add_package/fmt:link" /> |
| 615 | + to link this series to packages in other distribution series. |
| 616 | </p> |
| 617 | </div> |
| 618 | |
| 619 | <div class="portlet"> |
| 620 | - <form method="POST" |
| 621 | - tal:condition="request/lp:person" |
| 622 | - tal:attributes="action request/getURL"> |
| 623 | - |
| 624 | - <h2> |
| 625 | - Package in |
| 626 | - <span tal:replace="view/curr_ubuntu_series/title"> |
| 627 | - The Hoary Hedgehog |
| 628 | - </span> |
| 629 | - </h2> |
| 630 | - |
| 631 | - |
| 632 | - <div class="field"> |
| 633 | - <div> |
| 634 | - <label>Source package:</label> |
| 635 | - <span class="fieldRequired">(Required)</span> |
| 636 | - </div> |
| 637 | - <div> |
| 638 | - <input type="text" name="ubuntupkg" size="30" |
| 639 | - value="" tal:attributes="value view/curr_ubuntu_pkgname" /> |
| 640 | - <input type="submit" name="set_ubuntu_pkg" value="Update" /> |
| 641 | - </div> |
| 642 | - |
| 643 | - <p class="formHelp"> |
| 644 | - Ensure both the series ("<span tal:replace="context/name"> |
| 645 | - 1.0</span>") and the Ubuntu package are exactly correct, |
| 646 | - so that bugs, patches and translations are correctly shared. |
| 647 | - Don’t link "HEAD", "MAIN", or "TRUNK" to an Ubuntu |
| 648 | - package unless you are absolutely certain that the package is |
| 649 | - really based on the trunk of development. |
| 650 | - </p> |
| 651 | + <div metal:use-macro="context/@@launchpad_form/form"> |
| 652 | + <div metal:fill-slot="extra_info"> |
| 653 | + <h2> |
| 654 | + Package in |
| 655 | + <span tal:replace="view/default_distroseries/title" /> |
| 656 | + </h2> |
| 657 | + |
| 658 | + <p> |
| 659 | + Ensure both the <tal:series replace="context/title" /> and the |
| 660 | + Ubuntu package are exactly correct. Don’t link "HEAD", |
| 661 | + "MAIN", or "TRUNK" to an Ubuntu package unless you are |
| 662 | + absolutely certain that the package is really based on the |
| 663 | + trunk of development. |
| 664 | + </p> |
| 665 | </div> |
| 666 | - </form> |
| 667 | + </div> |
| 668 | </div> |
| 669 | |
| 670 | <div class="portlet" |
| 671 | tal:condition="view/ubuntu_history"> |
| 672 | - <h2>History of Ubuntu packages for this branch</h2> |
| 673 | + <h2> |
| 674 | + History of Ubuntu packages for |
| 675 | + <tal:series replace="context/title" /> |
| 676 | + </h2> |
| 677 | |
| 678 | <table class="listing"> |
| 679 | <thead> |
| 680 | @@ -74,6 +60,7 @@ |
| 681 | <th>By</th> |
| 682 | </tr> |
| 683 | </thead> |
| 684 | + |
| 685 | <tbody> |
| 686 | <tr tal:repeat="packaging view/ubuntu_history"> |
| 687 | <td> |
| 688 | @@ -83,11 +70,19 @@ |
| 689 | 13-05-2004 |
| 690 | </span> |
| 691 | </td> |
| 692 | - <td tal:content="packaging/distroseries/name">hoary</td> |
| 693 | - <td tal:content="packaging/sourcepackagename/name">evolution</td> |
| 694 | - <td><span tal:condition="packaging/owner" |
| 695 | - tal:replace="packaging/owner/displayname">mark |
| 696 | - </span></td> |
| 697 | + <td> |
| 698 | + <a tal:replace="structure packaging/distroseries/fmt:link" /> |
| 699 | + </td> |
| 700 | + <td> |
| 701 | + <a |
| 702 | + tal:attributes="href string:${packaging/distroseries/fmt:url}/+source/${packaging/sourcepackagename/name}" |
| 703 | + tal:content="packaging/sourcepackagename/name" /> |
| 704 | + </td> |
| 705 | + <td> |
| 706 | + <a |
| 707 | + tal:condition="packaging/owner" |
| 708 | + tal:replace="structure packaging/owner/fmt:link" /> |
| 709 | + </td> |
| 710 | </tr> |
| 711 | </tbody> |
| 712 | </table> |
This is my third branch to ensure valid upstream package links. There
are many oopses relating to the creation and efforts to fix invalid packages.
The root cause is a bad DB constraint and two views that do not do the
required sanity checks: +addpackage and +ubuntupkg
This branch fixes +ubuntupkg to make sane packages.
lp:~sinzui/launchpad/package-link-validation-2
Diff size: 677
Launchpad bug: https:/
https:/
https:/
Test command: ./bin/test -vv -t 'lp.(reg|
Pre-
Target release: 3.1.10
== Fixing upstream packaging links ==
Bug 456430 [+ubuntupkg can create duplicate packaging links]
The view does not verify that the distroseries + sourcepackage is not
already used. The user is either making a mistake, or the user needs to
correct the mistake before the link can be made. This code is embedded
into the ProductSeriesView. We want this to be a LaunchpadFormView, and it
probably should extend +addpackage view to ensure the rules are correct.
There are two key differences that this +ubuntupkg does differently than
+addpackage. It restricting the distroseries to the Ubuntu focus of
development, and it records the change in the history.
Bug 447565 [+ubuntupkg should link the source packages it lists]
The distroseries should also be linked.
Bug 83357 [Link to Ubuntu Package" doesn't allow choosing a release]
Adding a link to +addpackage will fix this.
== Rules ==
Bug 456430 [+ubuntupkg can create duplicate packaging links]
* Extract the +ubuntupkg code from ProductSeriesView.
* Make the ProductSeriesUb
Bug #447565 [+ubuntupkg should link the source packages it lists]
* Link the distroseries and packages in the table.
Bug 83357 [Link to Ubuntu Package" doesn't allow choosing a release]
* Add a link to +addpackage to the page.
== QA ==
On staging
* Visit a productseries
* Choose (+) Ubuntu packaging
* Verify there is a link to +addpackage
* Verify the table items are linked.
* Submit the form with 'gedit'
* Verify the form error message explains that the gedit is already
packaged in Ubuntu karmic. Follow the link
* Verify the “gedit” source package in Karmic page displays.
== Lint ==
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
== Test ==
* lib/lp/
* Removed test because it was not a story, it was testing the form
contract, it was testing behaviours that are guaranteed though
packaging tests in registry and soyuz.
* lib/lp/