From b8533172e3fd6fefb1e331cb133095682db3bb82 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 24 Jun 2022 09:26:46 -0400 Subject: [PATCH] Introducing a quick and dirty fix for abtests/admin (#17982) This is a "quick and dirty" hack. Do I like it? No. Do I want to delve into further debugging/refactoring of [a 72 line method][1]? Not at the moment. There are a few pathways forward, but for now, this is the pathway to hopefully give us insight into the **Why will this likely address the issue?** Prior to this commit, for an experiment we would render each of the goals (anywhere from 8 to 11); each of which would require 2 expensive queries; which means about 16 expensive queries. After this commit, the experiment page has none of those expensive queries; instead each goal now runs the 2 expensive queries. The hope is that this will help us show the results (albeit on multiple pages) while coming in under the response time out handlers we have in place. **Why no tests?** Ugh; I know right?!? I'm beginning to ask that myself. But for now, because this is only visible to tech_admins, the consequences of breaking are limited. In otherwords, this is not a customer facing feature, so it can be a bit less robust in it's testing. At least that's the rationalization I'm establishing. Further, local tests would not reveal the production environment complications of large data sets. The aforementioned expensive queries are blisteringly fast on my local machine...in part because I don't much field_test experiment data. Closes forem/forem#17981 Related to: - forem/forem#17895 - forem/forem#17869 [1]:https://github.com/ankane/field_test/blob/ab2d7d29d0b9f789831569f6d017ef3ab59d0517/lib/field_test/experiment.rb#L98-L160 --- .../field_test/experiments_controller.rb | 10 ++++ .../field_test/experiments/goal.html.erb | 52 ++++++++++++++++++ .../field_test/experiments/show.html.erb | 55 +++---------------- config/routes/admin.rb | 1 + 4 files changed, 70 insertions(+), 48 deletions(-) create mode 100644 app/views/field_test/experiments/goal.html.erb diff --git a/app/controllers/field_test/experiments_controller.rb b/app/controllers/field_test/experiments_controller.rb index 861f4d7e4..46e256f59 100644 --- a/app/controllers/field_test/experiments_controller.rb +++ b/app/controllers/field_test/experiments_controller.rb @@ -12,5 +12,15 @@ module FieldTest rescue FieldTest::ExperimentNotFound raise ActionController::RoutingError, "Experiment not found" end + + def goal + @experiment = FieldTest::Experiment.find(params[:experiment_id]) + @goal = params.fetch(:goal) + unless @experiment.goals.include?(@goal) + raise ActionController::RoutingError, "Experiment's goal not found" + end + rescue FieldTest::ExperimentNotFound + raise ActionController::RoutingError, "Experiment not found" + end end end diff --git a/app/views/field_test/experiments/goal.html.erb b/app/views/field_test/experiments/goal.html.erb new file mode 100644 index 000000000..f7d4f7da7 --- /dev/null +++ b/app/views/field_test/experiments/goal.html.erb @@ -0,0 +1,52 @@ + +<% experiment = @experiment %> +

<%= experiment.name %><% unless experiment.active? %> Completed<% end %>

+<%# NOTE: The root_path is the Rails's engine root path, not the application's root path (e.g. "/") %> +

<%= link_to "Back to Experiments", FieldTest.railtie_routes_url_helpers.experiment_path(experiment.id) %>

+ +

<%= @goal.titleize %>

+<% results = experiment.results(goal: @goal) %> + + + + + + + + + + + + + <% results.each do |variant, result| %> + + + + + + + + <% end %> + +
VariantParticipantsConversionsConversion RateProb Winning
+ <%= variant %> + <% if variant == experiment.winner %> + ✓ + <% end %> + <%= result[:participated] %><%= result[:converted] %> + <% if result[:conversion_rate] %> + <%= (100.0 * result[:conversion_rate]).round(FieldTest.precision) %>% + <% else %> + - + <% end %> + + <% if result[:prob_winning] %> + <% if result[:prob_winning] < 0.01 %> + < 1% + <% else %> + <%= (100.0 * result[:prob_winning]).round(FieldTest.precision) %>% + <% end %> + <% end %> +
diff --git a/app/views/field_test/experiments/show.html.erb b/app/views/field_test/experiments/show.html.erb index ba60a5c18..efb1bd317 100644 --- a/app/views/field_test/experiments/show.html.erb +++ b/app/views/field_test/experiments/show.html.erb @@ -14,55 +14,14 @@ > Goals for <%= experiment.name %> - <% experiment.goals.each do |goal| %> - <% results = experiment.results(goal: goal) %> - - <% if experiment.multiple_goals? %> -

<%= goal.titleize %>

+ <% if experiment.id.start_with?("feed_strategy") %> diff --git a/config/routes/admin.rb b/config/routes/admin.rb index aa9de7c1b..37f9eea32 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -14,6 +14,7 @@ namespace :admin do mount Sidekiq::Web => "sidekiq" mount FieldTest::Engine, at: "abtests" + get "abtests/experiments/:experiment_id/:goal", to: "/field_test/experiments#goal" flipper_ui = Flipper::UI.app(Flipper, { rack_protection: { except: %i[authenticity_token form_token json_csrf