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

Proposed by Richard Harding
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 16204
Proposed branch: lp:~rharding/launchpad/remove_assigned_specs_1068817
Merge into: lp:launchpad
Prerequisite: lp:~rharding/launchpad/fix_flash_1066898
Diff against target: 101 lines (+15/-14)
4 files modified
lib/lp/registry/doc/person-account.txt (+5/-2)
lib/lp/registry/doc/person.txt (+3/-1)
lib/lp/registry/interfaces/person.py (+0/-2)
lib/lp/registry/model/person.py (+7/-9)
To merge this branch: bzr merge lp:~rharding/launchpad/remove_assigned_specs_1068817
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+131558@code.launchpad.net

This proposal supersedes a proposal from 2012-10-26.

Commit message

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

Description of the change

= 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

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
12process. Matsubara's account was created during a code import.12process. Matsubara's account was created during a code import.
1313
14 >>> from zope.component import getUtility14 >>> from zope.component import getUtility
15 >>> from lp.blueprints.enums import SpecificationFilter
15 >>> from lp.services.identity.interfaces.emailaddress import (16 >>> from lp.services.identity.interfaces.emailaddress import (
16 ... IEmailAddressSet,17 ... IEmailAddressSet,
17 ... )18 ... )
@@ -100,7 +101,8 @@
100 >>> foobar.searchTasks(params).count() > 0101 >>> foobar.searchTasks(params).count() > 0
101 True102 True
102103
103 >>> len(foobar.assigned_specs) > 0104 >>> foobar.specifications(
105 ... foobar, filter=[SpecificationFilter.ASSIGNEE]).count() > 0
104 True106 True
105107
106 >>> foobar_pillars = []108 >>> foobar_pillars = []
@@ -173,7 +175,8 @@
173175
174...no assigned specs...176...no assigned specs...
175177
176 >>> len(foobar.assigned_specs)178 >>> foobar.specifications(
179 ... foobar, filter=[SpecificationFilter.ASSIGNEE]).count()
177 0180 0
178181
179...no owned teams...182...no owned teams...
180183
=== 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 @@
11831183
1184These 2 specifications are assigned to Carlos:1184These 2 specifications are assigned to Carlos:
11851185
1186 >>> for spec in carlos.assigned_specs:1186 >>> assigned_specs = carlos.specifications(
1187 ... carlos, filter=[SpecificationFilter.ASSIGNEE])
1188 >>> for spec in assigned_specs:
1187 ... print spec.name1189 ... print spec.name
1188 svg-support1190 svg-support
1189 extension-manager-upgrades1191 extension-manager-upgrades
11901192
=== 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 @@
848 "Any specifications related to this person, either because the are "848 "Any specifications related to this person, either because the are "
849 "a subscriber, or an assignee, or a drafter, or the creator. "849 "a subscriber, or an assignee, or a drafter, or the creator. "
850 "Sorted newest-first.")850 "Sorted newest-first.")
851 assigned_specs = Attribute(
852 "Specifications assigned to this person, sorted newest first.")
853851
854 def findVisibleAssignedInProgressSpecs(user):852 def findVisibleAssignedInProgressSpecs(user):
855 """List specifications in progress assigned to this person.853 """List specifications in progress assigned to this person.
856854
=== 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 @@
799 person=self, time_zone=time_zone, latitude=latitude,799 person=self, time_zone=time_zone, latitude=latitude,
800 longitude=longitude, last_modified_by=user)800 longitude=longitude, last_modified_by=user)
801801
802 # specification-related joins
803 @property
804 def assigned_specs(self):
805 return shortlist(Specification.selectBy(
806 assignee=self, orderBy=['-datecreated']))
807
808 def findVisibleAssignedInProgressSpecs(self, user):802 def findVisibleAssignedInProgressSpecs(self, user):
809 """See `IPerson`."""803 """See `IPerson`."""
810 return self.specifications(user, in_progress=True, quantity=5,804 return self.specifications(user, in_progress=True, quantity=5,
@@ -2175,8 +2169,8 @@
2175 product_set = getUtility(IProductSet)2169 product_set = getUtility(IProductSet)
2176 non_public_products = product_set.get_users_private_products(self)2170 non_public_products = product_set.get_users_private_products(self)
2177 if non_public_products.count() != 0:2171 if non_public_products.count() != 0:
2178 errors.append(('This account cannot be deactivated because it owns '2172 errors.append(('This account cannot be deactivated because it owns'
2179 'the following non-public products: ') +2173 ' the following non-public products: ') +
2180 ','.join([p.name for p in non_public_products]))2174 ','.join([p.name for p in non_public_products]))
21812175
2182 if self.account_status != AccountStatus.ACTIVE:2176 if self.account_status != AccountStatus.ACTIVE:
@@ -2226,8 +2220,12 @@
2226 "Bugtask %s assignee isn't the one expected: %s != %s" % (2220 "Bugtask %s assignee isn't the one expected: %s != %s" % (
2227 bug_task.id, bug_task.assignee.name, self.name))2221 bug_task.id, bug_task.assignee.name, self.name))
2228 bug_task.transitionToAssignee(None)2222 bug_task.transitionToAssignee(None)
2229 for spec in self.assigned_specs:2223
2224 assigned_specs = Store.of(self).find(
2225 Specification, assignee=self)
2226 for spec in assigned_specs:
2230 spec.assignee = None2227 spec.assignee = None
2228
2231 registry_experts = getUtility(ILaunchpadCelebrities).registry_experts2229 registry_experts = getUtility(ILaunchpadCelebrities).registry_experts
2232 for team in Person.selectBy(teamowner=self):2230 for team in Person.selectBy(teamowner=self):
2233 team.teamowner = registry_experts2231 team.teamowner = registry_experts