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

Adding more context to /admin/abtests (#17821)

Browse source 
* Adding more context to `/admin/abtests`

Prior to this commit, the `/admin/abtests` was fully rendered by the
field_test gem (see
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/index.html.erb
and
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/show.html.erb).

With this commit, we're pre-pending our own
partial (e.g. `app/views/field_test/experiments/_experiments.html.erb`)
in the view paths; which means instead of rendering
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/_experiments.html.erb
we render our newly created
`app/views/field_test/experiments/_experiments.html.erb`)

Why the ugly antics in the view?  As I'm not fully certain if this will
"meet" the full needs of those monitoring the experiments.  I'm also
constructing this to most closely match the spreadsheet that has been
assembled for tracking this information.

Why introduce another caching layer for the variant assembly?  The
original caching
layer (e.g. `Articles::Feeds::VariantAssembler.pre_assembled_variants`)
is in place to provide the quickest access to the assembled variants.
But due to the nature of the implemented view, we are needing a
programmatic representation of all variants.  And the added Rails cache
is there to minimize disk reads.

Closes forem/forem#17820

* Update app/models/articles/feeds/variant_assembler.rb

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>

* Update app/models/articles/feeds/variant_assembler.rb

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
This commit is contained in:
Jeremy Friesen 2022-06-03 11:27:09 -04:00 • committed by GitHub
parent 4459be2c03
commit 3fb085b3d9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 173 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
app/models/articles/feeds/variant_assembler.rb
View file

@ -11,6 +11,12 @@ module Articles
# The default extension for feed variants
EXTENSION = "json".freeze
# We have a "historical variant" that we renamed, hence we continue to maintain that name in
# our data through the following map.
VARIANT_NAME_MAP = {
"20220422-jennie-variant": :"20220422-variant"
}.freeze
# Assemble the named :variant based on the configuration of levers.
#
# @param variant [#to_sym,String,Symbol] the name of the variant we're assembling
@ -23,20 +29,42 @@ module Articles
# words, we have a mismatch in configuration.
#
# @return [Articles::Feeds::VariantQuery::Config]
def self.call(variant:, catalog: Articles::Feeds.lever_catalog, variants: variants_cache, dir: DIRECTORY)
# @see .experiment_config_hash_for
def self.call(variant:, catalog: Articles::Feeds.lever_catalog, variants: pre_assembled_variants, **kwargs)
variant = variant.to_sym
variants[variant] ||= begin
content = Rails.root.join(dir, "#{variant}.#{EXTENSION}").read
config = JSON.parse(content)
config = user_config_hash_for(variant: variant, **kwargs)
build_with(catalog: catalog, config: config, variant: variant)
end
end
# @return [Hash<Symbol, VariantQuery::Config>]
def self.variants_cache
@variants_cache ||= {}
# @param variant [#to_sym,String,Symbol] the name of the variant we're assembling
# @param dir [String] the relative directory that contains the variants.
#
# @return [Hash]
#
# @note Uses Rails.cache to minimize reads from file system. The reason for the Rails.cache
# and not leveraging the .pre_assembled_variants is that this method
# (e.g. .experiment_config_hash_for) handles all possible variant configurations (in
# contrast to the active variants).
#
# @see app/views/field_test/experiments/_experiments.html.erb
def self.user_config_hash_for(variant:, dir: DIRECTORY)
Rails.cache.fetch("feed-variant-#{variant}-#{ForemInstance.latest_commit_id}", expires_in: 24.hours) do
variant = VARIANT_NAME_MAP.fetch(variant.to_sym, variant)
content = Rails.root.join(dir, "#{variant}.#{EXTENSION}").read
JSON.parse(content)
end
end
private_class_method :variants_cache
# A memoized (e.g. cached) module instance variable that provides the quickest access for
# already assembled and active variant configurations.
#
# @return [Hash<Symbol, # VariantQuery::Config>]
def self.pre_assembled_variants
@pre_assembled_variants ||= {}
end
private_class_method :pre_assembled_variants
# @param catalog [Articles::Feeds::LeverCatalogBuilder]
# @param variant [Symbol]
@ -54,6 +82,7 @@ module Articles
VariantQuery::Config.new(
variant: variant,
levers: relevancy_levers,
description: config.fetch("description", ""),
order_by: catalog.fetch_order_by(config.fetch("order_by")),
max_days_since_published: config.fetch("max_days_since_published"),
)

1
app/services/articles/feeds/variant_query.rb
View file

@ -30,6 +30,7 @@ module Articles
Config = Struct.new(
:variant,
:description,
:levers, # Array <Articles::Feeds::RelevancyLever::Configured>
:order_by, # Articles::Feeds::OrderByLever
:max_days_since_published,

118
app/views/field_test/experiments/_experiments.html.erb Normal file
View file

@ -0,0 +1,118 @@
<style>
details details { padding: 1em 0 0 1.5em; }
</style>
<% experiments.reverse_each do |experiment| %>
<h2><%= experiment.name %><% unless experiment.active? %> <small class="closed">Completed</small><% end %></h2>
<details<%= " open" if experiment.active? %>>
<summary>Experiment Details</summary>
<% if experiment.description %>
<p class="description"><%= experiment.description %></p>
<% end %>
<details<%= " open" if experiment.active? %>>
<summary>Goals for <%= experiment.name %></summary>
<% experiment.goals.each do |goal| %>
<% results = experiment.results(goal: goal) %>
<% if experiment.multiple_goals? %>
<h3><%= goal.titleize %></h3>
<% end %>
<table>
<thead>
<tr>
<th>Variant</th>
<th style="width: 20%;">Participants</th>
<th style="width: 20%;">Conversions</th>
<th style="width: 20%;">Conversion Rate</th>
<th style="width: 20%;">Prob Winning</th>
</tr>
</thead>
<tbody>
<% results.each do |variant, result| %>
<tr>
<td>
<%= variant %>
<% if variant == experiment.winner %>
<span class="check">✓</span>
<% end %>
</td>
<td><%= result[:participated] %></td>
<td><%= result[:converted] %></td>
<td>
<% if result[:conversion_rate] %>
<%= (100.0 * result[:conversion_rate]).round(FieldTest.precision) %>%
<% else %>
-
<% end %>
</td>
<td>
<% if result[:prob_winning] %>
<% if result[:prob_winning] < 0.01 %>
&lt; 1%
<% else %>
<%= (100.0 * result[:prob_winning]).round(FieldTest.precision) %>%
<% end %>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
<% end %>
</details>
<% if experiment.id.start_with?("feed_strategy") %>
<% experiment.variants.each do |variant| %>
<details>
<summary>Config for <%= variant %> <% if variant == experiment.winner %><span class="check">✓</span><% end %></summary>
<% config = Articles::Feeds::VariantAssembler.user_config_hash_for(variant: variant) %>
<dl>
<dt>Description</dt>
<dd><%= config["description"] || "n/a" %></dd>
<dt>Weight</dt>
<dd><%= experiment.weights[experiment.variants.index(variant)] %></dd>
<dt>Order Lever</dt>
<dd><%= config["order_by"] %></dd>
<% config["levers"].each do |lever_key, lever| %>
<% next unless lever.key?("query_parameters") %>
<dt>Parameters for <strong><%= lever_key %></strong> relevancy lever</dt>
<% lever["query_parameters"].each do |key, value| %>
<dd><strong><%= key %>:</strong> <%= value %></dd>
<% end %>
<% end %>
</dl>
<table>
<% lever_range = config["levers"].map { |_, lever| lever["cases"].map(&:first) }.flatten.uniq.sort %>
<caption>Relevency Lever(s): Fallback and Range Factors</caption>
<thead>
<tr>
<th>Lever</th>
<th>Fallback</th>
<% lever_range.each do |i| %>
<th><%= i %></th>
<% end %>
</tr>
</thead>
<tbody>
<% config["levers"].each do |lever_key, lever| %>
<% cases = lever["cases"].each_with_object({}) { |(key, value), mem| mem[key] = value } %>
<tr>
<th><%= lever_key %></th>
<td><%= lever["fallback"] %></th>
<% lever_range.each do |i| %>
<td><%= cases.fetch(i, "&nbsp;".html_safe) %></td>
<% end %>
</tr>
<% end %>
</tbody>
</table>
</details>
<% end %>
<% end %>
<p><%= link_to "Data Details", experiment_path(experiment.id) %> <small>(individual user conversions)</small></p>
</details>
<% end %>

19
spec/models/articles/feeds/variant_assembler_spec.rb
View file

@ -1,10 +1,27 @@
require "rails_helper"
RSpec.describe Articles::Feeds::VariantAssembler do
describe ".user_config_hash_for" do
Rails.root.glob("#{described_class::DIRECTORY}/*.#{described_class::EXTENSION}").each do |pathname|
variant = pathname.basename(".json").to_s
context "when #{variant.inspect}" do
subject { described_class.user_config_hash_for(variant: variant) }
it { is_expected.to be_a(Hash) }
end
end
context "when \"20220422-jennie-variant\"" do
subject { described_class.user_config_hash_for(variant: "20220422-jennie-variant") }
it { is_expected.to be_a(Hash) }
end
end
describe ".call" do
Rails.root.glob("#{described_class::DIRECTORY}/*.#{described_class::EXTENSION}").each do |pathname|
variant = pathname.basename(".json").to_s.to_sym
context "for #{variant.inspect}" do
context "when #{variant.inspect}" do
# NOTE: We're providing the variants so as to not pollute the cache for other tests.
subject { described_class.call(variant: variant, variants: {}) }