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

[Richard Harding]

Remove person.assigned_specifications as it's limited in use and is a possible way to shoot yourself in the foot.

= 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

=== 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 08:58:29 +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-23 02:30:37 +0000
+++ lib/lp/registry/doc/person.txt	2012-10-26 08:58:29 +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-24 23:45:42 +0000
+++ lib/lp/registry/interfaces/person.py	2012-10-26 08:58:29 +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.")
     assigned_specs_in_progress = Attribute(
         "Specifications assigned to this person whose implementation is "
         "started but not yet completed, sorted newest first.")

=== modified file 'lib/lp/registry/javascript/product_views.js'
--- lib/lp/registry/javascript/product_views.js	2012-10-05 19:50:03 +0000
+++ lib/lp/registry/javascript/product_views.js	2012-10-26 08:58:29 +0000
@@ -24,6 +24,10 @@
             info_type.cache_to_choicesource(
                 LP.cache.information_type_data));
 
+        // We are not doing ajax saves of the information type so we need to
+        // disable the flash on save for the widget.
+        widget.set('flashEnabled', false);
+
         // When the information type changes, check if we should show/hide
         // UI components.
         widget.on('save', function (ev) {

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-10-24 23:45:42 +0000
+++ lib/lp/registry/model/person.py	2012-10-26 08:58:29 +0000
@@ -798,12 +798,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']))
-
     @property
     def assigned_specs_in_progress(self):
         replacements = sqlvalues(assignee=self)
@@ -2174,8 +2168,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:
@@ -2225,8 +2219,13 @@
                "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 = self.specifications(
+            self,
+            filter=[SpecificationFilter.ASSIGNEE])
+        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