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

[deploy] Remove sidebar variant from CTA (#8040)

Browse source 
* Remove sidebar variant from CTA

* Remove unneeded tests

* Modify algo *just a bit*
This commit is contained in:
Ben Halpern 2020-05-23 13:35:43 -04:00 • committed by GitHub
parent c23c4fbb2d
commit a53de929fe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 52 additions and 113 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/controllers/stories_controller.rb
View file

@ -260,7 +260,6 @@ class StoriesController < ApplicationController
not_found unless @article.user
@article_show = true
@variant_number = params[:variant_version] || (user_signed_in? ? 0 : rand(2))
@user = @article.user
@organization = @article.organization

8
app/models/html_variant.rb
View file

@ -1,7 +1,7 @@
class HtmlVariant < ApplicationRecord
include CloudinaryHelper
GROUP_NAMES = %w[article_show_sidebar_cta article_show_below_article_cta badge_landing_page campaign].freeze
GROUP_NAMES = %w[article_show_below_article_cta badge_landing_page campaign].freeze
validates :html, presence: true
validates :name, uniqueness: true
@ -23,11 +23,11 @@ class HtmlVariant < ApplicationRecord
end
class << self
def find_for_test(tags = [], group = "article_show_sidebar_cta")
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 in top 10
else # 90% chance return one of the top posts
find_top_for_test(tags_array, group)
end
end
@ -36,7 +36,7 @@ class HtmlVariant < ApplicationRecord
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..15)).sample
order("success_rate DESC").limit(rand(1..20)).sample
end
def find_random_for_test(tags_array, group)

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

@ -98,7 +98,7 @@
</div>
</div>
</div>
<% if @variant_number.to_i == 0 && !user_signed_in? && @article.body_markdown.size > 900 %>
<% 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 %>

97
app/views/articles/_sticky_nav.html.erb
View file

@ -72,63 +72,52 @@
</div>
<% end %>
</div>
<% if @variant_number.to_i == 1 %>
<% cache("html-variant-sidebar#{rand(300)}", expires_in: 2.hours) do %>
<% @html_variant = HtmlVariant.find_for_test(@article.cached_tag_list_array) %>
<% if @html_variant %>
<div class="html-variant-wrapper" id="html-variant-article-show-sidebar" data-variant-id="<%= @html_variant.id %>">
<%= @html_variant.html.html_safe %>
<% cache("article-sticky-nav-articles-#{@article.id}-#{@actor.last_article_at}", expires_in: 48.hours) do %>
<% @sticky_collection = StickyArticleCollection.new(@article, @organization || @user) %>
<% @sticky_articles = @sticky_collection.suggested_stickies %>
<% @user_stickies = @sticky_collection.user_stickies %>
<% if @user_stickies.any? %>
<% @user_stickies.each_with_index do |article, index| %>
<div class="primary-sticky-nav-element-wrapper">
<% if index == 0 %>
<div class="primary-sticky-nav-title">
<% if @actor.username.size < 18 %>
More from <a href="<%= @actor.path %>">@<%= @actor.username %></a>
<% else %>
More from<br /><a href="<%= @actor.path %>">@<%= @actor.username %></a>
<% end %>
</div>
<% end %>
<a class="primary-sticky-nav-element primary-sticky-nav-author-element" href="<%= article.path %>">
<img src="<%= ProfileImage.new(article.user).get(width: 90) %>" class="primary-sticky-nav-profile-image" loading="lazy" alt="<%= article.user.name %> profile image">
<%= article.title %>
<div class="primary-sticky-nav-element-details">
<% article.decorate.cached_tag_list_array.each do |tag| %>
<span>#<%= tag %></span>
<% end %>
</div>
</a>
</div>
<% end %>
<% end %>
<% else %>
<% cache("article-sticky-nav-articles-#{@article.id}-#{@actor.last_article_at}", expires_in: 48.hours) do %>
<% @sticky_collection = StickyArticleCollection.new(@article, @organization || @user) %>
<% @sticky_articles = @sticky_collection.suggested_stickies %>
<% @user_stickies = @sticky_collection.user_stickies %>
<% if @user_stickies.any? %>
<% @user_stickies.each_with_index do |article, index| %>
<div class="primary-sticky-nav-element-wrapper">
<% if index == 0 %>
<div class="primary-sticky-nav-title">
<% if @actor.username.size < 18 %>
More from <a href="<%= @actor.path %>">@<%= @actor.username %></a>
<% else %>
More from<br /><a href="<%= @actor.path %>">@<%= @actor.username %></a>
<% end %>
</div>
<% end %>
<a class="primary-sticky-nav-element primary-sticky-nav-author-element" href="<%= article.path %>">
<img src="<%= ProfileImage.new(article.user).get(width: 90) %>" class="primary-sticky-nav-profile-image" loading="lazy" alt="<%= article.user.name %> profile image">
<%= article.title %>
<div class="primary-sticky-nav-element-details">
<% article.decorate.cached_tag_list_array.each do |tag| %>
<span>#<%= tag %></span>
<% end %>
</div>
</a>
</div>
<% end %>
<% end %>
<% if @sticky_articles %>
<% @sticky_articles.each_with_index do |article, index| %>
<div class="primary-sticky-nav-element-wrapper">
<% if index == 0 %>
<div class="primary-sticky-nav-title">
<img src="<%= asset_path "emoji/apple-fire.png" %>" alt="Fire emoji" loading="lazy"> Trending on <a href="<%= app_url %>"><%= community_name %></a>
</div>
<% end %>
<a class="primary-sticky-nav-element" href="<%= article.path %>">
<img src="<%= ProfileImage.new(article.user).get(width: 90) %>" class="primary-sticky-nav-profile-image" loading="lazy" alt="<%= article.user.name %> profile image">
<%= article.title %>
<div class="primary-sticky-nav-element-details">
<% article.decorate.cached_tag_list_array.each do |tag| %>
<span>#<%= tag %></span>
<% end %>
</div>
</a>
</div>
<% end %>
<% if @sticky_articles %>
<% @sticky_articles.each_with_index do |article, index| %>
<div class="primary-sticky-nav-element-wrapper">
<% if index == 0 %>
<div class="primary-sticky-nav-title">
<img src="<%= asset_path "emoji/apple-fire.png" %>" alt="Fire emoji" loading="lazy"> Trending on <a href="<%= app_url %>"><%= community_name %></a>
</div>
<% end %>
<a class="primary-sticky-nav-element" href="<%= article.path %>">
<img src="<%= ProfileImage.new(article.user).get(width: 90) %>" class="primary-sticky-nav-profile-image" loading="lazy" alt="<%= article.user.name %> profile image">
<%= article.title %>
<div class="primary-sticky-nav-element-details">
<% article.decorate.cached_tag_list_array.each do |tag| %>
<span>#<%= tag %></span>
<% end %>
</div>
</a>
</div>
<% end %>
<% end %>
<% end %>

2
app/views/articles/show.html.erb
View file

@ -205,7 +205,7 @@
<% end %>
</div>
<% stick_nav_cache_key = "sticky-sidebar-#{@article.id}-#{(@organization || @user).profile_updated_at}-#{(@organization || @user).last_article_at}-#{@variant_number}-#{@user.pro?}" %>
<% stick_nav_cache_key = "sticky-sidebar-#{@article.id}-#{(@organization || @user).profile_updated_at}-#{(@organization || @user).last_article_at}-0-#{@user.pro?}" %>
<% cache(stick_nav_cache_key, expires_in: 50.hours) do %>
<%= render "articles/sticky_nav" %>
<% end %>

2
app/views/html_variants/_form.html.erb
View file

@ -25,7 +25,7 @@
<%= f.label :published %>
<%= f.check_box :published %>
<%= f.label :group %>
<%= f.select(:group, options_for_select(HtmlVariant::GROUP_NAMES, @html_variant.group || "article_show_sidebar_cta")) %>
<%= f.select(:group, options_for_select(HtmlVariant::GROUP_NAMES, @html_variant.group || "article_show_below_article_cta")) %>
</div>
<%= f.submit %>
<% end %>

1
app/views/html_variants/index.html.erb
View file

@ -7,7 +7,6 @@
<nav>
<a href="/html_variants" class="<%= "selected" if params[:state].blank? %>">Leaderboard</a>
<a href="/html_variants?state=mine" class="<%= "selected" if params[:state] == "mine" %>">Mine</a>
<a href="/html_variants?state=article_show_sidebar_cta" class="<%= "selected" if params[:state] == "article_show_sidebar_cta" %>">Show Page Sidebar</a>
<a href="/html_variants?state=article_show_below_article_cta" class="<%= "selected" if params[:state] == "article_show_below_article_cta" %>">Show Page Below Article</a>
<a href="/html_variants?state=badge_landing_page" class="<%= "selected" if params[:state] == "badge_landing_page" %>">Badge Landing Page</a>
<a href="/html_variants?state=campaign" class="<%= "selected" if params[:state] == "campaign" %>">Campaign</a>

2
spec/factories/html_variants.rb
View file

@ -4,6 +4,6 @@ FactoryBot.define do
name { Faker::Hipster.paragraph(sentence_count: 1) }
html { "<div>#{rand(10_000_000_000)}</div><h1>HEllo</h1>" }
success_rate { 0.3 }
group { "article_show_sidebar_cta" }
group { "article_show_below_article_cta" }
end
end

2
spec/requests/html_variants_spec.rb
View file

@ -58,7 +58,7 @@ RSpec.describe "HtmlVariants", type: :request do
name: "New post",
html: "Yo ho ho", tag_list: "yoyo",
published: true,
group: "article_show_sidebar_cta"
group: "article_show_below_article_cta"
}
}
expect(HtmlVariant.all.size).to eq(1)

48
spec/requests/stories_show_spec.rb
View file

@ -77,54 +77,6 @@ RSpec.describe "StoriesShow", type: :request do
expect(response.body).to include "<em>with <b><a href=\"#{user2.path}\">"
end
# sidebar HTML variant
it "renders html variant" do
html_variant = create(:html_variant, published: true, approved: true)
get article.path + "?variant_version=1"
expect(response.body).to include html_variant.html
end
it "does not render variant when no variants published" do
html_variant = create(:html_variant, published: false, approved: true)
get article.path + "?variant_version=1"
expect(response.body).not_to include html_variant.html
end
it "does not render html variant when user logged in" do
html_variant = create(:html_variant, published: true, approved: true)
sign_in user
get article.path
expect(response.body).not_to include html_variant.html
end
# Below article HTML variant
it "renders below article html variant" do
html_variant = create(:html_variant, published: true, approved: true, group: "article_show_below_article_cta")
article.update_column(:body_markdown, rand(36**1000).to_s(36).to_s) # min length for article
get article.path + "?variant_version=0"
expect(response.body).to include html_variant.html
end
it "does not render below article html variant for short article" do
html_variant = create(:html_variant, published: true, approved: true, group: "article_show_below_article_cta")
article.update_column(:body_markdown, rand(36**100).to_s(36).to_s) # ensure too short
get article.path + "?variant_version=0"
expect(response.body).not_to include html_variant.html
end
it "does not render below article variant when no variants published" do
html_variant = create(:html_variant, published: false, approved: true, group: "article_show_below_article_cta")
get article.path + "?variant_version=0"
expect(response.body).not_to include html_variant.html
end
it "does not render below article html variant when user logged in" do
html_variant = create(:html_variant, published: true, approved: true, group: "article_show_below_article_cta")
sign_in user
get article.path + "?variant_version=0"
expect(response.body).not_to include html_variant.html
end
it "renders articles of long length without breaking" do
# This is a pretty weak test, just to exercise different lengths with no breakage
article.update(title: (0...75).map { rand(65..90).chr }.join)