diff --git a/Gemfile b/Gemfile index 8421175b9..2daae25ee 100644 --- a/Gemfile +++ b/Gemfile @@ -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 diff --git a/Gemfile.lock b/Gemfile.lock index 57e9cc5db..d3ae2d938 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) diff --git a/app/lib/constants/site_config.rb b/app/lib/constants/site_config.rb index bd99ee250..f3535912a 100644 --- a/app/lib/constants/site_config.rb +++ b/app/lib/constants/site_config.rb @@ -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: { diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 5077b71e4..7dd8f8382 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -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 diff --git a/app/models/email_message.rb b/app/models/email_message.rb index ea00da3a4..e98c266f7 100644 --- a/app/models/email_message.rb +++ b/app/models/email_message.rb @@ -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("") + 6 - content[doctype_index..closing_html_index] + closing_body_index = content.index("") + 6 + content[doctype_index..closing_body_index] end def self.find_for_reports(feedback_message_ids) diff --git a/app/models/site_config.rb b/app/models/site_config.rb index a8b1e5160..b1bc480c0 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -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 diff --git a/app/services/email_digest_article_collector.rb b/app/services/email_digest_article_collector.rb index e42d713b3..32dae552b 100644 --- a/app/services/email_digest_article_collector.rb +++ b/app/services/email_digest_article_collector.rb @@ -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 diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index 23f2d092c..17255a575 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -652,21 +652,12 @@
- <%= 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] %> -
- -
- <%= 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] %>
<%= render "form_submission", f: f %> diff --git a/app/views/admin/email_messages/show.html.erb b/app/views/admin/email_messages/show.html.erb index e975943ee..5ff31921d 100644 --- a/app/views/admin/email_messages/show.html.erb +++ b/app/views/admin/email_messages/show.html.erb @@ -24,13 +24,9 @@ Sent at <%= @email.sent_at&.strftime("%b %e '%y") %> - - Opened At - <%= @email.opened_at&.strftime("%b %e '%y") %> - UTM Campaign - <%= @email.utm_campaign %> + <%= @email&.utm_campaign %> Mailer diff --git a/config/initializers/ahoy_email.rb b/config/initializers/ahoy_email.rb index 7da73b1fd..2ae16eeb8 100644 --- a/config/initializers/ahoy_email.rb +++ b/config/initializers/ahoy_email.rb @@ -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 diff --git a/db/migrate/20210323145228_drop_ahoy_messages_opened_at.rb b/db/migrate/20210323145228_drop_ahoy_messages_opened_at.rb new file mode 100644 index 000000000..38ced4564 --- /dev/null +++ b/db/migrate/20210323145228_drop_ahoy_messages_opened_at.rb @@ -0,0 +1,7 @@ +class DropAhoyMessagesOpenedAt < ActiveRecord::Migration[6.0] + def change + safety_assured do + remove_column :ahoy_messages, :opened_at + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c425c4c36..5379512eb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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" diff --git a/spec/mailers/digest_mailer_spec.rb b/spec/mailers/digest_mailer_spec.rb index 7393589f2..f6294a504 100644 --- a/spec/mailers/digest_mailer_spec.rb +++ b/spec/mailers/digest_mailer_spec.rb @@ -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 diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index 7bfc62624..1367f453b 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -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