-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a wrapper around the Mailjet API #57580
Changes from 10 commits
9ba3d34
34d9b17
8c8820b
540cb59
d5ae00d
85a4fdc
32125a0
ade5644
ebad19e
fe6573c
e26e532
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
require 'mailjet' | ||
require 'honeybadger/ruby' | ||
require_relative './shared_constants/mailjet_constants' | ||
|
||
module MailJet | ||
include MailJetConstants | ||
|
||
API_KEY = CDO.try(:mailjet_api_key).freeze | ||
SECRET_KEY = CDO.try(:mailjet_secret_key).freeze | ||
|
||
# We use MailJet when the following are true: | ||
# - The use_mailjet DCDO is true | ||
# - We are not in the test environment | ||
# - We have the required secrets set | ||
def self.enabled? | ||
DCDO.get('use_mailjet', false) && | ||
!Rails.env.test? && | ||
API_KEY.present? && | ||
SECRET_KEY.present? | ||
end | ||
|
||
def self.subaccount | ||
case Rails.env | ||
when 'production' | ||
'production' | ||
when 'staging' | ||
'staging' | ||
else | ||
'development' | ||
end | ||
end | ||
|
||
def self.create_contact_and_send_welcome_email(user) | ||
return unless enabled? | ||
|
||
return unless user&.id.present? | ||
return unless user.teacher? | ||
|
||
create_contact(user.email, user.name, user.created_at.to_datetime) | ||
send_template_email( | ||
user.email, | ||
user.name, | ||
EMAILS[:welcome] | ||
) | ||
end | ||
|
||
def self.create_contact(email, name, sign_up_date) | ||
return unless enabled? | ||
|
||
Mailjet.configure do |config| | ||
config.api_key = API_KEY | ||
config.secret_key = SECRET_KEY | ||
config.api_version = "v3" | ||
end | ||
|
||
contact = Mailjet::Contactdata.find(email) | ||
|
||
if contact&.id.nil? | ||
Mailjet::Contact.create( | ||
is_excluded_from_campaigns: true, | ||
email: email, | ||
name: name | ||
) | ||
contact = Mailjet::Contactdata.find(email) | ||
end | ||
|
||
sign_up_date_rfc3339 = sign_up_date.rfc3339 | ||
contact.update_attributes( | ||
data: [ | ||
{ | ||
name: 'sign_up_date', | ||
value: sign_up_date_rfc3339 | ||
} | ||
] | ||
) | ||
end | ||
|
||
def self.send_template_email(to_email, to_name, email_config) | ||
return unless enabled? | ||
|
||
Mailjet.configure do |config| | ||
config.api_key = API_KEY | ||
config.secret_key = SECRET_KEY | ||
config.api_version = "v3.1" | ||
end | ||
|
||
from_address = email_config[:from_address] | ||
from_name = email_config[:from_name] | ||
template_id = email_config[:template_id][subaccount.to_sym] | ||
|
||
Mailjet::Send.create(messages: | ||
[{ | ||
From: { | ||
Email: from_address, | ||
Name: from_name | ||
}, | ||
To: [ | ||
{ | ||
Email: to_email, | ||
Name: to_name | ||
} | ||
], | ||
TemplateID: template_id, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would there be value in enforcing that the template_id be found in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean to push the fetching of the template ID down to this function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just noticing that there's nothing to force future developers to use to use the template listing instead of hardcoding template ID's elsewhere in the codebase. Once we have examples of using the wrapper, the convention might be more clear though. |
||
TemplateLanguage: true, | ||
}] | ||
) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
module MailJetConstants | ||
EMAILS = { | ||
welcome: { | ||
template_id: { | ||
production: 5_421_128, | ||
staging: 5_826_411, | ||
development: 5_808_078 | ||
}, | ||
from_address: 'hadi_partovi@code.org', | ||
from_name: 'Hadi Partovi', | ||
} | ||
}.freeze | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
require_relative '../test_helper' | ||
require 'cdo/mailjet' | ||
|
||
class MailJetTest < Minitest::Test | ||
def setup | ||
MailJet.stubs(:enabled?).returns(true) | ||
end | ||
|
||
def test_create_contact | ||
email = 'fake.email@email.com' | ||
name = 'Fake Name' | ||
|
||
Mailjet::Contact.expects(:create).with(is_excluded_from_campaigns: true, email: email, name: name) | ||
|
||
mock_contact = mock('Mailjet::Contactdata') | ||
Mailjet::Contactdata.stubs(:find).with(email).returns(nil).then.returns(mock_contact) | ||
|
||
time = Time.now.to_datetime | ||
mock_contact.expects(:update_attributes).with(data: [{name: 'sign_up_date', value: time.rfc3339}]) | ||
|
||
MailJet.create_contact(email, name, time) | ||
end | ||
|
||
def test_create_contact_with_existing_contact | ||
email = 'fake.email@email.com' | ||
name = 'Fake Name' | ||
|
||
mock_existing_contact = mock('Mailjet::Contactdata') | ||
mock_existing_contact.stubs(:id).returns(123) | ||
Mailjet::Contactdata.expects(:find).with(email).returns(mock_existing_contact) | ||
|
||
Mailjet::Contact.expects(:create).never | ||
|
||
time = Time.now.to_datetime | ||
mock_existing_contact.expects(:update_attributes).with(data: [{name: 'sign_up_date', value: time.rfc3339}]) | ||
|
||
MailJet.create_contact(email, name, time) | ||
end | ||
|
||
def test_send_template_email | ||
to_email = 'fake.email@email.com' | ||
to_name = 'Fake Name' | ||
from_address = 'test@code.org' | ||
from_name = 'Test Name' | ||
template_id = 123 | ||
|
||
MailJet.stubs(:subaccount).returns('unit_test') | ||
|
||
email_config = { | ||
from_address: from_address, | ||
from_name: from_name, | ||
template_id: { | ||
unit_test: template_id | ||
} | ||
} | ||
|
||
Mailjet::Send.expects(:create).with do |params| | ||
messages = params[:messages] | ||
messages.length == 1 && | ||
messages[0][:From][:Email] == from_address && | ||
messages[0][:From][:Name] == from_name && | ||
messages[0][:To].length == 1 && | ||
messages[0][:To][0][:Email] == to_email && | ||
messages[0][:To][0][:Name] == to_name && | ||
messages[0][:TemplateID] == template_id | ||
end | ||
|
||
MailJet.send_template_email(to_email, to_name, email_config) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was re-reading the tech plan and was reminded that we're explicitly planning on NOT connecting to mailjet for Test and Drone. Is there a good place to make that intention clear in the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah -- I could add a check in
enabled?
. Let me do that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, sorry, I meant clear to future developers wondering why production and staging are getting values but not anything else (because we don't have a standard convention or clear guidance about the different environments)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh gotcha! Yeah I can add something here as well