Curtis Hovey has proposed merging lp:~sinzui/launchpad/merge-non-active-person
into lp:launchpad.
Commit message:
Allow users to merge non-active accounts.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1057947 in Launchpad itself: "LocationError: (None, 'name') when merging
an account with a NEW email address"
https://bugs.launchpad.net/launchpad/+bug/1057947
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/merge-non-active-person/+merge/127041
MergePeopleView.initialize() uses PersonSet.getByEmail() in a way that
won't find people by a NEW email address, causing merges of unactivated
accounts to fail like OOPS-e2f5d3b6769c012de049daf4a60c0941.
This is basically the same as bug #1019975, just at a later stage.
The fix was to pass filter_status=False to the method so that NEW
addresses can be used to look up the IPerson.
--------------------------------------------------------------------
RULES
Pre-implementation: no one
* Add a test for the current case with active accounts and validated
email addresses.
* Setup the same test with a NEW email address and non-active account.
Expect the LocationError: (None, 'name') result.
* Add filter_status=False to the view's call to PersonSet.getByEmail()
and verify the test passes.
QA
* Visit https://qastaging.launchpad.net/~ddw
* Follow the "Are you Ddw" link and Continue to send an confirmation email.
* Look in qastaging db for the token associated with your address:
select * from logintoken where created > '2012-09-28';
* Visit https://qastaging.launchpad.net/token/<TOKEN/+accountmerge
* Verify the page loads.
* Confirm
* Verify the page loads and you see that the merge is queued.
LINT
lib/lp/services/verification/browser/logintoken.py
lib/lp/services/verification/browser/tests/test_logintoken.py
LoC
I have a 3000 line credit at the moment.
TEST
./bin/test -vvc -t MergePeopleView lp.services.verification.browser.tests
IMPLEMENTATION
This is a rare case where the implementation plan was exactly as expected.
lib/lp/services/verification/browser/logintoken.py
lib/lp/services/verification/browser/tests/test_logintoken.py
--
https://code.launchpad.net/~sinzui/launchpad/merge-non-active-person/+merge/127041
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~sinzui/launchpad/merge-non-active-person into lp:launchpad.
=== modified file 'lib/lp/services/verification/browser/logintoken.py'
--- lib/lp/services/verification/browser/logintoken.py 2012-08-14 23:19:36 +0000
+++ lib/lp/services/verification/browser/logintoken.py 2012-09-28 18:09:24 +0000
@@ -535,7 +535,8 @@
def initialize(self):
self.redirectIfInvalidOrConsumedToken()
- self.dupe = getUtility(IPersonSet).getByEmail(self.context.email)
+ self.dupe = getUtility(IPersonSet).getByEmail(
+ self.context.email, filter_status=False)
def success(self, message):
# We're not a GeneralFormView, so we need to do the redirect
=== modified file 'lib/lp/services/verification/browser/tests/test_logintoken.py'
--- lib/lp/services/verification/browser/tests/test_logintoken.py 2012-06-14 05:18:22 +0000
+++ lib/lp/services/verification/browser/tests/test_logintoken.py 2012-09-28 18:09:24 +0000
@@ -4,6 +4,7 @@
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.services.identity.interfaces.account import AccountStatus
from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
from lp.services.verification.browser.logintoken import (
ClaimTeamView,
@@ -18,6 +19,7 @@
)
from lp.testing.deprecated import LaunchpadFormHarness
from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.views import create_initialized_view
class TestCancelActionOnLoginTokenViews(TestCaseWithFactory):
@@ -100,3 +102,46 @@
msgs = self._claimToken(token2)
self.assertEquals(
[u'claimee has already been converted to a team.'], msgs)
+
+
+class MergePeopleViewTestCase(TestCaseWithFactory):
+ """Test the view for confirming a merge via login token."""
+
+ layer = DatabaseFunctionalLayer
+
+ def assertWorkflow(self, claimer, dupe):
+ token = getUtility(ILoginTokenSet).new(
+ requester=claimer, requesteremail='m...@eg.dom',
+ email="h...@eg.dom", tokentype=LoginTokenType.ACCOUNTMERGE)
+ view = create_initialized_view(token, name="+accountmerge")
+ self.assertIs(False, view.mergeCompleted)
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ '.*to merge the Launchpad account named.*claimer', view.render())
+ view = create_initialized_view(
+ token, name="+accountmerge", principal=claimer,
+ form={'VALIDATE': 'Confirm'}, method='POST')
+ with person_logged_in(claimer):
+ view.render()
+ self.assertIs(True, view.mergeCompleted)
+ notifications = view.request.notifications
+ self.assertEqual(2, len(notifications))
+ text = notifications[0].message
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ "A merge is queued.*", text)
+
+ def test_confirm_email_for_active_account(self):
+ # Users can confirm they control an email address to merge a duplicate
+ # profile.
+ claimer = self.factory.makePerson(email='m...@eg.dom', name='claimer')
+ dupe = self.factory.makePerson(email='h...@eg.dom', name='dupe')
+ self.assertWorkflow(claimer, dupe)
+
+ def test_confirm_email_for_non_active_account(self):
+ # Users can confirm they control an email address to merge a
+ # non-active duplicate profile.
+ claimer = self.factory.makePerson(email='m...@eg.dom', name='claimer')
+ dupe = self.factory.makePerson(
+ email='h...@eg.dom', name='dupe',
+ email_address_status=EmailAddressStatus.NEW,
+ account_status=AccountStatus.NOACCOUNT)
+ self.assertWorkflow(claimer, dupe)
_______________________________________________
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
