Merge lp:~rharding/launchpad/remove_assigned_specs_1068817 into lp:launchpad

[Richard Harding]

Remove person.assigned_specs due to limited use and to prevent possible leakage.

= Summary =

person.assigned_specs is a limited used property that can be a potential way
to shoot yourself in the foot since it doesn't deal with checking permissions
on items.

Its single use case is in tests and deactivating an account when you do in
fact want to make sure you catch all specifications that need to be
unassigned.

== Pre Implementation ==

Had a chat with Aaron around the reasoning for cleaning this up.

== Implementation Notes ==

We just replace the simple query directly into the deactivate account work.
It's doing a lot of work and this doesn't add much to it.

The larger update is to the tests since many of the doctests used this as a
shortcut to get to the list of assigned specs.

== Tests ==

registry/doc/person.txt
registry/doc/person-account.txt

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'lib/lp/registry/doc/person-account.txt'
--- lib/lp/registry/doc/person-account.txt	2012-10-24 07:49:54 +0000
+++ lib/lp/registry/doc/person-account.txt	2012-10-26 13:32:24 +0000
@@ -12,6 +12,7 @@
 process. Matsubara's account was created during a code import.
 
     >>> from zope.component import getUtility
+    >>> from lp.blueprints.enums import SpecificationFilter
     >>> from lp.services.identity.interfaces.emailaddress import (
     ...     IEmailAddressSet,
     ...     )
@@ -100,7 +101,8 @@
     >>> foobar.searchTasks(params).count() > 0
     True
 
-    >>> len(foobar.assigned_specs) > 0
+    >>> foobar.specifications(
+    ...     foobar, filter=[SpecificationFilter.ASSIGNEE]).count() > 0
     True
 
     >>> foobar_pillars = []
@@ -173,7 +175,8 @@
 
 ...no assigned specs...
 
-    >>> len(foobar.assigned_specs)
+    >>> foobar.specifications(
+    ...     foobar, filter=[SpecificationFilter.ASSIGNEE]).count()
     0
 
 ...no owned teams...

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2012-10-26 07:15:49 +0000
+++ lib/lp/registry/doc/person.txt	2012-10-26 13:32:24 +0000
@@ -1183,7 +1183,9 @@
 
 These 2 specifications are assigned to Carlos:
 
-    >>> for spec in carlos.assigned_specs:
+    >>> assigned_specs = carlos.specifications(
+    ...     carlos, filter=[SpecificationFilter.ASSIGNEE])
+    >>> for spec in assigned_specs:
     ...     print spec.name
     svg-support
     extension-manager-upgrades

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2012-10-26 07:18:55 +0000
+++ lib/lp/registry/interfaces/person.py	2012-10-26 13:32:24 +0000
@@ -848,8 +848,6 @@
         "Any specifications related to this person, either because the are "
         "a subscriber, or an assignee, or a drafter, or the creator. "
         "Sorted newest-first.")
-    assigned_specs = Attribute(
-        "Specifications assigned to this person, sorted newest first.")
 
     def findVisibleAssignedInProgressSpecs(user):
         """List specifications in progress assigned to this person.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-10-26 07:18:55 +0000
+++ lib/lp/registry/model/person.py	2012-10-26 13:32:24 +0000
@@ -799,12 +799,6 @@
                 person=self, time_zone=time_zone, latitude=latitude,
                 longitude=longitude, last_modified_by=user)
 
-    # specification-related joins
-    @property
-    def assigned_specs(self):
-        return shortlist(Specification.selectBy(
-            assignee=self, orderBy=['-datecreated']))
-
     def findVisibleAssignedInProgressSpecs(self, user):
         """See `IPerson`."""
         return self.specifications(user, in_progress=True, quantity=5,
@@ -2175,8 +2169,8 @@
         product_set = getUtility(IProductSet)
         non_public_products = product_set.get_users_private_products(self)
         if non_public_products.count() != 0:
-            errors.append(('This account cannot be deactivated because it owns '
-                        'the following non-public products: ') +
+            errors.append(('This account cannot be deactivated because it owns'
+                        ' the following non-public products: ') +
                         ','.join([p.name for p in non_public_products]))
 
         if self.account_status != AccountStatus.ACTIVE:
@@ -2226,8 +2220,12 @@
                "Bugtask %s assignee isn't the one expected: %s != %s" % (
                     bug_task.id, bug_task.assignee.name, self.name))
             bug_task.transitionToAssignee(None)
-        for spec in self.assigned_specs:
+
+        assigned_specs = Store.of(self).find(
+            Specification, assignee=self)
+        for spec in assigned_specs:
             spec.assignee = None
+
         registry_experts = getUtility(ILaunchpadCelebrities).registry_experts
         for team in Person.selectBy(teamowner=self):
             team.teamowner = registry_experts