A partial archive of meta.discourse.org as of Tuesday July 18, 2017.

Additional email support

LeoMcA

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.

sam

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)

LeoMcA

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?

LeoMcA

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:

tgxworld

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.