Skip to content

Commit

Permalink
Implement periodic email digest (forem#123)
Browse files Browse the repository at this point in the history
* Create EmailDigest WIP

* Add email_digest_periodic to User model

* Add email digest setting to notification

* Draft thoughts (?)  WIP

* Implement EmailDigest's features

* Remove hard-coded email digest vars

* Create spec for EmailDigest

* Fix something

* Temp work

* Run migration & Yarn

* Move email logic out of EmailDigest

* Improve email logic

* Improve EmailLogic'specs

* Update copy

* Change positive_reactions_count limit

For non-followed articles we should have a higher limit. So this change modifies that.
  • Loading branch information
maestromac authored and benhalpern committed Mar 27, 2018
1 parent c1b0d08 commit 9d32af7
Show file tree
Hide file tree
Showing 13 changed files with 289 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Guardfile
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ rspec_options = {
# fast but no coverage
cmd: "bin/spring rspec -p",
#############################
failed_mode: :none,
failed_mode: :focus,
bundler_env: :clean_env
}

Expand Down
1 change: 1 addition & 0 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def user_params
website_url
summary
email_newsletter
email_digest_periodic
email_membership_newsletter
email_comment_notifications
email_mention_notifications
Expand Down
29 changes: 29 additions & 0 deletions app/labor/email_digest.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Usecase would be
# EmailDigest.send_periodic_digest_email
# OR
# EmailDigets.send_periodic_digest_email(Users.first(4))

class EmailDigest
def self.send_periodic_digest_email(users = [])
new(users).send_periodic_digest_email
end

def initialize(users = [])
@users = users.empty? ? get_users : users
end

def send_periodic_digest_email
@users.each do |user|
user_email_heuristic = EmailLogic.new(user).analyze
next unless user_email_heuristic.should_receive_email?
articles = user_email_heuristic.articles_to_send
DigestMailer.daily_digest(user, articles).deliver
end
end

private

def get_users
User.where(email_digest_periodic: true)
end
end
92 changes: 92 additions & 0 deletions app/labor/email_logic.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
class EmailLogic
attr_reader :open_percentage, :last_email_sent_at,
:days_until_next_email, :articles_to_send

def initialize(user)
@user = user
@open_percentage = nil
@ready_to_receive_email = nil
@last_email_sent_at = nil
@days_until_next_email = nil
@articles_to_send = []
end

def analyze
@last_email_sent_at = get_last_digest_email_user_recieved
@open_percentage = get_open_rate
@days_until_next_email = get_days_until_next_email
@ready_to_receive_email = get_user_readiness
if @ready_to_receive_email
@articles_to_send = get_articles_to_send
end
self
end

def should_receive_email?
@ready_to_receive_email
end

private

def get_articles_to_send
fresh_date = get_fresh_date
articles = if user_has_followings?
@user.followed_articles.
where("created_at > ?", fresh_date).
where("positive_reactions_count > ?", 15).
order("positive_reactions_count DESC").
limit(6)
else
Article.
where("positive_reactions_count > ?", 30).
order("positive_reactions_count DESC").
limit(6)
end
if articles.length < 3
@ready_to_receive_email = false
end
articles
end

def get_days_until_next_email
# Relies on hyperbolic tangent function to model the frequency of the digest email
max_day = ENV["PERIODIC_EMAIL_DIGEST_MAX"].to_i
min_day = ENV["PERIODIC_EMAIL_DIGEST_MIN"].to_i
result = max_day * (1 - Math.tanh(2 * @open_percentage))
result = result.round
result < min_day ? min_day : result
end

def get_open_rate
past_sent_emails = @user.email_messages.where(mailer: "DigestMailer#daily_digest").limit(10)

# Will stick with 50% open rate if @user has no/not-enough email digest history
return 0.5 if past_sent_emails.length < 10

past_sent_emails_count = past_sent_emails.count
past_opened_emails_count = past_sent_emails.where("opened_at IS NOT NULL").count
past_opened_emails_count / past_sent_emails_count
end

def get_user_readiness
return true unless @last_email_sent_at
# Has it been atleast x days since @user receive an email?
(Time.now.utc - @last_email_sent_at) >= @days_until_next_email.days.to_i
end

def get_last_digest_email_user_recieved
@user.email_messages.where(mailer: "DigestMailer#daily_digest").last&.sent_at
end

def get_fresh_date
a_month_ago = 1.month.ago.utc
return a_month_ago unless @last_email_sent_at
a_month_ago > @last_email_sent_at ? a_month_ago : @last_email_sent_at
end

def user_has_followings?
following_users = @user.cached_following_users_ids
following_tags = @user.cached_followed_tag_names
!following_users.empty? || !following_tags.empty?
end
end
5 changes: 5 additions & 0 deletions app/mailers/digest_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class DigestMailer < ApplicationMailer
def daily_digest(recipient)
@recipient = recipient
end
end
8 changes: 6 additions & 2 deletions app/views/users/_notifications.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
<div class="checkbox-field">
<div class="sub-field">
<%= f.check_box :email_newsletter %>
<%= f.label :email_newsletter, "Send me periodic newsletter emails" %>
<%= f.label :email_newsletter, "Send me weekly newsletter emails" %>
</div>
<div class="sub-field">
<%= f.check_box :email_digest_periodic %>
<%= f.label :email_digest_periodic, "Send me a periodic digest of top posts from my tags" %>
</div>
<% if current_user.a_sustaining_member? %>
<div class="sub-field">
<%= f.check_box :email_membership_newsletter %>
<%= f.label :email_membership_newsletter, "Send me membership newsletter emails" %>
<%= f.label :email_membership_newsletter, "Send me sustaining membership newsletter emails" %>
</div>
<% end %>
<div class="sub-field">
Expand Down
2 changes: 2 additions & 0 deletions config/initializers/_env_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
"KEEN_WRITE_KEY",
"MAILCHIMP_API_KEY",
"MAILCHIMP_NEWSLETTER_ID",
"PERIODIC_EMAIL_DIGEST_MAX",
"PERIODIC_EMAIL_DIGEST_MIN",
"RECAPTCHA_SECRET",
"RECAPTCHA_SITE",
"SERVICE_TIMEOUT",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddEmailDigestPeriodicToUser < ActiveRecord::Migration[5.1]
def change
add_column :users, :email_digest_periodic, :boolean, default: true, null: false
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20180316174324) do
ActiveRecord::Schema.define(version: 20180321170500) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -618,6 +618,7 @@
t.string "website_url"
t.datetime "workshop_expiration"
t.boolean "permit_adjacent_sponsors", default: true
t.boolean "email_digest_periodic", default: true, null: false
t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
t.index ["language_settings"], name: "index_users_on_language_settings", using: :gin
t.index ["organization_id"], name: "index_users_on_organization_id"
Expand Down
1 change: 1 addition & 0 deletions spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
confirmed_at { Time.now }
saw_onboarding { true }
signup_cta_variant { "navbar_basic" }
email_digest_periodic { false }

trait :super_admin do
after(:build) { |user| user.add_role(:super_admin) }
Expand Down
37 changes: 37 additions & 0 deletions spec/labor/email_digest_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
require "rails_helper"

class FakeDelegator < ActionMailer::MessageDelivery
# TODO: we should replace all usage of .deliver to .deliver_now
def deliver(*args); super end
end

RSpec.describe EmailDigest do
let(:user) { create(:user, email_digest_periodic: true) }
let(:author) { create(:user) }
let(:mock_delegator) { instance_double("FakeDelegator") }

before do
allow(DigestMailer).to receive(:daily_digest) { mock_delegator }
allow(mock_delegator).to receive(:deliver).and_return(true)
Delayed::Worker.delay_jobs = false
user
end

after do
Delayed::Worker.delay_jobs = true
end

describe "::send_daily_digest" do
context "when there's article to be sent" do
before { user.follow(author) }

it "send digest email when there's atleast 3 hot articles" do
3.times { create(:article, user_id: author.id, positive_reactions_count: 20) }
described_class.send_periodic_digest_email
expect(DigestMailer).to have_received(:daily_digest).with(
user, [instance_of(Article), instance_of(Article), instance_of(Article)]
)
end
end
end
end
77 changes: 77 additions & 0 deletions spec/labor/email_logic_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
require "rails_helper"

RSpec.describe EmailLogic do
let(:user) { create(:user) }

# TODO: improve this test suite, and improve it's speed

describe "#analyze" do
context "when user is brand new with no-follow" do
it "returns 0.5 for open_percentage" do
author = create(:user)
user.follow(author)
3.times { create(:article, user_id: author.id, positive_reactions_count: 20) }
h = described_class.new(user).analyze
expect(h.open_percentage).to eq(0.5)
end

it "provides top 3 articles" do
3.times { create(:article, positive_reactions_count: 20) }
h = described_class.new(user).analyze
expect(h.articles_to_send.length).to eq(3)
end

it "marks as not ready if there isn't atleast 3 articles" do
2.times { create(:article, positive_reactions_count: 20) }
h = described_class.new(user).analyze
expect(h.should_receive_email?).to eq(false)
end
end

context "when a user's open_percentage is low " do
before do
author = create(:user)
user.follow(author)
3.times { create(:article, user_id: author.id, positive_reactions_count: 20) }
10.times do
Ahoy::Message.create(mailer: "DigestMailer#daily_digest",
user_id: user.id, sent_at: Time.now.utc)
end
end

it "will not send email when user shouldn't receive any" do
h = described_class.new(user).analyze
expect(h.should_receive_email?).to eq(false)
end
end

context "when a user's open_percentage is high" do
before do
10.times do
Ahoy::Message.create(mailer: "DigestMailer#daily_digest", user_id: user.id,
sent_at: Time.now.utc, opened_at: Time.now.utc)
author = create(:user)
user.follow(author)
3.times { create(:article, user_id: author.id, positive_reactions_count: 20) }
end
end

it "evaluates that user is ready to recieve an email" do
Timecop.freeze(Date.today + 3) do
h = described_class.new(user).analyze
expect(h.should_receive_email?).to eq(true)
end
end
end
end

describe "#should_receive_email?" do
it "refelcts @ready_to_receive_email" do
author = create(:user)
user.follow(author)
3.times { create(:article, user_id: author.id, positive_reactions_count: 20) }
h = described_class.new(user).analyze
expect(h.should_receive_email?).to eq(true)
end
end
end
Loading

0 comments on commit 9d32af7

Please sign in to comment.