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.
Well that is not going to work 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)
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?
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.
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.
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?
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.
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:
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.