Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Re-add multiple emails in mozillians.org #1566

Merged
merged 2 commits into from
Jan 5, 2017

Conversation

akatsoulas
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 93.22% when pulling 960cb88 on akatsoulas:multiple-emails into f2e4b43 on mozilla:master.

ExternalAccount.objects.create(type=account_type,
user=request_user.userprofile,
identifier=email)
return [request_user]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to return a qs instead of a list with an object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have the object, I would prefer not to hit the db just to create a QS

eq_(len(returned_user), 1)
eq_(returned_user[0], user)

def test_email_already_exists(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to have the same test but using the primary email instead? To test both cases.


<!-- Add a new email -->
<div id="add-email">
<a id="nav-login" title="{{_('Add email using Auth0')}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since users don't really know/care that we are using auth0 (its mozilla branded anyway) I suggest we use something like Add email.

@johngian
Copy link
Contributor

johngian commented Jan 5, 2017

tested locally:

  • Logging in with primary email
  • Logging in with alternate email
  • Add alternate email
  • Make alternate email primary

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 93.227% when pulling 8d82a62 on akatsoulas:multiple-emails into 9422ecc on mozilla:master.

@akatsoulas akatsoulas merged commit cee7d78 into mozilla:master Jan 5, 2017
@akatsoulas akatsoulas deleted the multiple-emails branch January 5, 2017 13:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants