Merge lp:~edwin-grubbs/launchpad/bug-553384-deactivated-project-oops into lp:launchpad

[Edwin Grubbs]

Don't let a project linked to source packages be deactivated, since it will cause an oops on the $sourcepackage/+edit-packaging page.

Summary
-------

Fixed bug 553384 and bug 140526.

The $sourcepackage/+edit-packaging has an oops when it is linked to an
inactive project. To prevent this, it should not be possible to deactivate
a project until all its links to source packages have been removed.

Implementation details
----------------------

Don't allow deactivation of products linked to source packages in the
model or in the views.
    lib/lp/registry/model/product.py
    lib/lp/registry/browser/product.py
    lib/lp/registry/browser/tests/product-views.txt
    lib/lp/registry/tests/test_product.py

Removed workaround for bug 140526:
    lib/lp/bugs/browser/bugalsoaffects.py
    lib/lp/bugs/browser/tests/bugtask-adding-views.txt

Fixed tests:
    lib/lp/testing/__init__.py
    lib/lp/answers/doc/person.txt
    lib/lp/blueprints/doc/specification.txt
    lib/lp/blueprints/doc/sprint.txt
    lib/lp/registry/doc/milestone.txt
    lib/lp/registry/doc/person.txt
    lib/lp/registry/doc/pillar.txt
    lib/lp/registry/doc/product.txt
    lib/lp/registry/doc/project.txt
    lib/lp/registry/stories/pillar/xx-pillar-deactivation.txt
    lib/lp/registry/stories/product/xx-product-edit.txt
    lib/lp/registry/stories/project/xx-project-index.txt
    lib/lp/translations/doc/translationimportqueue.txt
    lib/lp/translations/stories/translationgroups/30-show-group-translation-targets.txt

Tests
-----

./bin/test -vv -t 'test_product|product-views.txt|xx-product-edit|doc/product.txt'

Demo and Q/A
------------

* Open http://launchpad.dev/firefox/+review-license
  * Uncheck the "active" input.
  * Click "Change"
  * The "active" field should have the error message:
    "This project cannot be deactivated since it is still linked to
    source packages."
* Open http://launchpad.dev/firefox/+admin
  * Uncheck the "active" input.
  * Click "Change"
  * The "active" field should have the error message:
    "This project cannot be deactivated since it is still linked to
    source packages."

[Gavin Panella (allenap)]

Hi Edwin,

This is nice, and it's good to see some XXXs go in the process. The
view code with the Storm validator as a backstop is elegant.

I've got a few questions and comments, so Needs Information for now.

Cheers, Gavin.

> === modified file 'lib/lp/answers/doc/person.txt'
> --- lib/lp/answers/doc/person.txt 2010-02-05 21:25:23 +0000
> +++ lib/lp/answers/doc/person.txt 2010-04-21 11:03:32 +0000
> @@ -351,6 +351,10 @@
> supported projects.
>
> >>> login('<email address hidden>')
> +
> + # A product cannot be deactivated if it is linked to source packages.
> + >>> from lp.testing import unlink_source_packages
> + >>> unlink_source_packages(firefox)
> >>> firefox.active = False
> >>> sorted(target.name
> ... for target in no_priv.getDirectAnswerQuestionTargets())
>
> === modified file 'lib/lp/blueprints/doc/specification.txt'
> --- lib/lp/blueprints/doc/specification.txt 2009-08-13 19:03:36 +0000
> +++ lib/lp/blueprints/doc/specification.txt 2010-04-21 11:03:32 +0000
> @@ -205,14 +205,18 @@
>
> Specs from inactive products are filtered out.
>
> - >>> from canonical.database.sqlbase import flush_database_updates
> - >>> login('<email address hidden>')
> - >>> upstream_firefox.active = False
> - >>> flush_database_updates()
> - >>> for spec in specset.specifications(filter=['install']):
> - ... print spec.name, spec.target.name
> - cluster-installation kubuntu
> - media-integrity-check ubuntu
> + >>> from canonical.database.sqlbase import flush_database_updates
> + >>> login('<email address hidden>')
> +
> + # A product cannot be deactivated if it is linked to source packages.
> + >>> from lp.testing import unlink_source_packages
> + >>> unlink_source_packages(upstream_firefox)
> + >>> upstream_firefox.active = False
> + >>> flush_database_updates()
> + >>> for spec in specset.specifications(filter=['install']):
> + ... print spec.name, spec.target.name
> + cluster-installation kubuntu
> + media-integrity-check ubuntu
>
>
> Reset firefox so we don't mess up later tests.
>
> === modified file 'lib/lp/blueprints/doc/sprint.txt'
> --- lib/lp/blueprints/doc/sprint.txt 2010-02-17 11:13:06 +0000
> +++ lib/lp/blueprints/doc/sprint.txt 2010-04-21 11:03:32 +0000
> @@ -151,14 +151,18 @@
>
> Inactive products are excluded from the listings.
>
> - >>> from canonical.launchpad.interfaces import IProductSet
> - >>> from canonical.launchpad.ftests import login
> - >>> firefox = getUtility(IProductSet).getByName('firefox')
> - >>> login("<email address hidden>")
> - >>> firefox.active = False
> - >>> flush_database_updates()
> - >>> ubz.specifications().count()
> - 0
> + >>> from canonical.launchpad.interfaces import IProductSet
> + >>> from canonical.launchpad.ftests import login
> + >>> firefox = getUtility(IProductSet).getByName('firefox')
> + >>> login("<email address hidden>")
> +
> + # A product cannot be deactivated if it is linked to source packages.
> + >>> from lp.testing import unlink_source_packages
> + >>> unlink_source_packages(firefox)
> + >>> firefox.active = False
> + >>> flush_database_updates()
> + >>> ubz.specifications().count()
> + 0
>
> ...

review: Needs Information (code)

[Brad Crittenden (bac)]

Hi Edwin,

Thanks for making this fix...it sure had lots of tentacles.

> === modified file 'lib/lp/answers/doc/person.txt'
> --- lib/lp/answers/doc/person.txt 2010-02-05 21:25:23 +0000
+++ lib/lp/answers/doc/person.txt 2010-04-20 20:36:22 +0000
> @@ -351,6 +351,10 @@
> supported projects.
>
> >>> login('<email address hidden>')
> +
> + # A product cannot be deactivated if it is linked to source packages.

I think this comment would be easier to read in context if it were
phrased positively, i.e.

# Unlink the source packages so the firefox project can be deactivated.

I see you've repeated the phrase many, many times in your changes.
I'll leave it up to you to decide whether you want to make the wording
change everywhere as it is only a minor improvement.

> + >>> from lp.testing import unlink_source_packages
> + >>> unlink_source_packages(firefox)
> >>> firefox.active = False
> >>> sorted(target.name
> ... for target in no_priv.getDirectAnswerQuestionTargets())

> === modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
> --- lib/lp/bugs/browser/bugalsoaffects.py 2010-01-15 03:32:46 +0000
> +++ lib/lp/bugs/browser/bugalsoaffects.py 2010-04-20 20:36:22 +0000
> @@ -103,26 +103,6 @@
> bugtask = self.context
> upstream = bugtask.target.upstream_product
> if upstream is not None:
> - if not upstream.active:
> - # XXX: Guilherme Salgado 2007-09-18 bug=140526: This is only
> - # possible because of bug 140526, which allows packages to
> - # be linked to inactive products.
> - series = bugtask.distribution.currentseries
> - assert series is not None, (
> - "This package is linked to a product series so this "
> - "package's distribution must have at least one distro "
> - "series.")
> - sourcepackage = series.getSourcePackage(
> - bugtask.sourcepackagename)
> - self.request.response.addWarningNotification(
> - structured(
> - _("""
> - This package is linked to an inactive upstream. You
> - can <a href="%(package_url)s/+edit-packaging">fix it</a>
> - to avoid this step in the future."""),
> - package_url=canonical_url(sourcepackage)))
> - return
> -

I'm really glad you found and removed these.

> try:
> valid_upstreamtask(bugtask.bug, upstream)
> except WidgetsError:

> === modified file 'lib/lp/registry/browser/product.py'
> --- lib/lp/registry/browser/product.py 2010-04-14 17:16:56 +0000
> +++ lib/lp/registry/browser/product.py 2010-04-20 20:36:22 +0000
> @@ -1444,6 +1444,15 @@
> """See `LaunchpadFormView`."""
> self.validate_private_bugs(data)
>
> + if data['active'] == False and self.context.active == True:
> + if len(self.context.sourcepackages) > 0:
> + self.setFieldError('active',
> + structured(
> + 'This project cannot be dea...

review: Approve (code)

[Edwin Grubbs (edwin-grubbs)]

=== modified file 'lib/lp/answers/doc/person.txt'
--- lib/lp/answers/doc/person.txt 2010-04-20 20:27:09 +0000
+++ lib/lp/answers/doc/person.txt 2010-04-21 16:05:41 +0000
@@ -352,7 +352,7 @@

     >>> login('<email address hidden>')

- # A product cannot be deactivated if it is linked to source packages.
+ # Unlink the source packages so the project can be deactivated.
     >>> from lp.testing import unlink_source_packages
     >>> unlink_source_packages(firefox)
     >>> firefox.active = False

=== modified file 'lib/lp/blueprints/doc/specification.txt'
--- lib/lp/blueprints/doc/specification.txt 2010-04-20 20:27:09 +0000
+++ lib/lp/blueprints/doc/specification.txt 2010-04-21 16:05:41 +0000
@@ -208,7 +208,7 @@
     >>> from canonical.database.sqlbase import flush_database_updates
     >>> login('<email address hidden>')

- # A product cannot be deactivated if it is linked to source packages.
+ # Unlink the source packages so the project can be deactivated.
     >>> from lp.testing import unlink_source_packages
     >>> unlink_source_packages(upstream_firefox)
     >>> upstream_firefox.active = False

=== modified file 'lib/lp/blueprints/doc/sprint.txt'
--- lib/lp/blueprints/doc/sprint.txt 2010-04-20 20:27:09 +0000
+++ lib/lp/blueprints/doc/sprint.txt 2010-04-21 16:05:41 +0000
@@ -156,7 +156,7 @@
     >>> firefox = getUtility(IProductSet).getByName('firefox')
     >>> login("<email address hidden>")

- # A product cannot be deactivated if it is linked to source packages.
+ # Unlink the source packages so the project can be deactivated.
     >>> from lp.testing import unlink_source_packages
     >>> unlink_source_packages(firefox)
     >>> firefox.active = False

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-04-19 20:03:36 +0000
+++ lib/lp/registry/browser/product.py 2010-04-21 14:34:21 +0000
@@ -1376,7 +1376,7 @@
         return self.next_url

-class EditPrivateBugsMixin:
+class ProductValidationMixin:

     def validate_private_bugs(self, data):
         """Perform validation for the private bugs setting."""
@@ -1387,8 +1387,20 @@
                     'for this project first.',
                     canonical_url(self.context, rootsite="bugs")))

-
-class ProductAdminView(ProductEditView, EditPrivateBugsMixin):
+ def validate_deactivation(self, data):
+ """Verify whether a product can be safely deactivated."""
+ if data['active'] == False and self.context.active == True:
+ if len(self.context.sourcepackages) > 0:
+ self.setFieldError('active',
+ structured(
+ 'This project cannot be deactivated since it is '
+ 'linked to one or more '
+ '<a href="%s">source packages</a>.',
+ canonical_url(self.context, view_name='+packages')))
+
+
+class ProductAdminView(ProductEditView, ProductValidationMixin):
+ """View for $project/+admin"""
     label = "Administer project details"
     field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]

@@ -1443,15 +1455,7 @@
     def validate(self, data):
         """See `La...

[Edwin Grubbs (edwin-grubbs)]

> Hi Edwin,
>
> This is nice, and it's good to see some XXXs go in the process. The
> view code with the Storm validator as a backstop is elegant.
>
> I've got a few questions and comments, so Needs Information for now.
>
> Cheers, Gavin.
>

Hi Gavin and Brad,

Thanks for the reviews. The incremental diff is in the previous comment.

> > === modified file 'lib/lp/registry/browser/product.py'
> > --- lib/lp/registry/browser/product.py 2010-04-14 17:16:56 +0000
> > +++ lib/lp/registry/browser/product.py 2010-04-21 11:03:32 +0000
> > @@ -1444,6 +1444,15 @@
> > """See `LaunchpadFormView`."""
> > self.validate_private_bugs(data)
> >
> > + if data['active'] == False and self.context.active == True:
> > + if len(self.context.sourcepackages) > 0:
> > + self.setFieldError('active',
> > + structured(
> > + 'This project cannot be deactivated since it is '
> > + 'linked to '
> > + '<a href="%s/+packages">source packages</a>.',
> > + canonical_url(self.context)))
>
> You could change this to instead pass view_name='+packages' to
> canonical_url().

Fixed.

> It is worth showing this message before the user tries to deactivate
> the project? Perhaps a message next to the control, and the control
> disabled.

That's a nice idea, however, the validation would still be necessary for
race conditions when the form is displayed before the source package is
linked. Since deactivation will be done much more often by reviewers
than regular users, I don't know if it is worth it. I believe Curtis
still reviews most of the projects, so I'll ask him.

> Also, the text "This project cannot be deactivated since it is linked
> to source packages" doesn't sound quite right. How about "... linked
> to one or more source packages"?

Fixed.

> Perhaps it's worth getting a UI review?

Since I'm already talking to Curtis, and he's a UI reviewer, I'll ask him.

> > +
> > @property
> > def cancel_url(self):
> > """See `LaunchpadFormView`."""
> > @@ -1491,6 +1500,15 @@
> > # supervisor.
> > self.validate_private_bugs(data)
> >
> > + if data['active'] == False and self.context.active == True:
> > + if len(self.context.sourcepackages) > 0:
> > + self.setFieldError('active',
> > + structured(
> > + 'This project cannot be deactivated since it is '
> > + 'linked to '
> > + '<a href="%s/+packages">source packages</a>.',
> > + canonical_url(self.context)))
>
> Same here, re. view_name.
>
> ProductReviewLicenseView.validate() uses the same validation code as
> in ProductAdminView.validate(). Consider putting this in a mixin class
> that both ProductAdminView and ProductReviewLicenseView can use.

Done.

> > +
> >
> > class ProductAddSeriesView(LaunchpadFormView):
> > """A form to add new product series"""
> >
> > === modified file 'lib/lp/registry/tests/test_product.py'
> > --- lib/lp/registry/tests/test_product.py 2...

[Gavin Panella (allenap)]

Thanks Edwin, looks good :)

review: Approve

[Curtis Hovey (sinzui)]

Hello Edwin, et al.

Regarding who and how projects are deactivated...

Users cannot deactivate a project, reviewers and admins can. The rule of not deactivating a linked project seemed obvious 6 months ago and I have always deleted bad packaging links /before/ deactivating a project. It is clear from the oops this was not obvious to everyone so I reported a bug to stop other reviewers from doing the wrong thing.

I do not deactivate projects with legitimate links to Ubuntu. This has frustrated two users who think their desires outweigh the community. We *will not* deactivate any linked project because someone else will just have it reactivated.

This issue relates to series deletion. Users cannot delete a series that is linked to a package. The user can delete the packaing links. In the case of the sugar project, the user recreated links for the correct series.

=== modified file 'lib/lp/answers/doc/person.txt'
--- lib/lp/answers/doc/person.txt	2010-02-05 21:25:23 +0000
+++ lib/lp/answers/doc/person.txt	2010-04-22 14:31:33 +0000
@@ -351,6 +351,10 @@
 supported projects.
 
     >>> login('[e-mail]')
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
     >>> firefox.active = False
     >>> sorted(target.name
     ...        for target in no_priv.getDirectAnswerQuestionTargets())

=== modified file 'lib/lp/blueprints/doc/specification.txt'
--- lib/lp/blueprints/doc/specification.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/blueprints/doc/specification.txt	2010-04-22 14:31:33 +0000
@@ -205,14 +205,18 @@
 
 Specs from inactive products are filtered out.
 
- >>> from canonical.database.sqlbase import flush_database_updates
- >>> login('[e-mail]')
- >>> upstream_firefox.active = False
- >>> flush_database_updates()
- >>> for spec in specset.specifications(filter=['install']):
- ...     print spec.name, spec.target.name
- cluster-installation kubuntu
- media-integrity-check ubuntu
+    >>> from canonical.database.sqlbase import flush_database_updates
+    >>> login('[e-mail]')
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(upstream_firefox)
+    >>> upstream_firefox.active = False
+    >>> flush_database_updates()
+    >>> for spec in specset.specifications(filter=['install']):
+    ...     print spec.name, spec.target.name
+    cluster-installation kubuntu
+    media-integrity-check ubuntu
 
 
 Reset firefox so we don't mess up later tests.

=== modified file 'lib/lp/blueprints/doc/sprint.txt'
--- lib/lp/blueprints/doc/sprint.txt	2010-02-17 11:13:06 +0000
+++ lib/lp/blueprints/doc/sprint.txt	2010-04-22 14:31:33 +0000
@@ -151,14 +151,18 @@
 
 Inactive products are excluded from the listings.
 
- >>> from canonical.launchpad.interfaces import IProductSet
- >>> from canonical.launchpad.ftests import login
- >>> firefox = getUtility(IProductSet).getByName('firefox')
- >>> login("[e-mail]")
- >>> firefox.active = False
- >>> flush_database_updates()
- >>> ubz.specifications().count()
- 0
+    >>> from canonical.launchpad.interfaces import IProductSet
+    >>> from canonical.launchpad.ftests import login
+    >>> firefox = getUtility(IProductSet).getByName('firefox')
+    >>> login("[e-mail]")
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
+    >>> firefox.active = False
+    >>> flush_database_updates()
+    >>> ubz.specifications().count()
+    0
 
 Reset firefox so we don't mess up later tests.
 

=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
--- lib/lp/bugs/browser/bugalsoaffects.py	2010-01-15 03:32:46 +0000
+++ lib/lp/bugs/browser/bugalsoaffects.py	2010-04-22 14:31:33 +0000
@@ -103,26 +103,6 @@
         bugtask = self.context
         upstream = bugtask.target.upstream_product
         if upstream is not None:
-            if not upstream.active:
-                # XXX: Guilherme Salgado 2007-09-18 bug=140526: This is only
-                # possible because of bug 140526, which allows packages to
-                # be linked to inactive products.
-                series = bugtask.distribution.currentseries
-                assert series is not None, (
-                    "This package is linked to a product series so this "
-                    "package's distribution must have at least one distro "
-                    "series.")
-                sourcepackage = series.getSourcePackage(
-                    bugtask.sourcepackagename)
-                self.request.response.addWarningNotification(
-                    structured(
-                    _("""
-                    This package is linked to an inactive upstream.  You
-                    can <a href="%(package_url)s/+edit-packaging">fix it</a>
-                    to avoid this step in the future."""),
-                    package_url=canonical_url(sourcepackage)))
-                return
-
             try:
                 valid_upstreamtask(bugtask.bug, upstream)
             except WidgetsError:

=== modified file 'lib/lp/bugs/browser/tests/bugtask-adding-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-adding-views.txt	2009-07-06 16:23:49 +0000
+++ lib/lp/bugs/browser/tests/bugtask-adding-views.txt	2010-04-22 14:31:33 +0000
@@ -162,42 +162,6 @@
     >>> len(add_task_view.request.response.notifications)
     0
 
-XXX: Because of bug 140526, it's possible to have a package linked to a series
-of an inactive upstream. In a case like that we must ask the user to specify
-the upstream, but we also display a warning telling the user there's a
-package linked to an inactive upstream. -- Guilherme Salgado, 2007-09-18
-
-    # Create the view manually just so we can get the correct upstream
-    # easily.
-    >>> from lp.bugs.browser.bugalsoaffects import (
-    ...     ChooseProductStep)
-    >>> view = ChooseProductStep(
-    ...     ubuntu_firefox_task, LaunchpadTestRequest(method='GET', form={}))
-    >>> upstream = view.context.target.upstream_product
-    >>> upstream.active
-    True
-
-    # Mark the upstream as inactive.
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> removeSecurityProxy(upstream).active = False
-    >>> removeSecurityProxy(upstream).syncUpdate()
-
-    >>> add_task_view = get_and_setup_view(
-    ...     ubuntu_firefox_task, '+choose-affected-product', form={},
-    ...     method='GET')
-
-    >>> add_task_view.step_name
-    'choose_product'
-    >>> print add_task_view.widgets['product']._getFormInput()
-    None
-    >>> for notification in add_task_view.request.response.notifications:
-    ...     print notification.message.strip()
-    This package is linked to an inactive upstream...
-
-    # Mark the upstream back as active.
-    >>> removeSecurityProxy(upstream).active = True
-    >>> removeSecurityProxy(upstream).syncUpdate()
-
 Let's take a look at the second step now, where we may enter the URL of
 the remote bug and confirm the bugtask creation.
 In order to show that all the events get fired off, let's create an

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-04-14 17:16:56 +0000
+++ lib/lp/registry/browser/product.py	2010-04-22 14:31:33 +0000
@@ -1376,7 +1376,7 @@
         return self.next_url
 
 
-class EditPrivateBugsMixin:
+class ProductValidationMixin:
 
     def validate_private_bugs(self, data):
         """Perform validation for the private bugs setting."""
@@ -1387,8 +1387,20 @@
                     'for this project first.',
                     canonical_url(self.context, rootsite="bugs")))
 
-
-class ProductAdminView(ProductEditView, EditPrivateBugsMixin):
+    def validate_deactivation(self, data):
+        """Verify whether a product can be safely deactivated."""
+        if data['active'] == False and self.context.active == True:
+            if len(self.context.sourcepackages) > 0:
+                self.setFieldError('active',
+                    structured(
+                        'This project cannot be deactivated since it is '
+                        'linked to one or more '
+                        '<a href="%s">source packages</a>.',
+                        canonical_url(self.context, view_name='+packages')))
+
+
+class ProductAdminView(ProductEditView, ProductValidationMixin):
+    """View for $project/+admin"""
     label = "Administer project details"
     field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]
 
@@ -1443,6 +1455,7 @@
     def validate(self, data):
         """See `LaunchpadFormView`."""
         self.validate_private_bugs(data)
+        self.validate_deactivation(data)
 
     @property
     def cancel_url(self):
@@ -1451,7 +1464,7 @@
 
 
 class ProductReviewLicenseView(ReturnToReferrerMixin,
-                               ProductEditView, EditPrivateBugsMixin):
+                               ProductEditView, ProductValidationMixin):
     """A view to review a project and change project privileges."""
     label = "Review project"
     field_names = [
@@ -1490,6 +1503,7 @@
         # Private bugs can only be enabled if the product has a bug
         # supervisor.
         self.validate_private_bugs(data)
+        self.validate_deactivation(data)
 
 
 class ProductAddSeriesView(LaunchpadFormView):

=== modified file 'lib/lp/registry/browser/tests/product-views.txt'
--- lib/lp/registry/browser/tests/product-views.txt	2010-04-14 20:54:40 +0000
+++ lib/lp/registry/browser/tests/product-views.txt	2010-04-22 14:31:33 +0000
@@ -155,9 +155,27 @@
     ['license_reviewed', 'license_approved', 'active', 'private_bugs',
      'reviewer_whiteboard']
 
+The reviewer cannot deactivate a project if it is linked
+to a source package.
+
+    >>> firefox.active
+    True
+
+    >>> form = {
+    ...     'field.active.used': '', # unchecked
+    ...     'field.reviewer_whiteboard': 'Looks bogus',
+    ...     'field.actions.change': 'Change',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     firefox, name='+review-license', form=form)
+    >>> view.errors
+    [...This project cannot be deactivated since it is linked to
+    ...source packages</a>.']
+
 The reviewer can deactivate a project if he concludes it is bogus.
 
-    >>> firefox.active
+    >>> product = factory.makeProduct(name='tomato', title='Tomato')
+    >>> product.active
     True
 
     >>> form = {
@@ -166,12 +184,12 @@
     ...     'field.actions.change': 'Change',
     ...     }
     >>> view = create_initialized_view(
-    ...     firefox, name='+review-license', form=form)
+    ...     product, name='+review-license', form=form)
     >>> view.errors
     []
-    >>> firefox.active
+    >>> product.active
     False
-    >>> print firefox.reviewer_whiteboard
+    >>> print product.reviewer_whiteboard
     Looks bogus
 
 The reviewer can enable privileged features like private bugs. He can
@@ -439,8 +457,8 @@
 officially supports.
 
     >>> from canonical.launchpad.testing.pages import find_tag_by_id
+    >>> product = factory.makeProduct(name='tomato', title='Tomato')
 
-    >>> product = factory.makeProduct(name='tomato', title='Tomato')
     >>> owner = product.owner
     >>> login_person(owner)
     >>> question = factory.makeQuestion(target=product)

=== modified file 'lib/lp/registry/doc/milestone.txt'
--- lib/lp/registry/doc/milestone.txt	2010-04-16 15:06:55 +0000
+++ lib/lp/registry/doc/milestone.txt	2010-04-22 14:31:33 +0000
@@ -221,6 +221,10 @@
 
     >>> print [milestone.name for milestone in gnome.milestones]
     [u'1.1.']
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(netapplet)
     >>> netapplet.active = False
     >>> syncUpdate(netapplet)
     >>> print [milestone.name for milestone in gnome.milestones]

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2010-04-18 17:20:47 +0000
+++ lib/lp/registry/doc/person.txt	2010-04-22 14:31:33 +0000
@@ -1344,6 +1344,10 @@
     >>> from canonical.launchpad.ftests import login
     >>> firefox = getUtility(IProductSet).getByName('firefox')
     >>> login("[e-mail]")
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
     >>> firefox.active = False
     >>> flush_database_updates()
     >>> cprov.specifications(filter=['svg']).count()

=== modified file 'lib/lp/registry/doc/pillar.txt'
--- lib/lp/registry/doc/pillar.txt	2010-04-19 15:13:39 +0000
+++ lib/lp/registry/doc/pillar.txt	2010-04-22 14:31:33 +0000
@@ -113,6 +113,9 @@
 
 But only if the pillar which they point to is active.
 
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
     >>> firefox.active = False
     >>> 'iceweasel' in pillar_set
     False

=== modified file 'lib/lp/registry/doc/product.txt'
--- lib/lp/registry/doc/product.txt	2010-04-16 15:06:55 +0000
+++ lib/lp/registry/doc/product.txt	2010-04-22 14:31:33 +0000
@@ -54,7 +54,7 @@
     >>> evo = p
 
 To fetch a product we use IProductSet.getByName() or IProductSet.__getitem__.
-The former will, by default, reeturn active and inactive products, while the
+The former will, by default, return active and inactive products, while the
 later returns only active ones. Both can be used to look up products by their
 aliases, though.
 
@@ -91,6 +91,9 @@
 
 Now, to test the active flag. If we disabled a product:
 
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(a52dec)
     >>> a52dec.active = False
 
 It should no longer be retrievable via ProductSet's __getitem__:
@@ -193,6 +196,9 @@
 
 Only active products are listed as translatables.
 
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(evo)
     >>> evo.active = False
     >>> for product in productset.getTranslatables():
     ...    print product.name
@@ -517,12 +523,12 @@
 
 Only active products are returned.
 
-    >>> firefox.active = False
+    >>> evo.active = False
     >>> from canonical.launchpad.ftests import syncUpdate
     >>> syncUpdate(firefox)
     >>> for product in productset.getProductsWithBranches():
     ...     print product.name
-    evolution
+    firefox
     gnome-terminal
     iso-codes
     landscape

=== modified file 'lib/lp/registry/doc/project.txt'
--- lib/lp/registry/doc/project.txt	2010-04-19 09:39:29 +0000
+++ lib/lp/registry/doc/project.txt	2010-04-22 14:31:33 +0000
@@ -31,7 +31,7 @@
 == Looking up existing projects ==
 
 To fetch a project we use IProjectGroupSet.getByName() or
-IProjectGroupSet.__getitem__. The former will, by default, reeturn active and
+IProjectGroupSet.__getitem__. The former will, by default, return active and
 inactive projects, while the latter returns only active ones. Both can be
 used to look up projects by their aliases, though.
 
@@ -108,6 +108,10 @@
     [u'applets', u'evolution', u'gnome-terminal', u'gnomebaker', u'netapplet']
 
     >>> netapplet = gnome.getProduct('netapplet')
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(netapplet)
     >>> netapplet.active = False
     >>> flush_database_updates()
     >>> [product.name for product in gnome.products]
@@ -161,6 +165,10 @@
 
     >>> from canonical.launchpad.interfaces import IProductSet
     >>> firefox = getUtility(IProductSet).getByName('firefox')
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
     >>> firefox.active = False
     >>> flush_database_updates()
     >>> filter = [SpecificationFilter.INCOMPLETE]

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-04-19 09:39:29 +0000
+++ lib/lp/registry/model/product.py	2010-04-22 14:31:33 +0000
@@ -273,7 +273,6 @@
 
     enable_bug_expiration = BoolCol(dbName='enable_bug_expiration',
         notNull=True, default=False)
-    active = BoolCol(dbName='active', notNull=True, default=True)
     license_reviewed = BoolCol(dbName='reviewed', notNull=True, default=False)
     reviewer_whiteboard = StringCol(notNull=False, default=None)
     private_bugs = BoolCol(
@@ -290,6 +289,18 @@
     bug_reporting_guidelines = StringCol(default=None)
     _cached_licenses = None
 
+    def _validate_active(self, attr, value):
+        # Validate deactivation.
+        if self.active == True and value == False:
+            if len(self.sourcepackages) > 0:
+                raise AssertionError(
+                    'This project cannot be deactivated since it is '
+                    'linked to source packages.')
+        return value
+
+    active = BoolCol(dbName='active', notNull=True, default=True,
+                     storm_validator=_validate_active)
+
     def _validate_license_info(self, attr, value):
         if not self._SO_creating and value != self.license_info:
             # Clear the license_reviewed and license_approved flags

=== modified file 'lib/lp/registry/stories/pillar/xx-pillar-deactivation.txt'
--- lib/lp/registry/stories/pillar/xx-pillar-deactivation.txt	2008-01-16 19:27:48 +0000
+++ lib/lp/registry/stories/pillar/xx-pillar-deactivation.txt	2010-04-22 14:31:33 +0000
@@ -7,12 +7,20 @@
 cover that part of the change. See bug #156263 for more details.
     -- kiko, 2007-10-23
 
+    >>> from lp.testing import unlink_source_packages
+    >>> from lp.registry.interfaces.pillar import IPillarNameSet
+    >>> from lp.registry.interfaces.product import IProduct
+    >>> from zope.component import getUtility
+    >>> pillar_set = getUtility(IPillarNameSet)
     >>> def toggleProject(name):
-    ...     admin_browser.open('http://launchpad.dev/%s' % name)
-    ...     admin_browser.getLink(url='+review').click()
-    ...     state = admin_browser.getControl('Active').selected
-    ...     admin_browser.getControl('Active').selected = (not state)
-    ...     admin_browser.getControl('Change').click()
+    ...     login('[e-mail]')
+    ...     pillar = pillar_set.getByName(name)
+    ...     if IProduct.providedBy(pillar) and pillar.active:
+    ...         # A product cannot be deactivated if
+    ...         # it is linked to source packages.
+    ...         unlink_source_packages(pillar)
+    ...     pillar.active = not pillar.active
+    ...     logout()
 
 We start off with active and visible projects:
 
@@ -72,13 +80,12 @@
     >>> anon_browser.getLink(url='/firefox').click()
     >>> anon_browser.title
     'Mozilla Firefox in Launchpad'
-    >>> print find_tag_by_id(admin_browser.contents, 'project-inactive')
+    >>> print find_tag_by_id(anon_browser.contents, 'project-inactive')
     None
 
     >>> anon_browser.open('http://launchpad.dev/projects/+index?text=mozilla')
     >>> anon_browser.getLink(url='/mozilla').click()
     >>> anon_browser.title
     'The Mozilla Project in Launchpad'
-    >>> print find_tag_by_id(admin_browser.contents, 'project-inactive')
+    >>> print find_tag_by_id(anon_browser.contents, 'project-inactive')
     None
-

=== modified file 'lib/lp/registry/stories/product/xx-product-edit.txt'
--- lib/lp/registry/stories/product/xx-product-edit.txt	2010-03-16 22:07:45 +0000
+++ lib/lp/registry/stories/product/xx-product-edit.txt	2010-04-22 14:31:33 +0000
@@ -332,27 +332,27 @@
 
 The Admins and Registry Experts can deactivate a project.
 
-    >>> registry_browser.open('http://launchpad.dev/firefox/+review-license')
+    >>> registry_browser.open('http://launchpad.dev/bzr/+review-license')
     >>> registry_browser.getControl(name='field.active').value = False
     >>> registry_browser.getControl(name='field.actions.change').click()
     >>> print registry_browser.url
-    http://launchpad.dev/firefox
+    http://launchpad.dev/bzr
 
 The product overview page should show a notice that a product is
 inactive with a link to a form to re-activate it. Admins and Commercial
 Admins can still see the product, but regular users can't.
 
-    >>> registry_browser.open('http://launchpad.dev/firefox')
+    >>> registry_browser.open('http://launchpad.dev/bzr')
     >>> contents = find_main_content(registry_browser.contents)
     >>> print extract_text(contents.find(id='project-inactive'))
     This project is currently inactive ...
 
-    >>> admin_browser.open('http://launchpad.dev/firefox')
+    >>> admin_browser.open('http://launchpad.dev/bzr')
     >>> contents = find_main_content(admin_browser.contents)
     >>> print extract_text(contents.find(id='project-inactive'))
     This project is currently inactive ...
 
-    >>> user_browser.open('http://launchpad.dev/firefox')
+    >>> user_browser.open('http://launchpad.dev/bzr')
     Traceback (most recent call last):
     ...
     NotFound...
@@ -361,12 +361,12 @@
 
     >>> registry_browser.getLink('Review project').click()
     >>> print registry_browser.url
-    http://launchpad.dev/firefox/+review-license
+    http://launchpad.dev/bzr/+review-license
 
     >>> registry_browser.getControl(name='field.active').value = True
     >>> registry_browser.getControl(name='field.actions.change').click()
     >>> print registry_browser.url
-    http://launchpad.dev/firefox
+    http://launchpad.dev/bzr
 
     >>> contents = find_main_content(registry_browser.contents)
     >>> print contents.find(id='project-inactive')

=== modified file 'lib/lp/registry/stories/project/xx-project-index.txt'
--- lib/lp/registry/stories/project/xx-project-index.txt	2010-04-19 08:11:52 +0000
+++ lib/lp/registry/stories/project/xx-project-index.txt	2010-04-22 14:31:33 +0000
@@ -150,6 +150,12 @@
     # (i.e. login()) and bypass the security proxy.
     >>> from lp.registry.model.product import Product
     >>> firefox = Product.byName('firefox')
+
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> login('[e-mail]')
+    >>> unlink_source_packages(firefox)
+    >>> logout()
     >>> firefox.active = False
     >>> firefox.syncUpdate()
 

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2009-10-23 13:48:28 +0000
+++ lib/lp/registry/tests/test_product.py	2010-04-22 14:31:33 +0000
@@ -26,6 +26,34 @@
     CommercialSubscription)
 from lp.testing import TestCaseWithFactory
 
+class TestProduct(TestCaseWithFactory):
+    """Tests product object."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_deactivation_failure(self):
+        # Ensure that a product cannot be deactivated if
+        # it is linked to source packages.
+        login('[e-mail]')
+        product = self.factory.makeProduct()
+        source_package = self.factory.makeSourcePackage()
+        self.assertEqual(True, product.active)
+        source_package.setPackaging(
+            product.development_focus, self.factory.makePerson())
+        self.assertRaises(
+            AssertionError,
+            setattr, product, 'active', False)
+
+    def test_deactivation_success(self):
+        # Ensure that a product can be deactivated if
+        # it is not linked to source packages.
+        login('[e-mail]')
+        product = self.factory.makeProduct()
+        self.assertEqual(True, product.active)
+        product.active = False
+        self.assertEqual(False, product.active)
+
+
 class TestProductFiles(unittest.TestCase):
     """Tests for downloadable product files."""
 

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-04-18 22:31:40 +0000
+++ lib/lp/testing/__init__.py	2010-04-22 14:31:33 +0000
@@ -31,6 +31,7 @@
     'TestCaseWithFactory',
     'test_tales',
     'time_counter',
+    'unlink_source_packages',
     # XXX: This really shouldn't be exported from here. People should import
     # it from Zope.
     'verifyObject',
@@ -83,6 +84,7 @@
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.windmill.testing import constants
 from lp.codehosting.vfs import branch_id_to_path, get_multi_server
+from lp.registry.interfaces.packaging import IPackagingUtil
 # Import the login and logout functions here as it is a much better
 # place to import them from in tests.
 from lp.testing._login import (
@@ -930,3 +932,15 @@
                             name, mock_args, real_args))
                     else:
                         break
+
+def unlink_source_packages(product):
+    """Remove all links between the product and source packages.
+
+    A product cannot be deactivated if it is linked to source packages.
+    """
+    packaging_util = getUtility(IPackagingUtil)
+    for source_package in product.sourcepackages:
+        packaging_util.deletePackaging(
+            source_package.productseries,
+            source_package.sourcepackagename,
+            source_package.distroseries)

=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt	2010-02-05 15:28:42 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt	2010-04-22 14:31:33 +0000
@@ -1043,6 +1043,9 @@
 An administrator deactivates Firefox, thinking that the project is being
 run in violation of Launchpad policies.
 
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> unlink_source_packages(firefox)
     >>> firefox.active = False
     >>> syncUpdate(firefox)
 

=== modified file 'lib/lp/translations/stories/translationgroups/30-show-group-translation-targets.txt'
--- lib/lp/translations/stories/translationgroups/30-show-group-translation-targets.txt	2009-08-25 16:42:56 +0000
+++ lib/lp/translations/stories/translationgroups/30-show-group-translation-targets.txt	2010-04-22 14:31:33 +0000
@@ -26,6 +26,13 @@
     >>> admin_browser.url
     'http://launchpad.dev/projectgroups'
 
+    # Unlink the source packages so the project can be deactivated.
+    >>> from lp.testing import unlink_source_packages
+    >>> from lp.registry.interfaces.product import IProductSet
+    >>> from zope.component import getUtility
+    >>> login('[e-mail]')
+    >>> unlink_source_packages(getUtility(IProductSet).getByName('netapplet'))
+    >>> logout()
     >>> admin_browser.open("http://launchpad.dev/netapplet/+admin")
     >>> admin_browser.getControl("Active").click()
     >>> admin_browser.getControl("Change").click()