Additional email address per user account support

After some chat with the @team, we’re moving forward with this feature request cautiously together, and in phases.

Phase 1

Phase 1 won’t change anything but the architecture of how a user’s email address(es) are stored. No new features, just some well executed database migrations:

  • Create a new user_email table, with:
    • email column, unique with special handling to support case insensitivity (treating bob@ and BOB@ as the same)
    • user_id column
    • timestamps
  • Migrate the email column from the user table into the new user_email table
    • Add email_id column to user table, not null to enforce primary email as user.email

The associations would therefore be:

  • User
    • belongs_to :email, class_name: 'UserEmail'
    • has_many :user_emails
  • UserEmail
    • belongs_to :user

@sam this is a little different from what we discussed, but I think it makes migration even easier (keeping user.email pointing to the primary email). Thoughts?

Phase 2

Phase 2 will involve adding server side handling of multiple email addresses. At this point this is less clearly defined, and still requires some discussion and planning. A feature almost certainly in this phase will be supporting email-in from alternative addresses.

Phase 3

Phase 3 is even less clearly defined than phase 2, and requires even more discussion and planning, but will focus on adding the UI and user flows to make all of the work done in phases 1&2 useful.


Coding will start on Monday, so I’ll update the topic then with the crucial parts of phase 1 I’ve forgotten to think through.

13 Likes

Well that is not going to work :slight_smile: cause the way you have it wired it would be User.find(1).email.email which is very confusing.

I don’t mind if we wire up a custom method that looks up primary email address but the belongs_to should be belongs_to :primary_user_email, class_name: 'UserEmail'

Also, phase 1 ensures this does not introduce and N+1 queries when looking at user lists (in particular admin can very easily regress here to N+1)

4 Likes

Duh, of course. That’s definitely not what I was going for.

Am I right in thinking there’s nothing inherent in the architecture I’ve proposed which avoids N+1 queries, but instead phase 1 gives us a chance to catch any that are happening? And the way to avoid them is eager loading in these instances, right?

Are you opposed to calling that method email to avoid having to go through each and every .email and replacing it, or is that a necessary part of avoiding N + 1?

After recovering from eating horrendous quantities of chocolate, I’ve finally got back to working on this.

This is what I’ve come up with for phase 1:

https://github.com/discourse/discourse/pull/4840

12 Likes

I’m working on getting this merged but on a closer look I realized that the proposed associations would not work. The UserEmail table needs to have a constraint that the user_id column is not null. Note that the constraint is essential as we don’t want to have an UserEmail record that belongs to no one. At the same time, we’re adding a primary_email_id column which makes sure that a user will always have an email. With those two constraints in place, it isn’t possible to create either an User or an UserEmail record.

Instead we should just add a primary column to the UserEmail table and just make primary_email a scope.

3 Likes

Thank you for working on this @LeoMcA :slight_smile: I’ve merged in your PR in

https://github.com/discourse/discourse/commit/d0b027d88deeabf8bc105419f7d3fae0087091cd

11 Likes

Happy to help, thanks for your and @sam’s work to ensure I didn’t break everything!

With phase 1 merged, I think it’s time to crack on with phase 2!

I’ve been through the changes as a result of phase 1 and found a few areas where we might want to add support for alternative email addresses:

  • email receiver
  • user email address updater
  • topic importers
  • authentication
  • sso

That being said, I think it makes more sense to add each of these as features in their own right, rather than all grouped together as one phase. After all, if phase 1 has been implemented as intended, none of these are broken at the moment, they’re just not as feature rich as they could be.

Adding support for receiving emails from alternative email addresses is the reason why we wanted to see support for alternative emails in Discourse within Mozilla, and in previous discussions it was an oft-mentioned use-case, so I’ll be getting started on that early next week - unless a member of the @team pickets my house or something.

8 Likes

Any updates on where we are with this? I believe all the groundwork is in place and tested as we have a one to many relationship between accounts and emails now in 1.9?

1 Like

Yeah, this dropped down the priority list as a result of a lot of work needing to happen internally on our new authentication system (and because of a dramatic bandwidth reduction on my end due to starting at university).

Most of that authentication work is done now, so this is pretty much next on the list.

Latest I should end up doing it is around Christmas.

7 Likes

Going slow is fine, as absorbing the initial one-to-one → one-to-many change was quite a bit of risky work!

6 Likes

I have a few PRs in the pipeline to finish this work off: one to allow multiple additional emails, another to add specs which show all aspects of email receiving working with additional emails, and one I’m currently working on around the UI.

I have a couple of questions:

First, what do we actually want to refer to these non-primary emails as? I’ve been using a number of different words (in my brain, and as a result in my code). There’s:

  • alternate (which screws with my poor british brain)
  • alternative
  • additional
  • secondary (which makes the most sense to me)

I don’t have particularly strong opinions about it, so if someone wants to tell me which to use I’ll start being consistent.

Secondly, is it going to feel like I’ve acted in bad faith if I don’t implement the user-facing UI for this? I ask because we’ll be implementing our own in our own authentication plugin, so any code I write in core implementing this is completely useless to Mozilla.

The PR I have lined up implements these things in core:

  • secondary emails in the user serializer
  • secondary emails returned when an admin clicks “show email”
  • secondary emails shown in the user admin page
  • a plugin outlet right below the existing email section on my/preferences/account

This allows a plugin developer to implement something like this very easily, with just a little ember:

Screenshot from 2018-01-05 13-04-26

I feel like this PR would be a fine place to end my work on this - as it adds everything necessary to core to allow an authentication plugin to manage the addition and removal of secondary emails - but as I said I don’t want to lie awake at night worrying the core team don’t like me because I haven’t followed through on the full implementation of this feature. :slight_smile:

12 Likes

Is this live in Discourse? A great feature, amazing work from the community!

1 Like

Its a in-progress to be merged PR, adding this to @tgxworld list after he is done with 2fa.

12 Likes

Once we have this, then we really need the feature to merge users…

7 Likes

Any news on this fantastic feature?

1 Like

It is scheduled for Discourse 2.1

9 Likes

Merged in

https://github.com/discourse/discourse/commit/c3129444ea366607023f0fa6bccddf76ca74792a

Thank you @LeoMcA

16 Likes

Here’s my final PR doing everything I mentioned in my previous post:

https://github.com/discourse/discourse/pull/6066

19 Likes

Thank you @LeoMcA for doing all the work to add secondary email support :heart: PR has been merged.

8 Likes

I was waiting for this feature, I couldn’t add a secondary email to my account:

I went to user admin there is no option to edit secondary email: