From b6fa936e362ea4bbf797650462ccc99a47a55744 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Mon, 1 Apr 2024 15:05:04 -0400 Subject: [PATCH] Customize ahoy email to track clicks async without redirect (#20809) * Initial controller creation * Ahoy custom clicks initial WIP * Add test for success and failure * Adjust some tests * Adjust test * Adjust tests and some code * Update app/views/mailers/digest_mailer/digest_email.html.erb * Remove s/t/u from safe params * Add simple test * Update app/controllers/ahoy/custom_email_clicks_controller.rb Co-authored-by: Mac Siri * Change to just email clicks * Email clicks controller test --------- Co-authored-by: Mac Siri --- .../ahoy/email_clicks_controller.rb | 35 +++++++++++++ app/javascript/packs/baseTracking.js | 30 +++++++++++ config/fastly/snippets/safe_params_list.vcl | 2 +- config/initializers/ahoy_email.rb | 52 +++++++++++++++++++ config/routes.rb | 9 ++++ spec/mailers/devise_mailer_spec.rb | 4 +- spec/mailers/notify_mailer_spec.rb | 18 ++++--- spec/requests/ahoy/email_clicks_spec.rb | 44 ++++++++++++++++ 8 files changed, 184 insertions(+), 10 deletions(-) create mode 100644 app/controllers/ahoy/email_clicks_controller.rb create mode 100644 spec/requests/ahoy/email_clicks_spec.rb diff --git a/app/controllers/ahoy/email_clicks_controller.rb b/app/controllers/ahoy/email_clicks_controller.rb new file mode 100644 index 000000000..96e56e925 --- /dev/null +++ b/app/controllers/ahoy/email_clicks_controller.rb @@ -0,0 +1,35 @@ +module Ahoy + class EmailClicksController < ApplicationController + skip_before_action :verify_authenticity_token # Signitures are used to verify requests here + before_action :verify_signature + + def create + data = { + token: @token, + campaign: @campaign, + url: @url, + controller: self + } + AhoyEmail::Utils.publish(:click, data) + head :ok # Renders a blank response with a 200 OK status + end + + private + + def verify_signature + @token = ahoy_params[:t].to_s + @campaign = ahoy_params[:c].to_s + @url = ahoy_params[:u].to_s + @signature = ahoy_params[:s].to_s + expected_signature = AhoyEmail::Utils.signature(token: @token, campaign: @campaign, url: @url) + + return if ActiveSupport::SecurityUtils.secure_compare(@signature, expected_signature) + + render plain: "Invalid signature", status: :forbidden + end + + def ahoy_params + params.permit(:t, :c, :u, :s) + end + end +end diff --git a/app/javascript/packs/baseTracking.js b/app/javascript/packs/baseTracking.js index 2adeef299..2ce6abaae 100644 --- a/app/javascript/packs/baseTracking.js +++ b/app/javascript/packs/baseTracking.js @@ -6,6 +6,7 @@ function initializeBaseTracking() { trackGoogleAnalytics3(); trackGoogleAnalytics4(); trackCustomImpressions(); + trackEmailClicks(); } // Google Anlytics 3 is deprecated, and mostly not supported, but some sites may still be using it for now. @@ -187,6 +188,35 @@ function trackCustomImpressions() { }, 1800) } +function trackEmailClicks() { + const urlParams = new URLSearchParams(window.location.search); + + if (urlParams.get('ahoy_click') === 'true' && urlParams.get('t') && urlParams.get('s') && urlParams.get('u')){ + const dataBody = { + t: urlParams.get('t'), + c: urlParams.get('c'), + u: decodeURIComponent(urlParams.get('u')), + s: urlParams.get('s'), + }; + window.fetch('/ahoy/email_clicks', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(dataBody), + credentials: 'same-origin' + }); + // Remove t,c,u,s params and ahoy_click param from url without modifying the history + urlParams.delete('t'); + urlParams.delete('c'); + urlParams.delete('u'); + urlParams.delete('s'); + urlParams.delete('ahoy_click'); + const newUrl = `${window.location.pathname }?${ urlParams.toString()}`; + window.history.replaceState({}, null, newUrl); + } +} + function showCookieConsentBanner() { // if current url includes ?cookietest=true if (shouldShowCookieBanner()) { diff --git a/config/fastly/snippets/safe_params_list.vcl b/config/fastly/snippets/safe_params_list.vcl index fa5dcb014..695f6ecbb 100644 --- a/config/fastly/snippets/safe_params_list.vcl +++ b/config/fastly/snippets/safe_params_list.vcl @@ -2,6 +2,6 @@ import querystring; sub vcl_recv { # return this URL with only the parameters that match this regular expression if (req.url !~ "/admin/" && req.url !~ "/search/" && req.url !~ "/bulk_show") { - set req.url = querystring.regfilter_except(req.url, "^(a_id|args|article_id|article_ids|articles|asc|callback_url|category|client_id|code|collection_id|commentable_id|commentable_type|confirmation_token|created_at|end|email|filter|followable_id|followable_type|forem_owner_secret|fork_id|i|key|message_offset|name|oauth_token|oauth_verifier|offset|onboarding|org_id|organization_id|p|page|per_page|p_id|placement_area|prefill|preview|purchaser|q|reactable_ids|redirect_uri|reported_url|reporter_username|response_type|scope|search|signature|sort|source_id|source_type|start|state|status|tag|tag_list|top|type_of|url|username|invitation_token|reset_password_token|ut|verb|invitation_slug|period|comments_sort|billboard|controller_action|bb_test_placement_area|cookies_allowed|members|bb_test_id|s|t|u)$"); + set req.url = querystring.regfilter_except(req.url, "^(a_id|args|article_id|article_ids|articles|asc|callback_url|category|client_id|code|collection_id|commentable_id|commentable_type|confirmation_token|created_at|end|email|filter|followable_id|followable_type|forem_owner_secret|fork_id|i|key|message_offset|name|oauth_token|oauth_verifier|offset|onboarding|org_id|organization_id|p|page|per_page|p_id|placement_area|prefill|preview|purchaser|q|reactable_ids|redirect_uri|reported_url|reporter_username|response_type|scope|search|signature|sort|source_id|source_type|start|state|status|tag|tag_list|top|type_of|url|username|invitation_token|reset_password_token|ut|verb|invitation_slug|period|comments_sort|billboard|controller_action|bb_test_placement_area|cookies_allowed|members|bb_test_id)$"); } } diff --git a/config/initializers/ahoy_email.rb b/config/initializers/ahoy_email.rb index 8e4af1f74..96d4fe44e 100644 --- a/config/initializers/ahoy_email.rb +++ b/config/initializers/ahoy_email.rb @@ -3,3 +3,55 @@ AhoyEmail.api = false AhoyEmail.default_options[:click] = Rails.env.production? ? ENV["AHOY_EMAIL_CLICK_ON"] == "YES" : true AhoyEmail.default_options[:utm_params] = false AhoyEmail.default_options[:message] = true + +# Monkeypatch to make track_links work async instead of as a blocking route. +require "ahoy_email" + +module AhoyEmail + class Processor + protected + + # rubocop:disable Metrics/CyclomaticComplexity + def track_links + return unless html_part? + + part = message.html_part || message + + doc = Nokogiri::HTML::Document.parse(part.body.raw_source) + doc.css("a[href]").each do |link| + uri = parse_uri(link["href"]) + next unless trackable?(uri) + + if options[:utm_params] && !skip_attribute?(link, "utm-params") + existing_params = uri.query_values(Array) || [] + UTM_PARAMETERS.each do |key| + next if existing_params.any? { |k, _v| k == key } || !options[key.to_sym] + existing_params << [key, options[key.to_sym]] + end + uri.query_values = existing_params + end + + if options[:click] && !skip_attribute?(link, "click") + signature = Utils.signature(token: token, campaign: campaign, url: link["href"]) + tracking_params = { + "ahoy_click" => true, + "t" => token, + "s" => signature, + "u" => CGI.escape(link["href"]), + "c" => campaign + }.reject { |_k, v| v.nil? || v.to_s.empty? } + + # Merge the existing and new tracking params + all_params = (uri.query_values(Array) || []) + tracking_params.to_a + uri.query_values = all_params + + # Preserve the port if present, especially for localhost in development + port_part = uri.port ? ":#{uri.port}" : "" + link["href"] = "#{uri.scheme}://#{uri.host}#{port_part}#{uri.path}" + link["href"] += "?#{uri.query}" unless uri.query.nil? || uri.query.empty? + end + end + part.body = doc.to_s.gsub("&", "&") + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 7123fe4a4..d621ec434 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,8 +16,17 @@ Rails.application.routes.draw do get "/confirm-email", to: "confirmations#new" delete "/sign_out", to: "devise/sessions#destroy" end + + # This route makes default Ahoy Email redirect URLs available to us + # However, we monkeypatch this behavior in config/initializers/ahoy_email.rb so this + # routes is not currently where emails pass through. mount AhoyEmail::Engine => "/ahoy" + # Custom controller for tracking clicks asynchronously + namespace :ahoy do + post "email_clicks", to: "email_clicks#create" + end + get "/r/mobile", to: "deep_links#mobile" get "/.well-known/apple-app-site-association", to: "deep_links#aasa" diff --git a/spec/mailers/devise_mailer_spec.rb b/spec/mailers/devise_mailer_spec.rb index 6dcc52af6..060ea307b 100644 --- a/spec/mailers/devise_mailer_spec.rb +++ b/spec/mailers/devise_mailer_spec.rb @@ -40,7 +40,7 @@ RSpec.describe DeviseMailer do end it "renders proper URL" do - expect(email.body.to_s).to include("confirmation_token%3Dfaketoken") # encoded URL + expect(email.body.to_s).to include("confirmation_token=faketoken") # encoded URL end end @@ -52,7 +52,7 @@ RSpec.describe DeviseMailer do end it "renders proper URL" do - expect(email.body.to_s).to include("confirmation_token%3Dfaketoken") + expect(email.body.to_s).to include("confirmation_token=faketoken") end end end diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index 043b1e345..4f2932dcb 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -124,7 +124,7 @@ RSpec.describe NotifyMailer do context "when rendering the HTML email for badge w/o credits" do it "includes the user URL" do - expect(email.html_part.body).to include("%2F#{user.username}") + expect(email.html_part.body).to include("/#{user.username}") end it "doesn't include the listings URL" do @@ -141,23 +141,27 @@ RSpec.describe NotifyMailer do ) end + it "includes the click tracking parameters" do + expect(email.html_part.body).to include("/hey?ahoy_click=true&t=") + end + it "includes the rewarding_context_message in the email" do expect(email.html_part.body).to include("Hello