Code review comment for lp:~sinzui/launchpad/non-existent-packages-bug-204119

Hi Curtis,

Thanks for the branch. It looks good save for a few small items below.

> === modified file 'lib/lp/registry/browser/productseries.py'
> --- lib/lp/registry/browser/productseries.py 2009-12-05 18:37:28 +0000
> +++ lib/lp/registry/browser/productseries.py 2010-01-11 23:10:26 +0000
> @@ -33,6 +33,9 @@
>
> from zope.component import getUtility
> from zope.app.form.browser import TextAreaWidget, TextWidget
> +from zope.formlib import form
> +from zope.schema import Choice
> +from zope.schema.vocabulary import SimpleTerm, SimpleVocabulary
>
> from z3c.ptcompat import ViewPageTemplateFile
>
> @@ -366,7 +369,7 @@
>
> class ProductSeriesUbuntuPackagingView(PackagingAddView):
>
> - field_names = ['sourcepackagename']
> + field_names = ['sourcepackagename', 'distroseries']
> page_title = 'Ubuntu source packaging'
> label = page_title
>
> @@ -374,8 +377,8 @@
> """Set the static packaging information for this series."""
> super(ProductSeriesUbuntuPackagingView, self).__init__(
> context, request)
> - ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
> - self._ubuntu_series = ubuntu.currentseries
> + self._ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
> + self._ubuntu_series = self._ubuntu.currentseries
> try:
> package = self.context.getPackage(self._ubuntu_series)
> self.default_sourcepackagename = package.sourcepackagename
> @@ -383,6 +386,28 @@
> # The package has never been set.
> self.default_sourcepackagename = None
>
> + def setUpFields(self):
> + """See `LaunchpadFormView`.
> +
> + The packaging is restricted to ubuntu series and the default value
> + is the current development series.
> + """
> + super(PackagingAddView, self).setUpFields()

As we discussed on IRC the class name is wrong.

> + series_vocabulary = SimpleVocabulary(
> + [SimpleTerm(series, series.name, series.named_version)
> + for series in self._ubuntu.series])
> + choice = Choice(__name__='distroseries',
> + title=_('Series'),
> + default=self._ubuntu_series,
> + vocabulary=series_vocabulary,
> + description=_(
> + "Select the Ubuntu series that this package is published "
> + "in. The current series is most important to the Ubuntu "
> + "community."),

I think "series where this package is published" sounds better. What
do you think?

> + required=True)
> + field = form.Fields(choice, render_context=self.render_context)
> + self.form_fields = self.form_fields.omit(choice.__name__) + field
> +
> def setUpWidgets(self):
> """See `LaunchpadFormView`.
>

> === modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
> --- lib/lp/registry/browser/tests/packaging-views.txt 2009-12-10 20:03:11 +0000
> +++ lib/lp/registry/browser/tests/packaging-views.txt 2010-01-11 23:10:26 +0000
> @@ -18,6 +18,8 @@
> >>> sourcepackagename = factory.makeSourcePackageName('hot')
> >>> sourcepackage = factory.makeSourcePackage(
> ... sourcepackagename=sourcepackagename, distroseries=hoary)
> + >>> spph = factory.makeSourcePackagePublishingHistory(
> + ... sourcepackagename=sourcepackagename, distroseries=hoary)
> >>> product = factory.makeProduct(name="hot", displayname='Hot')
> >>> productseries = factory.makeProductSeries(
> ... product=product, name='hotter')
> @@ -103,6 +105,22 @@
> ('sourcepackagename', u'Source Package Name', RequiredMissing())
> You must choose the source package name.
>
> +In the case of full functionality distributions like Ubuntu, the source
> +package must be published in the distro series.
> +
> + >>> vapour_spn = factory.makeSourcePackageName('vapour')

Would you mind en_US'ing this to 'vapor'?

> === modified file 'lib/lp/registry/stories/product/xx-product-package-pages.txt'
> --- lib/lp/registry/stories/product/xx-product-package-pages.txt 2009-11-12 23:24:53 +0000
> +++ lib/lp/registry/stories/product/xx-product-package-pages.txt 2010-01-11 23:10:26 +0000
> @@ -35,14 +35,15 @@
>
> >>> evo_owner.getLink(url='/ubuntu/hoary/+source/evolution') is not None
> True
> - >>> evo_owner.getLink(url='/evolution/trunk/+addpackage') is not None
> - True
> -
> -If you're not the owner or an admin, you still see the packages, but without
> -the link to add packages yourself.
> -
> - >>> browser.open('http://launchpad.dev/evolution/+packages')
> - >>> browser.getLink(url='/evolution/trunk/+addpackage')
> +
> +Any login users can still see the links to create a packaging link.

s/login/logged in/

> + >>> user_browser.open('http://launchpad.dev/evolution/+packages')
> + >>> print user_browser.getLink(url='/evolution/trunk/+ubuntupkg').url
> + http://launchpad.dev/evolution/trunk/+ubuntupkg
> +
> + >>> anon_browser.open('http://launchpad.dev/evolution/+packages')
> + >>> anon_browser.getLink(url='/evolution/trunk/+ubuntupkg')
> Traceback (most recent call last):
> ...
> LinkNotFoundError