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

Remove html variants tracking (#18594)

Browse source 
* Removed code related to tracking html_variants

* Remove code related to HtmlVariant#find_for_test

* Remove spec for HtmlVariant#success_rate
This commit is contained in:
Anna Buianova 2022-10-21 14:57:20 +03:00 • committed by GitHub
parent 6df45a6c16
commit f7a93aabbb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 11 additions and 289 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
app/assets/javascripts/initializers/initializeBaseTracking.js.erb
View file

@ -130,39 +130,6 @@ function trackCustomImpressions() {
var isBot = /bot|google|baidu|bing|msn|duckduckbot|teoma|slurp|yandex/i.test(navigator.userAgent) // is crawler
var windowBigEnough = window.innerWidth > 1023
// Sidebar HTML variant tracking
var stickyNav = document.getElementById('article-show-primary-sticky-nav');
var sidebarHTMLVariant = document.getElementById('html-variant-article-show-sidebar');
if (sidebarHTMLVariant && ArticleElement && tokenMeta && !isBot && windowBigEnough) {
var dataBody = {
html_variant_id: sidebarHTMLVariant.dataset.variantId,
article_id: ArticleElement.dataset.articleId,
};
var csrfToken = tokenMeta.getAttribute('content');
trackHTMLVariantTrial(dataBody, csrfToken)
var successLinks = stickyNav.querySelectorAll('a,button'); //track all links and button clicks within nav
for(var i = 0; i < successLinks.length; i++)
{
successLinks[i].addEventListener('click', function() { trackHtmlVariantSuccess(dataBody, csrfToken) });
}
}
// Below article HTML variant tracking
var belowArticleHTMLVariant = document.getElementById('html-variant-article-show-below-article');
if (belowArticleHTMLVariant && ArticleElement && tokenMeta && !isBot && windowBigEnough) {
var dataBody = {
html_variant_id: belowArticleHTMLVariant.dataset.variantId,
article_id: ArticleElement.dataset.articleId,
};
var csrfToken = tokenMeta.getAttribute('content');
trackHTMLVariantTrial(dataBody, csrfToken)
var successLinks = belowArticleHTMLVariant.querySelectorAll('a,button'); //track all links and button clicks within nav
for(var i = 0; i < successLinks.length; i++)
{
successLinks[i].addEventListener('click', function() { trackHtmlVariantSuccess(dataBody, csrfToken) });
}
}
// page view
if (ArticleElement && tokenMeta && !isBot) {
// See https://github.com/forem/forem/blob/main/app/controllers/page_views_controller.rb
@ -197,33 +164,6 @@ function trackCustomImpressions() {
}, 1800)
}
function trackHTMLVariantTrial(dataBody, csrfToken) {
var randomNumber = Math.floor(Math.random() * 10); // 1 in 10; Only track 1 in 10 impressions
if (randomNumber === 1) {
window.fetch('/html_variant_trials', {
method: 'POST',
headers: {
'X-CSRF-Token': csrfToken,
'Content-Type': 'application/json',
},
body: JSON.stringify(dataBody),
credentials: 'same-origin',
});
}
}
function trackHtmlVariantSuccess(dataBody, csrfToken) {
window.fetch('/html_variant_successes', {
method: 'POST',
headers: {
'X-CSRF-Token': csrfToken,
'Content-Type': 'application/json',
},
body: JSON.stringify(dataBody),
credentials: 'same-origin',
})
}
function trackPageView(dataBody, csrfToken) {
window.fetch('/page_views', {
method: 'POST',

8
app/controllers/admin/html_variants_controller.rb
View file

@ -5,12 +5,10 @@ module Admin
def index
relation = if params[:state] == "mine"
current_user.html_variants.order(created_at: :desc)
elsif params[:state] == "admin"
HtmlVariant.where(published: true, approved: false).order(created_at: :desc)
elsif params[:state].present?
HtmlVariant.where(published: true, approved: true, group: params[:state]).order(success_rate: :desc)
elsif params[:state].present? && params[:state] != "admin"
HtmlVariant.where(published: true, approved: true, group: params[:state]).order(created_at: :desc)
else
HtmlVariant.where(published: true, approved: true).order(success_rate: :desc)
HtmlVariant.where(published: true, approved: true).order(created_at: :desc)
end
@html_variants = relation.includes(:user).page(params[:page]).per(30)

8
app/controllers/html_variant_successes_controller.rb
View file

@ -1,8 +0,0 @@
class HtmlVariantSuccessesController < ApplicationMetalController
include ActionController::Head
def create
HtmlVariantSuccess.create(html_variant_id: params[:html_variant_id], article_id: params[:article_id])
head :ok
end
end

8
app/controllers/html_variant_trials_controller.rb
View file

@ -1,8 +0,0 @@
class HtmlVariantTrialsController < ApplicationMetalController
include ActionController::Head
def create
HtmlVariantTrial.create!(html_variant_id: params[:html_variant_id], article_id: params[:article_id])
head :ok
end
end

1
app/controllers/pages_controller.rb
View file

@ -23,7 +23,6 @@ class PagesController < ApplicationController
end
def badge
@html_variant = HtmlVariant.find_for_test([], "badge_landing_page")
render layout: false
set_surrogate_key_header "badge_page"
end

2
app/models/article.rb
View file

@ -104,8 +104,6 @@ class Article < ApplicationRecord
has_many :context_notifications, as: :context, inverse_of: :context, dependent: :delete_all
has_many :context_notifications_published, -> { where(context_notifications: { action: "Published" }) },
as: :context, inverse_of: :context, class_name: "ContextNotification"
has_many :html_variant_successes, dependent: :nullify
has_many :html_variant_trials, dependent: :nullify
has_many :notification_subscriptions, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all
has_many :page_views, dependent: :delete_all

33
app/models/html_variant.rb
View file

@ -5,15 +5,11 @@ class HtmlVariant < ApplicationRecord
belongs_to :user, optional: true
has_many :html_variant_successes, dependent: :destroy
has_many :html_variant_trials, dependent: :destroy
before_validation :strip_whitespace
validates :group, inclusion: { in: GROUP_NAMES }
validates :html, presence: true
validates :name, uniqueness: true
validates :success_rate, presence: true
validate :no_edits
@ -21,35 +17,6 @@ class HtmlVariant < ApplicationRecord
scope :relevant, -> { where(approved: true, published: true) }
def calculate_success_rate!
# x10 because we only capture every 10th
self.success_rate = html_variant_successes.size.to_f / (html_variant_trials.size * 10.0)
save!
end
class << self
def find_for_test(tags = [], group = "article_show_below_article_cta")
tags_array = tags + ["", nil]
if rand(10) == 1 # 10% return completely random
find_random_for_test(tags_array, group)
else # 90% chance return one of the top posts
find_top_for_test(tags_array, group)
end
end
private
def find_top_for_test(tags_array, group)
where(group: group, approved: true, published: true, target_tag: tags_array)
.order(success_rate: :desc).limit(rand(1..20)).sample
end
def find_random_for_test(tags_array, group)
where(group: group, approved: true, published: true, target_tag: tags_array)
.order(Arel.sql("RANDOM()")).first
end
end
private
def no_edits

4
app/models/html_variant_success.rb
View file

@ -1,4 +0,0 @@
class HtmlVariantSuccess < ApplicationRecord
belongs_to :html_variant
belongs_to :article, optional: true
end

4
app/models/html_variant_trial.rb
View file

@ -1,4 +0,0 @@
class HtmlVariantTrial < ApplicationRecord
belongs_to :html_variant
belongs_to :article, optional: true
end

25
app/views/articles/_actions.html.erb
View file

@ -29,13 +29,13 @@
</button>
<% else %>
<%= render partial: "articles/reaction_button",
locals: {
category: :unicorn,
description: t("views.reactions.unicorn.title"),
image_path: "unicorn.svg",
image_active_path: "unicorn-filled.svg",
aria_label: t("views.reactions.unicorn.aria_label")
} %>
locals: {
category: :unicorn,
description: t("views.reactions.unicorn.title"),
image_path: "unicorn.svg",
image_active_path: "unicorn-filled.svg",
aria_label: t("views.reactions.unicorn.aria_label")
} %>
<% end %>
<%= render partial: "articles/reaction_button",
@ -116,14 +116,3 @@
</div>
</div>
</div>
<% if !user_signed_in? && @article.body_markdown.size > 900 %>
<% cache("below-article-html-variant-#{rand(20)}", expires_in: 8.hours) do %>
<% @html_variant = HtmlVariant.find_for_test(@article.cached_tag_list_array, "article_show_below_article_cta") %>
<% if @html_variant %>
<div class="html-variant-wrapper" id="html-variant-article-show-below-article" data-variant-id="<%= @html_variant.id %>">
<%= @html_variant.html.html_safe %>
</div>
<% end %>
<% end %>
<% end %>

15
app/workers/html_variants/remove_old_data_worker.rb
View file

@ -1,15 +0,0 @@
module HtmlVariants
class RemoveOldDataWorker
include Sidekiq::Job
sidekiq_options queue: :low_priority, retry: 15
def perform
HtmlVariantTrial.delete_by("created_at < ?", 2.weeks.ago)
HtmlVariantSuccess.delete_by("created_at < ?", 2.weeks.ago)
HtmlVariant.find_each do |html_variant|
html_variant.calculate_success_rate! if html_variant.html_variant_successes.any?
end
end
end
end

2
config/routes.rb
View file

@ -122,8 +122,6 @@ Rails.application.routes.draw do
resources :videos, only: %i[index create new]
resources :video_states, only: [:create]
resources :twilio_tokens, only: [:show]
resources :html_variant_trials, only: [:create]
resources :html_variant_successes, only: [:create]
resources :tag_adjustments, only: %i[create destroy]
resources :rating_votes, only: [:create]
resources :page_views, only: %i[create update]

4
config/schedule.yml
View file

@ -86,10 +86,6 @@ award_community_wellness_badges:
- ""
- award_community_wellness
- ""
remove_old_html_variant_data:
description: "Deletes old HTML variants (runs hourly on the 10th minute after the hour)"
cron: "10 * * * *"
class: "HtmlVariants::RemoveOldDataWorker"
resave_supported_tags:
description: "Resaves supported tags to recalculate scores (runs daily at 00:25 UTC)"
cron: "25 0 * * *"

10
lib/data_update_scripts/20200910140109_cleanup_published_articles_with_duplicate_user_id_title_body_markdown.rb
View file

@ -65,15 +65,7 @@ module DataUpdateScripts
# NotificationSubscription, Notification and RatingVote rows will be removed
# Poll is ignored because it's related to the liquid tag inside the article, also user's can't use polls
# TagAdjustment is ignored as there's likely no reason for article to have an adjustment moved over
models_with_a_direct_relation = [
HtmlVariantSuccess,
HtmlVariantSuccess,
HtmlVariantTrial,
PageView,
]
models_with_a_direct_relation.each do |model_class|
model_class.where(article_id: articles_to_graft_ids).update_all(article_id: article_id)
end
PageView.where(article_id: articles_to_graft_ids).update_all(article_id: article_id)
Comment
.where(commentable_type: "Article", commentable_id: articles_to_graft_ids)

6
spec/factories/html_variant_successes.rb
View file

@ -1,6 +0,0 @@
FactoryBot.define do
factory :html_variant_success do
article
html_variant
end
end

2
spec/models/article_spec.rb
View file

@ -23,8 +23,6 @@ RSpec.describe Article, type: :model do
it { is_expected.to have_many(:comments).dependent(:nullify) }
it { is_expected.to have_many(:context_notifications).dependent(:delete_all) }
it { is_expected.to have_many(:mentions).dependent(:delete_all) }
it { is_expected.to have_many(:html_variant_successes).dependent(:nullify) }
it { is_expected.to have_many(:html_variant_trials).dependent(:nullify) }
it { is_expected.to have_many(:notification_subscriptions).dependent(:delete_all) }
it { is_expected.to have_many(:notifications).dependent(:delete_all) }
it { is_expected.to have_many(:page_views).dependent(:delete_all) }

34
spec/models/html_variant_spec.rb
View file

@ -9,46 +9,12 @@ RSpec.describe HtmlVariant, type: :model do
it { is_expected.to belong_to(:user).optional }
it { is_expected.to have_many(:html_variant_trials).dependent(:destroy) }
it { is_expected.to have_many(:html_variant_successes).dependent(:destroy) }
it { is_expected.to validate_inclusion_of(:group).in_array(described_class::GROUP_NAMES) }
it { is_expected.to validate_presence_of(:html) }
it { is_expected.to validate_presence_of(:success_rate) }
it { is_expected.to validate_uniqueness_of(:name) }
end
end
it "calculates success rate" do
4.times { HtmlVariantTrial.create!(html_variant_id: html_variant.id) }
HtmlVariantSuccess.create!(html_variant_id: html_variant.id)
html_variant.calculate_success_rate!
expect(html_variant.success_rate).to eq(0.025)
end
it "finds for test without tag" do
html_variant.save!
expect(described_class.find_for_test.id).to eq(html_variant.id)
end
it "finds for test with tag given" do
html_variant.target_tag = "hello"
html_variant.save!
expect(described_class.find_for_test(["hello"]).id).to eq(html_variant.id)
end
it "does not find if different tag targeted" do
html_variant.target_tag = "different_tag_yolo"
html_variant.save!
expect(described_class.find_for_test(["hello"])).to be_nil
end
it "finds if no tag targeted and tag given" do
html_variant.update(target_tag: nil)
expect(described_class.find_for_test(["hello"]).id).to eq(html_variant.id)
end
it "prefixes an image with cloudinary", cloudinary: true do
html = "<div><img src='https://devimages.com/image.jpg' /></div>"
html_variant.update(approved: false, html: html)

6
spec/models/html_variant_success_spec.rb
View file

@ -1,6 +0,0 @@
require "rails_helper"
RSpec.describe HtmlVariantSuccess, type: :model do
it { is_expected.to belong_to(:html_variant) }
it { is_expected.to belong_to(:article).optional }
end

6
spec/models/html_variant_trial_spec.rb
View file

@ -1,6 +0,0 @@
require "rails_helper"
RSpec.describe HtmlVariantTrial, type: :model do
it { is_expected.to belong_to(:html_variant) }
it { is_expected.to belong_to(:article).optional }
end

19
spec/requests/html_variant_successes_spec.rb
View file

@ -1,19 +0,0 @@
require "rails_helper"
RSpec.describe "HtmlVariantSuccesses", type: :request do
let(:user) { create(:user) }
let(:article) { create(:article, user_id: user.id, approved: true) }
let(:html_variant) { create(:html_variant) }
describe "POST /html_variant_successes" do
it "rejects non-permissioned user" do
sidekiq_perform_enqueued_jobs do
post "/html_variant_successes", params: {
article_id: article.id,
html_variant_id: html_variant.id
}
end
expect(HtmlVariantSuccess.all.size).to eq(1)
end
end
end

19
spec/requests/html_variant_trials_spec.rb
View file

@ -1,19 +0,0 @@
require "rails_helper"
RSpec.describe "HtmlVariantTrials", type: :request do
let(:user) { create(:user) }
let(:article) { create(:article, user_id: user.id, approved: true) }
let(:html_variant) { create(:html_variant) }
describe "POST /html_variant_trials" do
it "rejects non-permissioned user" do
sidekiq_perform_enqueued_jobs do
post "/html_variant_trials", params: {
article_id: article.id,
html_variant_id: html_variant.id
}
end
expect(HtmlVariantTrial.all.size).to eq(1)
end
end
end

24
spec/workers/html_variants/remove_old_data_worker_spec.rb
View file

@ -1,24 +0,0 @@
require "rails_helper"
RSpec.describe HtmlVariants::RemoveOldDataWorker, type: :worker do
let(:worker) { subject }
include_examples "#enqueues_on_correct_queue", "low_priority"
describe "#perform" do
it "removes old html variants" do
Timecop.freeze do
old_trial_id = create(:html_variant_trial, created_at: 1.month.ago).id
old_success_id = create(:html_variant_success, created_at: 1.month.ago).id
new_trial = create(:html_variant_trial, created_at: Time.current)
new_trial.html_variant.update(success_rate: 0)
worker.perform
expect(HtmlVariantTrial.find_by(id: old_trial_id)).to be_nil
expect(HtmlVariantSuccess.find_by(id: old_success_id)).to be_nil
expect(HtmlVariantTrial.find_by(id: new_trial.id)).not_to be_nil
end
end
end
end