Openfoodnetwork: Email Confirmation | Add confirmable email to user model

[daniellemoorhead]

Change the user model to have confirmable email addresses

Other issues that need to be created:

• [ ] Change Discourse login to check for confirmed user email address (2)
• [ ] Replace enterprise.email by enterprise.contact which is a user (3)

[pdelacroix]

Replace enterprise.email by enterprise.contact which is a user

Is that not what's the owner reference is already doing?

[Matt-Yorkley]

It looks like you're right, owner is already a reference to a user. I notice there's two fields for email and email_address as well...

It looks like email is used for order notifications and email_address is used for enterprise contact. I guess we can leave them as they are?

[Matt-Yorkley]

After a lot of rummaging around in the code/database and discussion in Slack, it looks like we should leave enterprise.email alone, as it's a separate field for where to send the order emails? And enterprise.owner is already a reference to a user.

[oeoeaio]

@Matt-Yorkley @ltrls Are you clear on the current function of all of the confusing email-related parts of the Enterprise model?

• owner is the owner. At the moment, their email address is only relevant in the sense that it is used to populate email initially...
• email is used for communication from the OFN to the enterprise user, and is also the address that is 'confirmable' in the devise sense. It defaults to the owner's email address, but can be changed afterwards if desired. I would like to replace this with an association (either a list of emails or a list of users) at some point, but we need to keep it for now.
• email_address is a contact email that is only really used to display to customers on the front end. We could probably abstract this out into a separate 'EnterpriseContact' model at some point, along with other contact details.
• contact is currently a text string designed to hold the name of the enterprise user (mainly used to say "Hi [contact]" at the start of emails, I think?).

So I don't think we can do the second checkbox at this stage, mainly because enterprise.contact is already taken...

[enricostano]

Nice @oeoeaio , would be great to have that knowledge embedded in the code!

[Matt-Yorkley]

@oeoeaio are there two different contact roles needed here that need to be preserved, ie:

• the current contact and email_address seem to be for displaying in the front end, more as a point of contact for user inquiries?
• the email field receives emails related to orders and running the enterprise?

Are we merging these into a single contact, or keeping them separate?

[oeoeaio]

@Matt-Yorkley They are not to be merged, they are used for different things. My preference would be for contact to become contact_name, email_address to become contact_email and email to be removed (replaced by an association with a user who will receive notifications).

Ultimately, it would be cool to pull out all of the contact fields into a separate model, but I think that is outside the scope for now. Also, see my comment here about using a flag on the existing managers association to handle notifications, rather than adding a new one. Probably also outside of scope.

[sauloperez]

The least we can do to ease that much complexity is to improve the naming (It should be clear at first sight the purpose of each of them). I rename the database columns or maybe add alias methods on the models as first step until we have the time to implement the refactor/s you suggest. Agree that this should be done on another PR.

[myriamboure]

@Matt-Yorkley I think that the second point have been treated already, am I right? But not the first one on discourse login? Just to know and create the appropriate issue :-) Thanks!

[Matt-Yorkley]

The discourse login changes were included in the large merged PR for emails, so both of these should be done now and ready to close.