Merge lp:~edwin-grubbs/launchpad/bug-676966-disallow-merging-people-with-ppas into lp:launchpad

Proposed by Edwin Grubbs
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 12123
Proposed branch: lp:~edwin-grubbs/launchpad/bug-676966-disallow-merging-people-with-ppas
Merge into: lp:launchpad
Diff against target: 251 lines (+123/-12)
6 files modified
lib/lp/registry/browser/peoplemerge.py (+8/-0)
lib/lp/registry/browser/person.py (+4/-8)
lib/lp/registry/browser/tests/test_peoplemerge.py (+54/-3)
lib/lp/registry/doc/person-merge.txt (+16/-0)
lib/lp/registry/interfaces/person.py (+9/-0)
lib/lp/registry/model/person.py (+32/-1)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-676966-disallow-merging-people-with-ppas
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
j.c.sackett (community) code* Approve
Review via email: mp+43116@code.launchpad.net

Commit message

[r=jcsackett,sinzui][ui=none][bug=676966] Don't allow a person/team to be merged until its PPAs are deleted.

Description of the change

Summary
-------

To prevent oopses in the PPA publisher, a form error message is
displayed when the user tries to merge a person or team with a PPA
containing published packages.

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

Since the merge views are using the same check as renaming a person with
a PPA, I have moved that check into the model and updated the rename
person view. Person.merge() also checks that the merged person does not
have a PPA with published packages.
    lib/lp/registry/browser/person.py
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/model/person.py

Added check to the AdminMergeBaseView so that the check applies to
+adminpeoplemerge and +adminteammerge.
    lib/lp/registry/browser/peoplemerge.py
    lib/lp/registry/browser/tests/test_peoplemerge.py
    lib/lp/registry/doc/person-merge.txt

Tests
-----

./bin/test -vv -t whatever

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

* Open http://launchpad.dev/...

Lint
----

Linting changed files:
  lib/lp/registry/browser/peoplemerge.py
  lib/lp/registry/browser/person.py
  lib/lp/registry/browser/tests/test_peoplemerge.py
  lib/lp/registry/doc/person-merge.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/person.py

./lib/lp/registry/browser/peoplemerge.py
     143: E301 expected 1 blank line, found 2
./lib/lp/registry/doc/person-merge.txt
       1: narrative uses a moin header.
      22: narrative uses a moin header.
      32: narrative uses a moin header.
      61: narrative exceeds 78 characters.
      85: narrative exceeds 78 characters.
     105: narrative uses a moin header.
     122: narrative uses a moin header.
     339: narrative uses a moin header.
./lib/lp/registry/interfaces/person.py
     493: E302 expected 2 blank lines, found 1
./lib/lp/registry/model/person.py
    1480: W291 trailing whitespace
    1480: Line has trailing whitespace.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :
Download full text (4.7 KiB)

Hey Edwin--

I really like this branch. I just have some suggestions/questions on comments below.

> === modified file 'lib/lp/registry/browser/peoplemerge.py'
> --- lib/lp/registry/browser/peoplemerge.py 2010-12-03 16:33:03 +0000
> +++ lib/lp/registry/browser/peoplemerge.py 2010-12-08 18:32:00 +0000
> @@ -132,6 +132,13 @@
> if dupe_person == target_person and dupe_person is not None:
> self.addError(_("You can't merge ${name} into itself.",
> mapping=dict(name=dupe_person.name)))
> + # We cannot merge the teams if there is a PPA with published
> + # packages on the duplicate person, unless that PPA is removed.
> + if dupe_person.has_ppa_with_published_packages:
> + self.addError(_(
> + "${name} has a PPA with published packages; we "
> + "can't merge it.", mapping=dict(name=dupe_person.name)))

It might be good to direct someone to info on what they can do to get around this? In
other places where we throw an error in merging we indicate that a team needs to be removed or something; perhaps "the PPA must be deleted before we can merge ${name}, after the merge a new PPA can be recreated?

> === modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
> --- lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-03 16:33:03 +0000
> +++ lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-08 18:32:00 +0000
> @@ -152,3 +155,51 @@
> view = self.getView()
> self.assertEqual([], view.errors)
> self.assertEqual(self.target_team, self.dupe_team.merged)
> +
> + def test_cannot_merge_team_with_ppa_containing_published_packages(self):
> + # The PPA must be removed before the team can be merged.
> + login_celebrity('admin')
> + self.dupe_team.subscriptionpolicy = TeamSubscriptionPolicy.MODERATED
> + archive = self.dupe_team.createPPA()
> + self.factory.makeSourcePackagePublishingHistory(archive=archive)
> + login_celebrity('registry_experts')
> + view = self.getView()
> + self.assertEqual(
> + [u"dupe-team has a PPA with published packages; "
> + "we can't merge it."],
> + view.errors)

I'd change the comment to just "A team cannot be merged with published PPA" since the test doesn't demonstrate removing the PPA to make it work.

> === modified file 'lib/lp/registry/doc/person-merge.txt'
> --- lib/lp/registry/doc/person-merge.txt 2010-12-01 23:39:05 +0000
> +++ lib/lp/registry/doc/person-merge.txt 2010-12-08 18:32:00 +0000
> @@ -425,6 +425,8 @@
> >>> test_team.deactivateAllMembers(comment, personset.getByName('name16'))
> >>> for team in test_team.super_teams:
> ... test_team.retractTeamMembership(team, test_team.teamowner)
> +
> +
> >>> personset.merge(test_team, landscape)
>
> # The resulting Landscape-developers no new super teams, has

Thanks for the cleanup.

> @@ -438,3 +440,18 @@
> []
> >>> list(IPollSubset(landscape).getAll())
> []
> +
> +A person with a PPA can't be merged until the PPA is deleted.
> +
> + >>> person_with_ppa = factory.makePerson()
> + >>> ot...

Read more...

review: Needs Information (code*)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (7.6 KiB)

Hi JC,

Thanks for the review. Here are the changes you asked for.

> Hey Edwin--
>
> I really like this branch. I just have some suggestions/questions on comments
> below.
>
> > === modified file 'lib/lp/registry/browser/peoplemerge.py'
> > --- lib/lp/registry/browser/peoplemerge.py 2010-12-03 16:33:03 +0000
> > +++ lib/lp/registry/browser/peoplemerge.py 2010-12-08 18:32:00 +0000
> > @@ -132,6 +132,13 @@
> > if dupe_person == target_person and dupe_person is not None:
> > self.addError(_("You can't merge ${name} into itself.",
> > mapping=dict(name=dupe_person.name)))
> > + # We cannot merge the teams if there is a PPA with published
> > + # packages on the duplicate person, unless that PPA is removed.
> > + if dupe_person.has_ppa_with_published_packages:
> > + self.addError(_(
> > + "${name} has a PPA with published packages; we "
> > + "can't merge it.", mapping=dict(name=dupe_person.name)))
>
> It might be good to direct someone to info on what they can do to get around
> this? In
> other places where we throw an error in merging we indicate that a team needs
> to be removed or something; perhaps "the PPA must be deleted before we can
> merge ${name}, after the merge a new PPA can be recreated?

I did something similar but a little bit shorter.

> > === modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
> > --- lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-03 16:33:03
> +0000
> > +++ lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-08 18:32:00
> +0000
> > @@ -152,3 +155,51 @@
> > view = self.getView()
> > self.assertEqual([], view.errors)
> > self.assertEqual(self.target_team, self.dupe_team.merged)
> > +
> > + def
> test_cannot_merge_team_with_ppa_containing_published_packages(self):
> > + # The PPA must be removed before the team can be merged.
> > + login_celebrity('admin')
> > + self.dupe_team.subscriptionpolicy =
> TeamSubscriptionPolicy.MODERATED
> > + archive = self.dupe_team.createPPA()
> > + self.factory.makeSourcePackagePublishingHistory(archive=archive)
> > + login_celebrity('registry_experts')
> > + view = self.getView()
> > + self.assertEqual(
> > + [u"dupe-team has a PPA with published packages; "
> > + "we can't merge it."],
> > + view.errors)
>
> I'd change the comment to just "A team cannot be merged with published PPA"
> since the test doesn't demonstrate removing the PPA to make it work.

Done.

> > === modified file 'lib/lp/registry/doc/person-merge.txt'
> > --- lib/lp/registry/doc/person-merge.txt 2010-12-01 23:39:05 +0000
> > +++ lib/lp/registry/doc/person-merge.txt 2010-12-08 18:32:00 +0000
> > @@ -425,6 +425,8 @@
> > >>> test_team.deactivateAllMembers(comment,
> personset.getByName('name16'))
> > >>> for team in test_team.super_teams:
> > ... test_team.retractTeamMembership(team, test_team.teamowner)
> > +
> > +
> > >>> personset.merge(test_team, landscape)
> >
> > # The resulting Landscape-developers no new...

Read more...

Revision history for this message
j.c.sackett (jcsackett) wrote :

Edwin--

Thanks for those changes.

review: Approve (code*)
Revision history for this message
Curtis Hovey (sinzui) wrote :

As we discussed on IRC "PPA that must be removed" should be "PPA that must be deleted" since that is the link the user must use.

has_ppa_with_published_packages() will allow users to merge active ppas without packages. I think these ppas will show up in searches. Can you talk to someone on the soyuz team

review: Needs Information (code)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (6.6 KiB)

> As we discussed on IRC "PPA that must be removed" should be "PPA that must be
> deleted" since that is the link the user must use.
>
> has_ppa_with_published_packages() will allow users to merge active ppas
> without packages. I think these ppas will show up in searches. Can you talk to
> someone on the soyuz team

Hi Curtis,

Thanks for the review.

I have changed the condition to not allow merges on a person or team with a PPA in the ACTIVE or DELETING status whether or not it has published packages, so that links to merged persons don't show up in https://launchpad.net/~PERSON/+archivesubscriptions

Incremental Diff:

=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py 2010-12-15 20:17:39 +0000
+++ lib/lp/registry/browser/peoplemerge.py 2010-12-17 04:08:34 +0000
@@ -134,11 +134,12 @@
                   mapping=dict(name=dupe_person.name)))
         # We cannot merge the teams if there is a PPA with published
         # packages on the duplicate person, unless that PPA is removed.
- if dupe_person.has_ppa_with_published_packages:
+ if dupe_person.has_existing_ppa:
             self.addError(_(
- "${name} has a PPA that must be removed before it "
- "can be merged.", mapping=dict(name=dupe_person.name)))
-
+ "${name} has a PPA that must be deleted before it "
+ "can be merged. It may take ten minutes to remove the "
+ "deleted PPA's files.",
+ mapping=dict(name=dupe_person.name)))

     def render(self):
         # Subclasses may define other actions that they will render manually

=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-15 20:17:39 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-17 04:07:09 +0000
@@ -156,17 +156,17 @@
         self.assertEqual([], view.errors)
         self.assertEqual(self.target_team, self.dupe_team.merged)

- def test_cannot_merge_team_with_ppa_containing_published_packages(self):
- # A team with a published PPA cannot be merged.
+ def test_cannot_merge_team_with_ppa(self):
+ # A team with a PPA cannot be merged.
         login_celebrity('admin')
         self.dupe_team.subscriptionpolicy = TeamSubscriptionPolicy.MODERATED
         archive = self.dupe_team.createPPA()
- self.factory.makeSourcePackagePublishingHistory(archive=archive)
         login_celebrity('registry_experts')
         view = self.getView()
         self.assertEqual(
- [u'dupe-team has a PPA that must be removed before '
- 'it can be merged.'],
+ [u"dupe-team has a PPA that must be deleted before it can be "
+ "merged. It may take ten minutes to remove the deleted PPA's "
+ "files."],
             view.errors)

     def test_registry_delete_team_with_super_teams(self):
@@ -205,13 +205,13 @@
         return create_initialized_view(
             self.person_set, '+adminpeoplemerge', form=form)

- def test_cannot_merge_person_with_ppa_containing_published_packages(self):
- # The PPA must be r...

Read more...

Revision history for this message
Curtis Hovey (sinzui) wrote :

This branch looks good to land.

review: Approve (code)
Revision history for this message
Julian Edwards (julian-edwards) wrote :

I think that the restriction on merging people with PPAs with or without publications is wrong. I originally suggested that we allow the merge to take place even though there's a PPA as long as it has no packages (i.e. it's a brand new one). Deleting these types of PPAs has zero affect other than to change its status; there's nothing to actually delete on disk.

The +archivesubscriptions problem should be solved differently.

Also, has_existing_ppa and has_ppa_with_published_packages
should not be properties on IPerson, they should be on IArchiveSet so
that Soyuz-specific queries are kept in the lp.soyuz domain.

The Bugs team made this mistake with one of their bits of code that knows too
much about Soyuz publishing tables and caused circular imports when running
bin/test. It also makes future changes to soyuz code harder because the coder will not expect this sort of code to be outside the soyuz domain.

I recommend that this be fixed ASAP as tech debt. I filed bug 693357 about this.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py 2010-12-13 16:28:55 +0000
+++ lib/lp/registry/browser/peoplemerge.py 2010-12-20 20:45:28 +0000
@@ -132,6 +132,14 @@
132 if dupe_person == target_person and dupe_person is not None:132 if dupe_person == target_person and dupe_person is not None:
133 self.addError(_("You can't merge ${name} into itself.",133 self.addError(_("You can't merge ${name} into itself.",
134 mapping=dict(name=dupe_person.name)))134 mapping=dict(name=dupe_person.name)))
135 # We cannot merge the teams if there is a PPA with published
136 # packages on the duplicate person, unless that PPA is removed.
137 if dupe_person.has_existing_ppa:
138 self.addError(_(
139 "${name} has a PPA that must be deleted before it "
140 "can be merged. It may take ten minutes to remove the "
141 "deleted PPA's files.",
142 mapping=dict(name=dupe_person.name)))
135143
136 def render(self):144 def render(self):
137 # Subclasses may define other actions that they will render manually145 # Subclasses may define other actions that they will render manually
138146
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-12-17 17:49:50 +0000
+++ lib/lp/registry/browser/person.py 2010-12-20 20:45:28 +0000
@@ -327,7 +327,6 @@
327from lp.soyuz.browser.archivesubscription import (327from lp.soyuz.browser.archivesubscription import (
328 traverse_archive_subscription_for_subscriber,328 traverse_archive_subscription_for_subscriber,
329 )329 )
330from lp.soyuz.enums import ArchiveStatus
331from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet330from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
332from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet331from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
333from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease332from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
@@ -4138,16 +4137,13 @@
41384137
4139 When a user has a PPA renames are prohibited.4138 When a user has a PPA renames are prohibited.
4140 """4139 """
4141 active_ppas = [4140 has_ppa_with_published_packages = (
4142 ppa for ppa in self.context.ppas4141 self.context.has_ppa_with_published_packages)
4143 if ppa.status in (ArchiveStatus.ACTIVE, ArchiveStatus.DELETING)]4142 if has_ppa_with_published_packages:
4144 num_packages = sum(
4145 ppa.getPublishedSources().count() for ppa in active_ppas)
4146 if num_packages > 0:
4147 # This makes the field's widget display (i.e. read) only.4143 # This makes the field's widget display (i.e. read) only.
4148 self.form_fields['name'].for_display = True4144 self.form_fields['name'].for_display = True
4149 super(PersonEditView, self).setUpWidgets()4145 super(PersonEditView, self).setUpWidgets()
4150 if num_packages > 0:4146 if has_ppa_with_published_packages:
4151 self.widgets['name'].hint = _(4147 self.widgets['name'].hint = _(
4152 'This user has an active PPA with packages published and '4148 'This user has an active PPA with packages published and '
4153 'may not be renamed.')4149 'may not be renamed.')
41544150
=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
--- lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-13 16:28:55 +0000
+++ lib/lp/registry/browser/tests/test_peoplemerge.py 2010-12-20 20:45:28 +0000
@@ -12,8 +12,11 @@
12 )12 )
13from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities13from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
14from canonical.testing.layers import DatabaseFunctionalLayer14from canonical.testing.layers import DatabaseFunctionalLayer
15from lp.registry.interfaces.person import IPersonSet
16from lp.registry.interfaces.mailinglist import MailingListStatus15from lp.registry.interfaces.mailinglist import MailingListStatus
16from lp.registry.interfaces.person import (
17 IPersonSet,
18 TeamSubscriptionPolicy,
19 )
17from lp.testing import (20from lp.testing import (
18 celebrity_logged_in,21 celebrity_logged_in,
19 login_celebrity,22 login_celebrity,
@@ -92,8 +95,8 @@
92 def setUp(self):95 def setUp(self):
93 super(TestAdminTeamMergeView, self).setUp()96 super(TestAdminTeamMergeView, self).setUp()
94 self.person_set = getUtility(IPersonSet)97 self.person_set = getUtility(IPersonSet)
95 self.dupe_team = self.factory.makeTeam()98 self.dupe_team = self.factory.makeTeam(name='dupe-team')
96 self.target_team = self.factory.makeTeam()99 self.target_team = self.factory.makeTeam(name='target-team')
97 login_celebrity('registry_experts')100 login_celebrity('registry_experts')
98101
99 def getView(self, form=None):102 def getView(self, form=None):
@@ -153,6 +156,19 @@
153 self.assertEqual([], view.errors)156 self.assertEqual([], view.errors)
154 self.assertEqual(self.target_team, self.dupe_team.merged)157 self.assertEqual(self.target_team, self.dupe_team.merged)
155158
159 def test_cannot_merge_team_with_ppa(self):
160 # A team with a PPA cannot be merged.
161 login_celebrity('admin')
162 self.dupe_team.subscriptionpolicy = TeamSubscriptionPolicy.MODERATED
163 archive = self.dupe_team.createPPA()
164 login_celebrity('registry_experts')
165 view = self.getView()
166 self.assertEqual(
167 [u"dupe-team has a PPA that must be deleted before it can be "
168 "merged. It may take ten minutes to remove the deleted PPA's "
169 "files."],
170 view.errors)
171
156 def test_registry_delete_team_with_super_teams(self):172 def test_registry_delete_team_with_super_teams(self):
157 # Registry admins can delete teams with super team memberships.173 # Registry admins can delete teams with super team memberships.
158 self.target_team = getUtility(ILaunchpadCelebrities).registry_experts174 self.target_team = getUtility(ILaunchpadCelebrities).registry_experts
@@ -164,3 +180,38 @@
164 view = self.getView()180 view = self.getView()
165 self.assertEqual([], view.errors)181 self.assertEqual([], view.errors)
166 self.assertEqual(self.target_team, self.dupe_team.merged)182 self.assertEqual(self.target_team, self.dupe_team.merged)
183
184
185class TestAdminPeopleMergeView(TestCaseWithFactory):
186 """Test the AdminPeopleMergeView rules."""
187
188 layer = DatabaseFunctionalLayer
189
190 def setUp(self):
191 super(TestAdminPeopleMergeView, self).setUp()
192 self.person_set = getUtility(IPersonSet)
193 self.dupe_person = self.factory.makePerson(name='dupe-person')
194 self.target_person = self.factory.makePerson()
195 login_celebrity('registry_experts')
196
197 def getView(self, form=None):
198 if form is None:
199 form = {
200 'field.dupe_person': self.dupe_person.name,
201 'field.target_person': self.target_person.name,
202 'field.actions.reassign_emails_and_merge':
203 'Reassign E-mails and Merge',
204 }
205 return create_initialized_view(
206 self.person_set, '+adminpeoplemerge', form=form)
207
208 def test_cannot_merge_person_with_ppa(self):
209 # A person with a PPA cannot be merged.
210 login_celebrity('admin')
211 archive = self.dupe_person.createPPA()
212 view = self.getView()
213 self.assertEqual(
214 [u"dupe-person has a PPA that must be deleted before it can be "
215 "merged. It may take ten minutes to remove the deleted PPA's "
216 "files."],
217 view.errors)
167218
=== modified file 'lib/lp/registry/doc/person-merge.txt'
--- lib/lp/registry/doc/person-merge.txt 2010-12-15 22:11:11 +0000
+++ lib/lp/registry/doc/person-merge.txt 2010-12-20 20:45:28 +0000
@@ -422,6 +422,8 @@
422 >>> test_team.deactivateAllMembers(comment, personset.getByName('name16'))422 >>> test_team.deactivateAllMembers(comment, personset.getByName('name16'))
423 >>> for team in test_team.super_teams:423 >>> for team in test_team.super_teams:
424 ... test_team.retractTeamMembership(team, test_team.teamowner)424 ... test_team.retractTeamMembership(team, test_team.teamowner)
425
426
425 >>> personset.merge(test_team, landscape)427 >>> personset.merge(test_team, landscape)
426428
427 # The resulting Landscape-developers no new super teams and its429 # The resulting Landscape-developers no new super teams and its
@@ -432,3 +434,17 @@
432 [u'salgado', u'name12']434 [u'salgado', u'name12']
433 >>> [team.name for team in landscape.super_teams]435 >>> [team.name for team in landscape.super_teams]
434 []436 []
437
438A person with a PPA can't be merged.
439
440 >>> person_with_ppa = factory.makePerson()
441 >>> other_person = factory.makePerson()
442 >>> archive = person_with_ppa.createPPA()
443 >>> email = IMasterObject(person_with_ppa.preferredemail)
444 >>> email.status = EmailAddressStatus.VALIDATED
445 >>> email.destroySelf()
446 >>> transaction.commit()
447 >>> personset.merge(person_with_ppa, other_person)
448 Traceback (most recent call last):
449 ...
450 AssertionError: from_person has a ppa in ACTIVE or DELETING status
435451
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2010-12-16 18:56:02 +0000
+++ lib/lp/registry/interfaces/person.py 2010-12-20 20:45:28 +0000
@@ -862,6 +862,15 @@
862 # Really IArchive, see archive.py862 # Really IArchive, see archive.py
863 value_type=Reference(schema=Interface)))863 value_type=Reference(schema=Interface)))
864864
865 has_existing_ppa = Bool(
866 title=_("Does this person have a ppa that is active or not fully "
867 "deleted?"),
868 readonly=True, required=False)
869
870 has_ppa_with_published_packages = Bool(
871 title=_("Does this person have a ppa with published packages?"),
872 readonly=True, required=False)
873
865 entitlements = Attribute("List of Entitlements for this person or team.")874 entitlements = Attribute("List of Entitlements for this person or team.")
866875
867 structural_subscriptions = Attribute(876 structural_subscriptions = Attribute(
868877
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2010-12-17 04:48:01 +0000
+++ lib/lp/registry/model/person.py 2010-12-20 20:45:28 +0000
@@ -268,11 +268,15 @@
268 VOUCHER_STATUSES,268 VOUCHER_STATUSES,
269 )269 )
270from lp.services.worlddata.model.language import Language270from lp.services.worlddata.model.language import Language
271from lp.soyuz.enums import ArchivePurpose271from lp.soyuz.enums import (
272 ArchivePurpose,
273 ArchiveStatus,
274 )
272from lp.soyuz.interfaces.archive import IArchiveSet275from lp.soyuz.interfaces.archive import IArchiveSet
273from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet276from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
274from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet277from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
275from lp.soyuz.model.archive import Archive278from lp.soyuz.model.archive import Archive
279from lp.soyuz.model.publishing import SourcePackagePublishingHistory
276from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease280from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
277from lp.translations.model.hastranslationimports import (281from lp.translations.model.hastranslationimports import (
278 HasTranslationImportsMixin,282 HasTranslationImportsMixin,
@@ -2659,6 +2663,29 @@
2659 return Archive.selectBy(2663 return Archive.selectBy(
2660 owner=self, purpose=ArchivePurpose.PPA, orderBy='name')2664 owner=self, purpose=ArchivePurpose.PPA, orderBy='name')
26612665
2666 @property
2667 def has_existing_ppa(self):
2668 """See `IPerson`."""
2669 result = Store.of(self).find(
2670 Archive,
2671 Archive.owner == self.id,
2672 Archive.purpose == ArchivePurpose.PPA,
2673 Archive.status.is_in(
2674 [ArchiveStatus.ACTIVE, ArchiveStatus.DELETING]))
2675 return not result.is_empty()
2676
2677 @property
2678 def has_ppa_with_published_packages(self):
2679 """See `IPerson`."""
2680 result = Store.of(self).find(
2681 Archive,
2682 SourcePackagePublishingHistory.archive == Archive.id,
2683 Archive.owner == self.id,
2684 Archive.purpose == ArchivePurpose.PPA,
2685 Archive.status.is_in(
2686 [ArchiveStatus.ACTIVE, ArchiveStatus.DELETING]))
2687 return not result.is_empty()
2688
2662 def getPPAByName(self, name):2689 def getPPAByName(self, name):
2663 """See `IPerson`."""2690 """See `IPerson`."""
2664 return getUtility(IArchiveSet).getPPAOwnedByPerson(self, name)2691 return getUtility(IArchiveSet).getPPAOwnedByPerson(self, name)
@@ -3759,6 +3786,10 @@
3759 if getUtility(IEmailAddressSet).getByPerson(from_person).count() > 0:3786 if getUtility(IEmailAddressSet).getByPerson(from_person).count() > 0:
3760 raise AssertionError('from_person still has email addresses.')3787 raise AssertionError('from_person still has email addresses.')
37613788
3789 if from_person.has_existing_ppa:
3790 raise AssertionError(
3791 'from_person has a ppa in ACTIVE or DELETING status')
3792
3762 if from_person.is_team and from_person.allmembers.count() > 0:3793 if from_person.is_team and from_person.allmembers.count() > 0:
3763 raise AssertionError(3794 raise AssertionError(
3764 "Only teams without active members can be merged")3795 "Only teams without active members can be merged")