ritstudentgovernment / petitions

PawPrints petition application for the RIT community.
https://pawprints.rit.edu
MIT License
35 stars 13 forks source link

Genericize emails #88

Closed ceko closed 8 years ago

ceko commented 8 years ago

This is a large change, I'd like your input. All emails have been tested but I'm not familiar enough with the app to be totally comfortable yet. I made several changes to the settings file, here is how mine looks:

{
  ...
  "MAIL": {
    "default_domain": "naz.edu",
    "gateway_url": "smtp://web_smtp_relay:password_redacted@smtp.naz.edu:25",
    "templates": {
      "default": {
        "from": "web_tech@naz.edu",
        "to": "web_tech@naz.edu"
      },
      "report_petition": {
        "subject": "[petitions] Petition Reported",
        "template": "report_petition"
      },
      "petition_approved": {
        "subject": "PawPrints - A petition you created has been approved",
        "template": "petition_approved"
      },
      "petition_rejected": {
        "subject": "PawPrints - A petition you created has been rejected",
        "template": "petition_rejected"
      },
      "petition_threshold_reached": {
        "subject": "PawPrints - Petition Reaches Signature Threshold",
        "template": "petition_threshold_reached"        
      },
      "petition_response_received": {
        "subject": "PawPrints - A petition you signed has received a response",
        "template": "petition_response_received"
      },
      "petition_status_update": {
        "subject": "PawPrints - A petition you signed has a status update",
        "template": "petition_status_update"
      }
    }
  },
  ...
}

The MAIL section is new. I pushed the old MAIL_URL to MAIL.gateway_url. Also sending emails made a couple assumptions that I couldn't meet. I pulled out references to @rit.edu and made it reference the MAIL.default_domain variable instead. Also, we don't follow username@naz.edu religiously, so in one of my previous pull requests, I store your email address in your profile, and I default to using that if it's not blank.

MAIL.templates is a map of keys with the values you'd expect to find in the Email.send function. The new Mailer object uses this map to determine what the dictionary passed to Email.send should be. That class is pretty well commented, so I'd look there first.

The template variable is used to process a template loaded by the https://github.com/meteorhacks/meteor-ssr package. I use this to override email text in my external package, and because it's just nicer working with templates when you're sending email.

This change accomplishes a couple things. Firstly, I can now override all aspects of an email, I can have individualized from, to, subject, and text from an external package. Another thing which isn't included in this push, is that I can override any setting from the Mailer object. For example, I can change the to address to always be web_tech@naz.edu on development, so I don't accidentally send emails to the wrong person. This would be accomplished by having an overrides section that is similar to the default section, but would override any settings, even ones passed in by the user.

Please give me your thoughts on this, I will push out a change or two tomorrow.

ceko commented 8 years ago

This also has a fix for https://github.com/ritstudentgovernment/petitions/issues/87 in it

ceko commented 8 years ago

I updated the settings.json.sample file to include the new settings, and I added a __comments key... which was the best I could do to get valid comments in a JSON file. I also added template_overrides to the templates section and moved server-side template compilation to startup rather than having compilation happen on-demand. This gives the benefit of catching errors on server startup rather than at runtime.

Defaults and overrides are no longer children of Meteor.settings.MAIL.templates and are under Meteor.settings.MAIL.template_defaults and Meteor.settings.MAIL.template_overrides instead. Feature-wise I'm satisfied with how this class works.

When I've finished customizing the base Petitions app I will include examples on how I'm overriding templates and emails. Maybe we can even open-source our package.

NathanCastle commented 8 years ago

Looks like an awesome improvement! I like the new consistency with how emails are handled.