launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06031
[Merge] lp:~wgrant/launchpad/use-people-where-possible into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/use-people-where-possible into lp:launchpad with lp:~wgrant/launchpad/mailinglists-use-people as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/use-people-where-possible/+merge/87464
This branch changes a few tests to use Persons instead of Accounts, since we no longer need support for personless accounts. This is preparation for the abolition of EmailAddress.account.
I also dropped AccountPrincipalMixin, a personless account publication class that has been unreferenced since SSO was split out.
--
https://code.launchpad.net/~wgrant/launchpad/use-people-where-possible/+merge/87464
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/use-people-where-possible into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py 2012-01-04 11:55:32 +0000
@@ -399,10 +399,11 @@
def test_add_email_address_taken(self):
email_address = self.factory.getUniqueEmailAddress()
- self.factory.makeAccount(
+ self.factory.makePerson(
+ name='deadaccount',
displayname='deadaccount',
email=email_address,
- status=AccountStatus.NOACCOUNT)
+ account_status=AccountStatus.NOACCOUNT)
form = {
'field.VALIDATED_SELECTED': self.valid_email_address,
'field.VALIDATED_SELECTED-empty-marker': 1,
@@ -411,8 +412,13 @@
}
view = create_initialized_view(self.person, "+editemails", form=form)
error_msg = view.errors[0]
- expected_msg = ("The email address '%s' is already registered to an "
- "account, deadaccount." % email_address)
+ expected_msg = (
+ "The email address '%s' is already registered to "
+ "<a href=\"http://launchpad.dev/~deadaccount\";>deadaccount</a>. "
+ "If you think that is a duplicated account, you can "
+ "<a href=\"http://launchpad.dev/people/+requestmerge?";
+ "field.dupe_person=deadaccount\">merge it</a> into your account."
+ % email_address)
self.assertEqual(expected_msg, error_msg)
@@ -445,10 +451,11 @@
def test_validate_email_catches_taken_emails(self):
email_address = self.factory.getUniqueEmailAddress()
- self.factory.makeAccount(
+ self.factory.makePerson(
+ name='libertylandaccount',
displayname='libertylandaccount',
email=email_address,
- status=AccountStatus.NOACCOUNT)
+ account_status=AccountStatus.NOACCOUNT)
form = {
'field.actions.create': 'Create Team',
'field.contactemail': email_address,
@@ -461,9 +468,10 @@
}
person_set = getUtility(IPersonSet)
view = create_initialized_view(person_set, '+newteam', form=form)
- expected_msg = ('%s is already registered in Launchpad and is '
- 'associated with the libertylandaccount '
- 'account.' % email_address)
+ expected_msg = (
+ '%s is already registered in Launchpad and is associated with '
+ '<a href="http://launchpad.dev/~libertylandaccount";>'
+ 'libertylandaccount</a>.' % email_address)
error_msg = view.errors[0].errors[0]
self.assertEqual(expected_msg, error_msg)
=== modified file 'lib/lp/registry/doc/mailinglist-subscriptions.txt'
--- lib/lp/registry/doc/mailinglist-subscriptions.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/registry/doc/mailinglist-subscriptions.txt 2012-01-04 11:55:32 +0000
@@ -1336,10 +1336,8 @@
Now Umma's account is suspended by a Launchpad administrator.
>>> from lp.services.identity.interfaces.account import (
- ... AccountStatus, IAccountSet)
- >>> umma_account = getUtility(IAccountSet).getByEmail(
- ... 'umma.person@xxxxxxxxxxx')
- >>> umma_account.status = AccountStatus.SUSPENDED
+ ... AccountStatus)
+ >>> umma.account.status = AccountStatus.SUSPENDED
>>> transaction.commit()
Umma is no longer subscribed to the mailing list...
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2011-12-30 06:14:56 +0000
+++ lib/lp/registry/tests/test_personset.py 2012-01-04 11:55:32 +0000
@@ -201,12 +201,13 @@
def test_existing_deactivated_account(self):
# An existing deactivated account will be reactivated.
- account = self.factory.makeAccount('purchaser',
- status=AccountStatus.DEACTIVATED)
+ person = self.factory.makePerson(
+ account_status=AccountStatus.DEACTIVATED)
openid_ident = removeSecurityProxy(
- account).openid_identifiers.any().identifier
+ person.account).openid_identifiers.any().identifier
- person, db_updated = self.callGetOrCreate(openid_ident)
+ found_person, db_updated = self.callGetOrCreate(openid_ident)
+ self.assertEqual(person, found_person)
self.assertEqual(AccountStatus.ACTIVE, person.account.status)
self.assertTrue(db_updated)
self.assertEqual(
@@ -215,10 +216,10 @@
def test_existing_suspended_account(self):
# An existing suspended account will raise an exception.
- account = self.factory.makeAccount('purchaser',
- status=AccountStatus.SUSPENDED)
+ person = self.factory.makePerson(
+ account_status=AccountStatus.SUSPENDED)
openid_ident = removeSecurityProxy(
- account).openid_identifiers.any().identifier
+ person.account).openid_identifiers.any().identifier
self.assertRaises(
AccountSuspendedError, self.callGetOrCreate, openid_ident)
@@ -236,12 +237,12 @@
def test_no_matching_account_existing_email(self):
# The openid_identity of the account matching the email will
# updated.
- other_account = self.factory.makeAccount('test', email='a@xxxxx')
+ other_person = self.factory.makePerson('a@xxxxx')
person, db_updated = self.callGetOrCreate(
u'other-openid-identifier', 'a@xxxxx')
- self.assertEqual(other_account, person.account)
+ self.assertEqual(other_person, person)
self.assert_(
u'other-openid-identifier' in [
identifier.identifier for identifier in removeSecurityProxy(
=== modified file 'lib/lp/registry/tests/test_xmlrpc.py'
--- lib/lp/registry/tests/test_xmlrpc.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_xmlrpc.py 2012-01-04 11:55:32 +0000
@@ -88,10 +88,11 @@
def test_getOrCreateSoftwareCenterCustomer_xmlrpc_error(self):
# A suspended account results in an appropriate xmlrpc fault.
- suspended_account = self.factory.makeAccount(
- 'Joe Blogs', email='a@xxxxx', status=AccountStatus.SUSPENDED)
+ suspended_person = self.factory.makePerson(
+ displayname='Joe Blogs', email='a@xxxxx',
+ account_status=AccountStatus.SUSPENDED)
openid_identifier = removeSecurityProxy(
- suspended_account).openid_identifiers.any().identifier
+ suspended_person.account).openid_identifiers.any().identifier
# assertRaises doesn't let us check the type of Fault.
fault_raised = False
=== modified file 'lib/lp/services/webapp/servers.py'
--- lib/lp/services/webapp/servers.py 2011-12-30 07:32:58 +0000
+++ lib/lp/services/webapp/servers.py 2012-01-04 11:55:32 +0000
@@ -1066,24 +1066,6 @@
"""The publication used for the main Launchpad site."""
-class AccountPrincipalMixin:
- """Mixin for publication that works with person-less accounts."""
-
- def getPrincipal(self, request):
- """Return the authenticated principal for this request.
-
- This is only necessary because, unlike in LaunchpadBrowserPublication,
- here we want principals representing personless accounts to be
- returned, so that personless accounts can use our OpenID server.
- """
- auth_utility = getUtility(IPlacelessAuthUtility)
- principal = auth_utility.authenticate(request)
- if principal is None:
- principal = auth_utility.unauthenticatedPrincipal()
- assert principal is not None, "Missing unauthenticated principal."
- return principal
-
-
# ---- feeds
class FeedsPublication(LaunchpadBrowserPublication):
=== modified file 'lib/lp/services/webapp/tests/test_login.py'
--- lib/lp/services/webapp/tests/test_login.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_login.py 2012-01-04 11:55:32 +0000
@@ -43,6 +43,7 @@
AccountStatus,
IAccountSet,
)
+from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.timeline.requesttimeline import get_request_timeline
from lp.services.webapp.dbpolicy import MasterDatabasePolicy
@@ -345,8 +346,11 @@
# the identity URL that's associated with the existing email address.
identifier = u'4w7kmzU'
email = 'test@xxxxxxxxxxx'
- account = self.factory.makeAccount(
- 'Test account', email=email, status=AccountStatus.DEACTIVATED)
+ person = self.factory.makePerson(
+ displayname='Test account', email=email,
+ account_status=AccountStatus.DEACTIVATED,
+ email_address_status=EmailAddressStatus.NEW)
+ account = person.account
account_set = getUtility(IAccountSet)
self.assertRaises(
LookupError, account_set.getByOpenIDIdentifier, identifier)
@@ -365,7 +369,7 @@
self.assertEquals(AccountStatus.ACTIVE, account.status)
self.assertEquals(
- email, removeSecurityProxy(account.preferredemail).email)
+ email, removeSecurityProxy(person.preferredemail).email)
person = IPerson(account, None)
self.assertIsNot(None, person)
self.assertEquals('Test account', person.displayname)
@@ -379,24 +383,24 @@
# The user has the account's password and is trying to login, so we'll
# just re-activate their account.
email = 'foo@xxxxxxxxxxx'
- account = self.factory.makeAccount(
- 'Test account', email=email, status=AccountStatus.DEACTIVATED)
- self.assertIs(None, IPerson(account, None))
+ person = self.factory.makePerson(
+ displayname='Test account', email=email,
+ account_status=AccountStatus.DEACTIVATED,
+ email_address_status=EmailAddressStatus.NEW)
openid_identifier = removeSecurityProxy(
- account).openid_identifiers.any().identifier
+ person.account).openid_identifiers.any().identifier
openid_response = FakeOpenIDResponse(
'http://testopenid.dev/+id/%s' % openid_identifier,
- status=SUCCESS, email=email, full_name=account.displayname)
+ status=SUCCESS, email=email, full_name=person.displayname)
with SRegResponse_fromSuccessResponse_stubbed():
view, html = self._createAndRenderView(openid_response)
- self.assertIsNot(None, IPerson(account, None))
self.assertTrue(view.login_called)
response = view.request.response
self.assertEquals(httplib.TEMPORARY_REDIRECT, response.getStatus())
self.assertEquals(view.request.form['starting_url'],
response.getHeader('Location'))
- self.assertEquals(AccountStatus.ACTIVE, account.status)
- self.assertEquals(email, account.preferredemail.email)
+ self.assertEquals(AccountStatus.ACTIVE, person.account.status)
+ self.assertEquals(email, person.preferredemail.email)
# We also update the last_write flag in the session, to make sure
# further requests use the master DB and thus see the newly created
# stuff.
@@ -406,23 +410,23 @@
# The account was created by one of our scripts but was never
# activated, so we just activate it.
email = 'foo@xxxxxxxxxxx'
- account = self.factory.makeAccount(
- 'Test account', email=email, status=AccountStatus.NOACCOUNT)
- self.assertIs(None, IPerson(account, None))
+ person = self.factory.makePerson(
+ displayname='Test account', email=email,
+ account_status=AccountStatus.DEACTIVATED,
+ email_address_status=EmailAddressStatus.NEW)
openid_identifier = IStore(OpenIdIdentifier).find(
OpenIdIdentifier.identifier,
- OpenIdIdentifier.account_id == account.id).order_by(
+ OpenIdIdentifier.account_id == person.account.id).order_by(
OpenIdIdentifier.account_id).order_by(
OpenIdIdentifier.account_id).first()
openid_response = FakeOpenIDResponse(
'http://testopenid.dev/+id/%s' % openid_identifier,
- status=SUCCESS, email=email, full_name=account.displayname)
+ status=SUCCESS, email=email, full_name=person.displayname)
with SRegResponse_fromSuccessResponse_stubbed():
view, html = self._createAndRenderView(openid_response)
- self.assertIsNot(None, IPerson(account, None))
self.assertTrue(view.login_called)
- self.assertEquals(AccountStatus.ACTIVE, account.status)
- self.assertEquals(email, account.preferredemail.email)
+ self.assertEquals(AccountStatus.ACTIVE, person.account.status)
+ self.assertEquals(email, person.preferredemail.email)
# We also update the last_write flag in the session, to make sure
# further requests use the master DB and thus see the newly created
# stuff.
@@ -431,10 +435,10 @@
def test_suspended_account(self):
# There's a chance that our OpenID Provider lets a suspended account
# login, but we must not allow that.
- account = self.factory.makeAccount(
- 'Test account', status=AccountStatus.SUSPENDED)
+ person = self.factory.makePerson(
+ account_status=AccountStatus.SUSPENDED)
with SRegResponse_fromSuccessResponse_stubbed():
- view, html = self._createViewWithResponse(account)
+ view, html = self._createViewWithResponse(person.account)
self.assertFalse(view.login_called)
main_content = extract_text(find_main_content(html))
self.assertIn('This account has been suspended', main_content)
@@ -442,9 +446,9 @@
def test_negative_openid_assertion(self):
# The OpenID provider responded with a negative assertion, so the
# login error page is shown.
- account = self.factory.makeAccount('Test account')
+ person = self.factory.makePerson()
view, html = self._createViewWithResponse(
- account, response_status=FAILURE,
+ person.account, response_status=FAILURE,
response_msg='Server denied check_authentication')
self.assertFalse(view.login_called)
main_content = extract_text(find_main_content(html))
@@ -455,13 +459,13 @@
# user already has a valid cookie, so we add a notification message to
# the response and redirect to the starting_url specified in the
# OpenID response.
- test_account = self.factory.makeAccount('Test account')
+ test_person = self.factory.makePerson()
class StubbedOpenIDCallbackViewLoggedIn(StubbedOpenIDCallbackView):
- account = test_account
+ account = test_person.account
view, html = self._createViewWithResponse(
- test_account, response_status=FAILURE,
+ test_person.account, response_status=FAILURE,
response_msg='Server denied check_authentication',
view_class=StubbedOpenIDCallbackViewLoggedIn)
self.assertFalse(view.login_called)
@@ -484,9 +488,9 @@
# Completing an OpenID association *can* make an HTTP request to the
# OP, so it's a potentially long action. It is logged to the
# request timeline.
- account = self.factory.makeAccount('Test account')
+ person = self.factory.makePerson()
with SRegResponse_fromSuccessResponse_stubbed():
- view, html = self._createViewWithResponse(account)
+ view, html = self._createViewWithResponse(person.account)
start, stop = get_request_timeline(view.request).actions[-2:]
self.assertEqual(start.category, 'openid-association-complete-start')
self.assertEqual(start.detail, '')
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-12-30 06:14:56 +0000
+++ lib/lp/testing/factory.py 2012-01-04 11:55:32 +0000
@@ -614,7 +614,7 @@
email_address_status=None, hide_email_addresses=False,
displayname=None, time_zone=None, latitude=None, longitude=None,
selfgenerated_bugnotifications=False, member_of=(),
- homepage_content=None):
+ homepage_content=None, account_status=None):
"""Create and return a new, arbitrary Person.
:param email: The email address for the new person.
@@ -679,6 +679,8 @@
removeSecurityProxy(email).status = email_address_status
+ if account_status:
+ removeSecurityProxy(person.account).status = account_status
self.makeOpenIdIdentifier(person.account)
for team in member_of:
