maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Upgrade to Ahoy 2.0.x (#13085)

Browse source 
* First set of changes for Ahoy 2.0

* Fix line length

* Change test

* Remove unused test

* Remove ahoy_messages.opened_at

* Simply digest email sending calculation to not rely on last_opened

* Remove outdated comment

* Fix typo in test

* Rework site config email digest to a single value

* Fix merge duplication

* Remove UTM references from ahoy emails

* Fix credits tests

* Remove UTM from expected email params
This commit is contained in:
Josh Puetz 2021-04-06 10:12:32 -05:00 • committed by GitHub
parent a5b2d109d5
commit f6dc638190
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 55 additions and 238 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Gemfile
View file

@ -14,7 +14,7 @@ gem "active_record_union", "~> 1.3" # Adds proper union and union_all methods to
gem "acts-as-taggable-on", "~> 7.0" # A tagging plugin for Rails applications that allows for custom tagging along dynamic contexts
gem "acts_as_follower", github: "forem/acts_as_follower", branch: "master" # Allow any model to follow any other model
gem "addressable", "~> 2.7" # A replacement for the URI implementation that is part of Ruby's standard library
gem "ahoy_email", "~> 1.1" # Email analytics for Rails
gem "ahoy_email", "~> 2.0.2" # Email analytics for Rails
gem "ahoy_matey", "~> 3.2" # Tracking analytics for Rails
gem "ancestry", "~> 3.2" # Ancestry allows the records of a ActiveRecord model to be organized in a tree structure
gem "blazer", "~> 2.4.2" # Allows admins to query data

4
Gemfile.lock
View file

@ -86,7 +86,7 @@ GEM
activerecord (>= 5.0, < 6.2)
addressable (2.7.0)
public_suffix (>= 2.0.2, < 5.0)
ahoy_email (1.1.1)
ahoy_email (2.0.2)
actionmailer (>= 5)
addressable (>= 2.3.2)
nokogiri
@ -849,7 +849,7 @@ DEPENDENCIES
acts-as-taggable-on (~> 7.0)
acts_as_follower!
addressable (~> 2.7)
ahoy_email (~> 1.1)
ahoy_email (~> 2.0.2)
ahoy_matey (~> 3.2)
amazing_print (~> 1.3)
ancestry (~> 3.2)

8
app/lib/constants/site_config.rb
View file

@ -254,12 +254,8 @@ module Constants
"See: https://github.com/thepracticaldev/dev.to/pull/6345",
placeholder: "$pay.somethinglikethis.co/value"
},
periodic_email_digest_max: {
description: "Determines the maximum for the periodic email digest",
placeholder: 0
},
periodic_email_digest_min: {
description: "Determines the mininum for the periodic email digest",
periodic_email_digest: {
description: "Determines how often periodic email digests are sent",
placeholder: 2
},
recaptcha_site_key: {

8
app/mailers/notify_mailer.rb
View file

@ -1,4 +1,7 @@
class NotifyMailer < ApplicationMailer
has_history extra: -> { { feedback_message_id: params[:feedback_message_id] } },
only: :feedback_message_resolution_email
def new_reply_email
@comment = params[:comment]
@user = @comment.parent_user
@ -66,9 +69,6 @@ class NotifyMailer < ApplicationMailer
@user = User.find_by(email: params[:email_to])
@email_body = params[:email_body]
track utm_campaign: params[:email_type]
track extra: { feedback_message_id: params[:feedback_message_id] }
mail(to: params[:email_to], subject: params[:email_subject])
end
@ -76,8 +76,6 @@ class NotifyMailer < ApplicationMailer
@user = User.find(params[:user_id])
@email_body = params[:email_body]
track utm_campaign: "user_contact"
mail(to: @user.email, subject: params[:email_subject])
end

4
app/models/email_message.rb
View file

@ -6,8 +6,8 @@ class EmailMessage < Ahoy::Message
# reasons to define behavior here, similar to how we use the Tag model.
def body_html_content
doctype_index = content.index("<!DOCTYPE")
closing_html_index = content.index("</html>") + 6
content[doctype_index..closing_html_index]
closing_body_index = content.index("</body>") + 6
content[doctype_index..closing_body_index]
end
def self.find_for_reports(feedback_message_ids)

3
app/models/site_config.rb
View file

@ -89,8 +89,7 @@ class SiteConfig < RailsSettings::Base
}
# Email digest frequency
field :periodic_email_digest_max, type: :integer, default: 2
field :periodic_email_digest_min, type: :integer, default: 0
field :periodic_email_digest, type: :integer, default: 2
# Jobs
field :jobs_url, type: :string

28
app/services/email_digest_article_collector.rb
View file

@ -50,27 +50,7 @@ class EmailDigestArticleCollector
return true unless last_email_sent_at
# Has it been at least x days since @user received an email?
Time.current - last_email_sent_at >= days_until_next_email
end
def days_until_next_email
# Relies on hyperbolic tangent function to model the frequency of the digest email
max_day = SiteConfig.periodic_email_digest_max
min_day = SiteConfig.periodic_email_digest_min
result = max_day * (1 - Math.tanh(2 * open_rate))
result = result.round
[result, min_day].max
end
def open_rate
email_count = last_user_emails.count
# Will stick with 50% open rate if @user has no/not-enough email digest history
return 0.5 if email_count < 10
past_opened_emails_count = last_user_emails.count { |msg| msg.opened_at.present? }
past_opened_emails_count / email_count
Time.current - last_email_sent_at >= SiteConfig.periodic_email_digest
end
def last_email_sent_at
@ -89,10 +69,6 @@ class EmailDigestArticleCollector
end
def last_user_emails
@last_user_emails ||= @user
.email_messages
.select(:sent_at, :opened_at)
.where(mailer: "DigestMailer#digest_email")
.limit(10)
@last_user_emails ||= @user.email_messages.select(:sent_at).where(mailer: "DigestMailer#digest_email").limit(10)
end
end

19
app/views/admin/configs/show.html.erb
View file

@ -652,21 +652,12 @@
<div id="emailDigestBodyContainer" class="card-body collapse hide" aria-labelledby="emailDigestBodyContainer">
<fieldset class="grid gap-4">
<div class="crayons-field">
<%= admin_config_label :periodic_email_digest_max %>
<%= admin_config_description Constants::SiteConfig::DETAILS[:periodic_email_digest_max][:description] %>
<%= f.number_field :periodic_email_digest_max,
<%= admin_config_label :periodic_email_digest %>
<%= admin_config_description Constants::SiteConfig::DETAILS[:periodic_email_digest][:description] %>
<%= f.number_field :periodic_email_digest,
class: "crayons-textfield",
value: SiteConfig.periodic_email_digest_max,
placeholder: Constants::SiteConfig::DETAILS[:periodic_email_digest_max][:placeholder] %>
</div>
<div class="crayons-field">
<%= admin_config_label :periodic_email_digest_min %>
<%= admin_config_description Constants::SiteConfig::DETAILS[:periodic_email_digest_max][:description] %>
<%= f.number_field :periodic_email_digest_min,
class: "crayons-textfield",
value: SiteConfig.periodic_email_digest_min,
placeholder: Constants::SiteConfig::DETAILS[:periodic_email_digest_min][:placeholder] %>
value: SiteConfig.periodic_email_digest,
placeholder: Constants::SiteConfig::DETAILS[:periodic_email_digest][:placeholder] %>
</div>
<%= render "form_submission", f: f %>

6
app/views/admin/email_messages/show.html.erb
View file

@ -24,13 +24,9 @@
<td>Sent at</td>
<td><%= @email.sent_at&.strftime("%b %e '%y") %></td>
</tr>
<tr>
<td>Opened At</td>
<td><%= @email.opened_at&.strftime("%b %e '%y") %></td>
</tr>
<tr>
<td>UTM Campaign</td>
<td><%= @email.utm_campaign %></td>
<td><%= @email&.utm_campaign %></td>
</tr>
<tr>
<td>Mailer</td>

6
config/initializers/ahoy_email.rb
View file

@ -1,5 +1,5 @@
# enable tracking for open, click and UTM params
AhoyEmail.api = true
AhoyEmail.default_options[:open] = true
AhoyEmail.default_options[:click] = true
AhoyEmail.default_options[:utm_params] = true
AhoyEmail.default_options[:click] = false
AhoyEmail.default_options[:utm_params] = false
AhoyEmail.default_options[:message] = true

7
db/migrate/20210323145228_drop_ahoy_messages_opened_at.rb Normal file
View file

@ -0,0 +1,7 @@
class DropAhoyMessagesOpenedAt < ActiveRecord::Migration[6.0]
def change
safety_assured do
remove_column :ahoy_messages, :opened_at
end
end
end

1
db/schema.rb
View file

@ -36,7 +36,6 @@ ActiveRecord::Schema.define(version: 2021_03_31_181505) do
t.text "content"
t.bigint "feedback_message_id"
t.string "mailer"
t.datetime "opened_at"
t.datetime "sent_at"
t.text "subject"
t.text "to"

12
spec/mailers/digest_mailer_spec.rb
View file

@ -18,17 +18,5 @@ RSpec.describe DigestMailer, type: :mailer do
expected_from = "#{SiteConfig.community_name} Digest <#{SiteConfig.email_addresses[:default]}>"
expect(email["from"].value).to eq(expected_from)
end
it "includes the tracking pixel" do
email = described_class.with(user: user, articles: [article]).digest_email
expect(email.body).to include("open.gif")
end
it "includes UTM params" do
email = described_class.with(user: user, articles: [article]).digest_email
expect(email.body).to include(CGI.escape("utm_medium=email"))
expect(email.body).to include(CGI.escape("utm_source=digest_mailer"))
expect(email.body).to include(CGI.escape("utm_campaign=digest_email"))
end
end
end

136
spec/mailers/notify_mailer_spec.rb
View file

@ -23,16 +23,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([comment.user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=new_reply_email"))
end
end
describe "#new_follower_email" do
@ -53,16 +43,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=new_follower_email"))
end
end
describe "#new_mention_email" do
@ -82,16 +62,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user2.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=new_mention_email"))
end
end
describe "#unread_notifications_email" do
@ -110,16 +80,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=unread_notifications_email"))
end
end
describe "#video_upload_complete_email" do
@ -138,16 +98,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([article.user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=video_upload_complete_email"))
end
end
describe "#new_badge_email" do
@ -185,15 +135,13 @@ RSpec.describe NotifyMailer, type: :mailer do
context "when rendering the HTML email for badge with credits" do
it "includes the listings URL" do
expect(email_with_credits.html_part.body).to include(
CGI.escape(
Rails.application.routes.url_helpers.listings_url(host: SiteConfig.app_domain),
),
Rails.application.routes.url_helpers.listings_url(host: SiteConfig.app_domain),
)
end
it "includes the about listings URL" do
expect(email_with_credits.html_part.body).to include(
CGI.escape(Rails.application.routes.url_helpers.about_listings_url(host: SiteConfig.app_domain)),
Rails.application.routes.url_helpers.about_listings_url(host: SiteConfig.app_domain),
)
end
@ -223,18 +171,8 @@ RSpec.describe NotifyMailer, type: :mailer do
end
context "when rendering the HTML email for badge w/o credits" do
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=new_badge_email"))
end
it "includes the user URL" do
expect(email.html_part.body).to include(CGI.escape(URL.user(user)))
expect(email.html_part.body).to include(URL.user(user))
end
it "doesn't include the listings URL" do
@ -253,21 +191,21 @@ RSpec.describe NotifyMailer, type: :mailer do
it "includes the rewarding_context_message in the email" do
expect(email.html_part.body).to include("Hello <a")
expect(email.html_part.body).to include(CGI.escape(URL.url("/hey")))
expect(email.html_part.body).to include(URL.url("/hey"))
end
it "does not include the nil rewarding_context_message in the email" do
allow(badge_achievement).to receive(:rewarding_context_message).and_return(nil)
expect(email.html_part.body).not_to include("Hello <a")
expect(email.html_part.body).not_to include(CGI.escape(URL.url("/hey")))
expect(email.html_part.body).not_to include(URL.url("/hey"))
end
it "does not include the empty rewarding_context_message in the email" do
allow(badge_achievement).to receive(:rewarding_context_message).and_return("")
expect(email.html_part.body).not_to include("Hello <a")
expect(email.html_part.body).not_to include(CGI.escape(URL.url("/hey")))
expect(email.html_part.body).not_to include(URL.url("/hey"))
end
end
@ -336,16 +274,6 @@ RSpec.describe NotifyMailer, type: :mailer do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=#{email_params[:email_type]}"))
end
it "tracks the feedback message ID after delivery" do
assert_emails 1 do
email.deliver_now
@ -400,14 +328,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_campaign=user_contact"))
end
end
describe "#new_message_email" do
@ -428,16 +348,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([direct_message.direct_receiver.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=new_message_email"))
end
end
describe "#account_deleted_email" do
@ -456,16 +366,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "does not include UTM params" do
expect(email.html_part.body).not_to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).not_to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).not_to include(CGI.escape("utm_campaign=account_deleted_email"))
end
end
describe "#export_email" do
@ -493,10 +393,6 @@ RSpec.describe NotifyMailer, type: :mailer do
expected_filename = "devto-export-#{Date.current.iso8601}.zip"
expect(email.attachments[0].filename).to eq(expected_filename)
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
end
describe "#tag_moderator_confirmation_email" do
@ -518,16 +414,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=tag_moderator_confirmation_email"))
end
end
describe "#trusted_role_email" do
@ -548,16 +434,6 @@ RSpec.describe NotifyMailer, type: :mailer do
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
it "includes the tracking pixel" do
expect(email.html_part.body).to include("open.gif")
end
it "includes UTM params" do
expect(email.html_part.body).to include(CGI.escape("utm_medium=email"))
expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer"))
expect(email.html_part.body).to include(CGI.escape("utm_campaign=trusted_role_email"))
end
end
describe "#channel_invite_email" do

16
spec/requests/admin/configs_spec.rb
View file

@ -279,24 +279,18 @@ RSpec.describe "/admin/config", type: :request do
end
describe "Email digest frequency" do
it "updates periodic_email_digest_max" do
post "/admin/config", params: { site_config: { periodic_email_digest_max: 1 },
it "updates periodic_email_digest" do
post "/admin/config", params: { site_config: { periodic_email_digest: 1 },
confirmation: confirmation_message }
expect(SiteConfig.periodic_email_digest_max).to eq(1)
end
it "updates periodic_email_digest_min" do
post "/admin/config", params: { site_config: { periodic_email_digest_min: 3 },
confirmation: confirmation_message }
expect(SiteConfig.periodic_email_digest_min).to eq(3)
expect(SiteConfig.periodic_email_digest).to eq(1)
end
it "rejects update without proper confirmation" do
expect do
post "/admin/config", params: { site_config: { periodic_email_digest_min: 6 },
post "/admin/config", params: { site_config: { periodic_email_digest: 6 },
confirmation: "Incorrect yo!" }
end.to raise_error ActionController::BadRequest
expect(SiteConfig.periodic_email_digest_min).not_to eq(6)
expect(SiteConfig.periodic_email_digest).not_to eq(6)
end
end

1
spec/requests/admin/feedback_messages_spec.rb
View file

@ -71,7 +71,6 @@ RSpec.describe "/admin/reports", type: :request do
{
feedback_message_id: send_email_params[:feedback_message_id],
subject: send_email_params[:email_subject],
utm_campaign: send_email_params[:email_type],
to: send_email_params[:email_to]
}.stringify_keys
end

32
spec/services/email_digest_article_collector_spec.rb
View file

@ -25,38 +25,36 @@ RSpec.describe EmailDigestArticleCollector, type: :service do
end
end
context "when a user's open_percentage is low " do
context "when it's been less than the set number of digest email days" do
before do
author = create(:user)
user.follow(author)
user.update(following_users_count: 1)
create_list(:article, 3, user_id: author.id, public_reactions_count: 20, score: 20)
10.times do
Ahoy::Message.create(mailer: "DigestMailer#digest_email",
user_id: user.id, sent_at: Time.current.utc)
end
Ahoy::Message.create(mailer: "DigestMailer#digest_email",
user_id: user.id, sent_at: Time.current.utc)
end
it "will return no articles when user shouldn't receive any" do
articles = described_class.new(user).articles_to_send
expect(articles).to be_empty
Timecop.freeze(SiteConfig.periodic_email_digest.days.from_now - 1) do
articles = described_class.new(user).articles_to_send
expect(articles).to be_empty
end
end
end
context "when a user's open_percentage is high" do
context "when it's been more than the set number of digest email days" do
before do
10.times do
Ahoy::Message.create(mailer: "DigestMailer#digest_email", user_id: user.id,
sent_at: Time.current.utc, opened_at: Time.current.utc)
author = create(:user)
user.follow(author)
user.update(following_users_count: 1)
create_list(:article, 3, user_id: author.id, public_reactions_count: 40, score: 40)
end
Ahoy::Message.create(mailer: "DigestMailer#digest_email", user_id: user.id,
sent_at: Time.current.utc)
author = create(:user)
user.follow(author)
user.update(following_users_count: 1)
create_list(:article, 3, user_id: author.id, public_reactions_count: 40, score: 40)
end
it "evaluates that user is ready to receive an email" do
Timecop.freeze(3.days.from_now) do
Timecop.freeze((SiteConfig.periodic_email_digest + 1).days.from_now) do
articles = described_class.new(user).articles_to_send
expect(articles).not_to be_empty
end

BIN
vendor/cache/ahoy_email-2.0.2.gem vendored Normal file
View file

Binary file not shown.