Brad Crittenden has proposed merging lp:~bac/launchpad/bug-823490 into
lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #823490 in Launchpad itself: "AssertionError claiming a team."
https://bugs.launchpad.net/launchpad/+bug/823490
For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-823490/+merge/72577
= Summary =
If two outstanding attempts to claim an automatically created team are
both acted upon, the second one lost and caused an OOPS. An error
should be shown to the user instead.
== Proposed fix ==
Don't use assert but instead raise and catch an exception and show the
error.
== Pre-implementation notes ==
None
== Implementation details ==
As above.
== Tests ==
bin/test -vvt test_logintoken -t doc/person.txt
== Demo and Q/A ==
Go to https://launchpad.dev/~doc and click "Is this a team you run?"
Enter d...@lists.ubuntu.com for the email address.
Do it again.
Now you should have two emails with the subject "Launchpad: Claim existing
team". Each will have a different token.
Follow the link in the first one to claim the team successfully. You don't
have to enter any info, just click 'Continue'.
Follow the link in the second one to attempt to claim the team again. You
should get an error message.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/browser/logintoken.py
lib/canonical/launchpad/browser/tests/test_logintoken.py
lib/lp/registry/doc/person.txt
lib/lp/registry/model/person.py
--
https://code.launchpad.net/~bac/launchpad/bug-823490/+merge/72577
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~bac/launchpad/bug-823490 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/logintoken.py'
--- lib/canonical/launchpad/browser/logintoken.py 2011-04-09 02:52:13 +0000
+++ lib/canonical/launchpad/browser/logintoken.py 2011-08-23 14:00:29 +0000
@@ -18,7 +18,6 @@
import cgi
import urllib
-import pytz
from zope.app.form.browser import TextAreaWidget
from zope.component import getUtility
from zope.interface import (
@@ -73,9 +72,6 @@
)
-UTC = pytz.UTC
-
-
class LoginTokenSetNavigation(GetitemNavigation):
usedfor = ILoginTokenSet
@@ -245,7 +241,15 @@
@action(_('Continue'), name='confirm')
def confirm_action(self, action, data):
- self.claimed_profile.convertToTeam(team_owner=self.context.requester)
+ # Avoid circular imports.
+ from lp.registry.model.person import AlreadyConvertedException
+ try:
+ self.claimed_profile.convertToTeam(
+ team_owner=self.context.requester)
+ except AlreadyConvertedException, e:
+ self.request.response.addErrorNotification(e)
+ self.context.consume()
+ return
# Although we converted the person to a team it seems that the
# security proxy still thinks it's an IPerson and not an ITeam,
# which means to edit it we need to be logged in as the person we
=== modified file 'lib/canonical/launchpad/browser/tests/test_logintoken.py'
--- lib/canonical/launchpad/browser/tests/test_logintoken.py 2011-08-12 11:37:08 +0000
+++ lib/canonical/launchpad/browser/tests/test_logintoken.py 2011-08-23 14:00:29 +0000
@@ -11,6 +11,7 @@
)
from canonical.launchpad.ftests import LaunchpadFormHarness
from canonical.launchpad.interfaces.authtoken import LoginTokenType
+from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.testing import TestCaseWithFactory
@@ -51,7 +52,7 @@
def _testCancelAction(self, view_class, token):
"""Test the 'Cancel' action of the given view, using the given token.
-
+
To test that the action works, we just submit the form with that
action, check that there are no errors and make sure that the view's
next_url is what we expect.
@@ -63,3 +64,35 @@
self.assertEquals(actions['field.actions.cancel'].submitted(), True)
self.assertEquals(harness.view.errors, [])
self.assertEquals(harness.view.next_url, self.expected_next_url)
+
+
+class TestClaimTeamView(TestCaseWithFactory):
+ """Test the claiming of a team via login token."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ TestCaseWithFactory.setUp(self)
+ self.claimer = self.factory.makePerson(name='claimer')
+ self.claimee_email = 'clai...@example.com'
+ self.claimee = self.factory.makePerson(
+ name='claimee', email=self.claimee_email,
+ email_address_status=EmailAddressStatus.NEW)
+
+ def _claimToken(self, token):
+ harness = LaunchpadFormHarness(token, ClaimTeamView)
+ harness.submit('confirm', {})
+ return [n.message for n in harness.request.notifications]
+
+ def test_CannotClaimTwice(self):
+ token1 = getUtility(ILoginTokenSet).new(
+ requester=self.claimer, requesteremail=None,
+ email=self.claimee_email, tokentype=LoginTokenType.TEAMCLAIM)
+ token2 = getUtility(ILoginTokenSet).new(
+ requester=self.claimer, requesteremail=None,
+ email=self.claimee_email, tokentype=LoginTokenType.TEAMCLAIM)
+ msgs = self._claimToken(token1)
+ self.assertEquals([u'Team claimed successfully'], msgs)
+ msgs = self._claimToken(token2)
+ self.assertEquals(
+ [u'claimee has already been converted to a team.'], msgs)
=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt 2011-06-28 15:04:29 +0000
+++ lib/lp/registry/doc/person.txt 2011-08-23 14:00:29 +0000
@@ -529,7 +529,7 @@
>>> not_a_person.convertToTeam(team_owner=landscape_devs)
Traceback (most recent call last):
...
- AssertionError: Can't convert a team to a team.
+ AlreadyConvertedException: foo-v has already been converted to a team.
Team members
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-08-17 18:08:36 +0000
+++ lib/lp/registry/model/person.py 2011-08-23 14:00:29 +0000
@@ -7,6 +7,7 @@
__metaclass__ = type
__all__ = [
+ 'AlreadyConvertedException',
'get_recipients',
'generate_nick',
'IrcID',
@@ -14,6 +15,7 @@
'JabberID',
'JabberIDSet',
'JoinTeamEvent',
+ 'NicknameGenerationError',
'Owner',
'Person',
'person_sort_key',
@@ -301,6 +303,10 @@
)
+class AlreadyConvertedException(Exception):
+ """Raised when an attempt to claim a team that has been claimed."""
+
+
class JoinTeamEvent:
"""See `IJoinTeamEvent`."""
@@ -674,7 +680,9 @@
def convertToTeam(self, team_owner):
"""See `IPerson`."""
- assert not self.is_team, "Can't convert a team to a team."
+ if self.is_team:
+ raise AlreadyConvertedException(
+ "%s has already been converted to a team." % self.name)
assert self.account_status == AccountStatus.NOACCOUNT, (
"Only Person entries whose account_status is NOACCOUNT can be "
"converted into teams.")
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
