Merge lp:~jcsackett/launchpad/user-email-existing-account-576757 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Robert Collins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11562 |
Proposed branch: | lp:~jcsackett/launchpad/user-email-existing-account-576757 |
Merge into: | lp:launchpad |
Diff against target: |
131 lines (+67/-10) 2 files modified
lib/lp/registry/browser/person.py (+26/-9) lib/lp/registry/browser/tests/test_person_view.py (+41/-1) |
To merge this branch: | bzr merge lp:~jcsackett/launchpad/user-email-existing-account-576757 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Collins (community) | Approve | ||
Review via email: mp+35575@code.launchpad.net |
Commit message
Fixes a bad conditional in email validation that assumes the email must have a person, causing an OOPS. Now the validation checks the email info to create the best possible error message from what's available.
Description of the change
Summary
=======
Fixes a condition which raised an OOPS on adding emails if the email already was associated with an account but not a person.
Proposed Fix
============
Create or improve a checker for email addresses so that the condition can't happen.
Pre-implementation notes
=======
Spoke with Curtis Hovey (sinzui) who proposed that the error wasn't actually where the OOPS was raised, but earlier in checking the availability of the email address entered.
Implementation details
=======
Adds a few conditions to the email validation. Where the previous setup simply assumed the email has a person to create the error message, the function now checks that the person actually exists, and if not checks for account to create the message. If no account exists, it defaults to a very basic error message.
Tests
=====
bin/test -t test_add_email
Demo and Q/A
============
If you create an Account without a Person in the database for launchpad.dev, then attempt to add that email to an existing account in the launchpad.dev, it should provide an error message rather than OOPSing.
Lint
====
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
There's a fair amount of duplicate code in this, mainly in tests. It would be nice to look at reducing that in future.