Split "Experiment Details" out from feature gate info, add more experiment metadata

app/controllers/admin/decisions_controller.rb

@@ -14,6 +14,6 @@ def create
   private
 
   def decision_params
-    params.require(:decision).permit(:variant)
+    params.require(:decision).permit(:variant, :takeaways)
   end
 end

app/controllers/admin/split_details_controller.rb

@@ -18,6 +18,19 @@ def update
   private
 
   def split_detail_params
-    params.fetch(:split_detail, {}).permit(:hypothesis, :assignment_criteria, :description, :owner, :location, :platform)
+    params.fetch(:split_detail, {}).permit(
+      :owner,
+      :platform,
+      :location,
+      :control_variant,
+      :start_date,
+      :end_date,
+      :description,
+      :hypothesis,
+      :assignment_criteria,
+      :takeaways,
+      :tests,
+      :segments,
+    )
   end
 end

app/models/decision.rb

@@ -1,16 +1,27 @@
 class Decision
   include ActiveModel::Model
 
-  attr_accessor :split, :variant
+  attr_accessor :split, :variant, :takeaways
 
   validates :split, :variant, presence: true
 
   def save!
     raise "Variant must be present in the split" unless split.has_variant?(variant)
 
-    split.reconfigure!(
-      weighting_registry: { variant => 100 },
-      decided_at: Time.zone.now
-    )
+    split.with_lock do
+      save_takeaways! if instance_variable_defined?(:@takeaways)
+      split.reconfigure!(
+        weighting_registry: { variant => 100 },
+        decided_at: Time.zone.now,
+      )
+    end
+  end
+
+  private
+
+  def save_takeaways!
+    split.build_experiment_detail unless split.experiment_detail
+    split.experiment_detail.takeaways = @takeaways
+    split.experiment_detail.save!
   end
 end

app/models/experiment_detail.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ExperimentDetail < ActiveRecord::Base
+  belongs_to :split
+
+  def start_date
+    super || split.created_at.to_date
+  end
+
+  def end_date
+    super || split.decided_at&.to_date
+  end
+
+  def control_variant
+    super || split.registry.keys.find { |v| v == 'control' }
+  end
+
+  def self.available_tests
+    @available_tests ||= {}
+  end
+
+  def self.available_segments
+    @available_segments ||= {}
+  end
+end

app/models/split.rb

@@ -1,6 +1,7 @@
 class Split < ActiveRecord::Base
   belongs_to :owner_app, required: true, class_name: "App", inverse_of: :splits
 
+  has_one :experiment_detail, dependent: :nullify
   has_many :previous_split_registries, dependent: :nullify
   has_many :assignments, -> { for_presentation }, dependent: :nullify, inverse_of: :split
   has_many :bulk_assignments, dependent: :nullify
@@ -14,6 +15,7 @@ class Split < ActiveRecord::Base
 
   validates :name, presence: true, uniqueness: true
   validates :registry, presence: true
+  validates :experiment_detail, absence: true, if: :feature_gate?
 
   validate :name_must_be_snake_case
   validate :name_must_only_be_prefixed_with_app_name
@@ -24,6 +26,8 @@ class Split < ActiveRecord::Base
 
   enum platform: %i(mobile desktop)
 
+  delegate :description, :hypothesis, :assignment_criteria, :takeaways, to: :experiment_detail, allow_nil: true
+
   before_validation :cast_registry
 
   scope :for_presentation, ->(app_build: nil) do
@@ -87,10 +91,6 @@ def detail
     @detail ||= SplitDetail.new(split: self)
   end
 
-  def has_details?
-    %w(hypothesis assignment_criteria description owner location platform).any? { |attr| public_send(attr).present? }
-  end
-
   def has_variant?(variant)
     registry.key?(variant.to_s)
   end

app/models/split_detail.rb

@@ -5,11 +5,11 @@ class SplitDetail
 
   attr_accessor :split
   delegate :name, :variants, to: :split
-  delegate_attribute :hypothesis, :assignment_criteria, :description, :owner, :location, :platform, to: :split
+  delegate_attribute :owner, :platform, :location, to: :split
+  delegate_attribute :control_variant, :start_date, :end_date, :description,
+                     :hypothesis, :assignment_criteria, :takeaways, :tests, :segments,
+                     to: :experiment_detail
 
-  validates :hypothesis, presence: true, if: -> { split.hypothesis_was.present? }
-  validates :assignment_criteria, presence: true, if: -> { split.assignment_criteria_was.present? }
-  validates :description, presence: true, if: -> { split.description_was.present? }
   validates :owner, presence: true, if: -> { split.owner_was.present? }
   validates :location, presence: true, if: -> { split.location_was.present? }
   validates :platform, presence: true, if: -> { split.platform_was.present? }
@@ -29,9 +29,14 @@ def variant_details
     end
   end
 
+  def experiment_detail
+    split.experiment_detail || split.build_experiment_detail
+  end
+
   def save
     if valid?
       split.save!
+      split.experiment_detail.save!
       true
     else
       false

app/views/admin/decisions/new.html.erb

@@ -18,5 +18,9 @@
     <%= f.input :variant, label: 'Target Variant', collection: @split.variants, as: :radio_buttons, include_blank: false %>
   </div>
 
+  <% unless @split.feature_gate? %>
+    <%= f.input :takeaways, as: :text, hint: "Example: We decided to keep the changes, as it increased conversion rates by 3% (plus or minus 2%)" %>
+  <% end %>
+
   <%= render "shared/form_footer", f: f, submit_text: "Decide Split", submit_disable_with_text: "Saving your Split Decision...", confirm_text: "\nYou are deciding '#{@split.name}'.\nDo you wish to proceed?" %>
 <% end %>

app/views/admin/split_details/edit.html.erb

@@ -9,11 +9,17 @@
   <%= f.input :platform, collection: Split.platforms.keys %>
   <%= f.input :location, label: "Location", hint: "Example: Sign Up" %>
 
-  <%= f.input :description, as: :text, hint: "Example: Users often get confused during the account creation process when reaching step 2, 'Personal Details'. We are testing two different versions of this page that aim to clear up that confusion." %>
-
-  <%= f.input :hypothesis, as: :text, hint: "Example: By rearranging some of the fields and adding helper text we can improve sign up completion rates and decrease the amount of support tickets we receive." %>
-
-  <%= f.input :assignment_criteria, as: :text, hint: "Example: Assigned when any user enters the first step of the signup flow. Mobile users are not affected." %>
+  <% unless @split_detail.split.feature_gate? %>
+    <%= f.input :control_variant, as: :select, collection: @split_detail.split.registry.keys, include_blank: true %>
+    <%= f.input :start_date, as: :date, hint: "" %>
+    <%= f.input :end_date, as: :date, include_blank: true, hint: "" %>
+    <%= f.input :description, as: :text, hint: "Example: Users often get confused during the account creation process when reaching step 2, 'Personal Details'. We are testing two different versions of this page that aim to clear up that confusion." %>
+    <%= f.input :hypothesis, as: :text, hint: "Example: By rearranging some of the fields and adding helper text we can improve sign up completion rates and decrease the amount of support tickets we receive." %>
+    <%= f.input :assignment_criteria, as: :text, hint: "Example: Assigned when any user enters the first step of the signup flow. Mobile users are not affected." %>
+    <%= f.input :takeaways, as: :text, disabled: !@split_detail.split.decided?, hint: "Example: We decided to keep the changes, as it increased conversion rates by 3% (plus or minus 2%)" %>
+    <%= f.input :tests, as: :check_boxes, collection: ExperimentDetail.available_tests.keys | @split_detail.tests, hint: "" %>
+    <%= f.input :segments, as: :check_boxes, collection: ExperimentDetail.available_segments.keys | @split_detail.segments, hint: "" %>
+  <% end %>
 
   <%= render "shared/form_footer", f: f, back_path: admin_split_path(@split_detail.split.id), submit_text: "Update Details", submit_disable_with_text: "Updating Split..." %>
 <% end %>

app/views/admin/splits/_details.html.erb

@@ -1,55 +1,23 @@
-<article class="InfoCard InfoCard--constrained my-5">
+<article class="InfoCard fs-SplitOverview my-5">
   <div class="InfoCard-header">
-    <h4>Split Details</h4>
-    <%= link_to "Decide Split", new_admin_split_decision_path(@split), class: 'decide-split-link btn btn-primary btn-large' %>
+    <h4>Split Overview</h4>
+
+    <%= link_to "Edit", edit_admin_split_details_path(split), class: 'edit-details-link' %>
   </div>
+
   <hr class="InfoCard-divider">
-  <div class="InfoCard-description">
-    <table class="BasicTable">
-      <tr class="population-row">
-        <td>Population Size</td>
-        <td>
-          <span class='population'>
-            <%= @split.assignments.count %><% if @split.feature_gate? %>* [<a href="#gate_population">feature gate</a>]<% end %>
-          </span>
-        </td>
-        <td>
-          <span><%= link_to "Edit", new_admin_split_bulk_assignment_path(@split), class: 'upload-new-assignments-link' %></span>
-        </td>
-      </tr>
-      <tr>
-        <td>Status</td>
-        <% if @split.finished? %>
-          <td class="u-status--finished">Finished (Retired)
-            <% if @split.decided? %>
-              (decision: <%= @split.decided_variant %>)
-           <% end %>
-           <span class="p-3">🏁</span></td>
-        <% elsif @split.decided? %>
-          <td class="u-status--decided">Decided (decision: <%= @split.decided_variant %>)</td>
-        <% else %>
-          <td class="u-status--active">Active</td>
-        <% end %>
-        <td>&nbsp;</td>
-      </tr>
-      <tr>
-        <td>App</td>
-        <td><%= @split.owner_app.name.capitalize %></td>
-        <td>&nbsp;</td>
-      </tr>
-      <tr>
-        <td>Creation</td>
-        <td>
-          <span><%= @split.created_at %></span>
-        </td>
-        <td>&nbsp;</td>
-      </tr>
-    </table>
-    <% if @split.feature_gate? %>
-      <p>
-      <a id="gate_population"></a>
-      * Feature gates no longer track assignment events and population reflects only visitors assigned to specific variants via the chrome extension or admin tool.
-      </p>
-    <% end %>
-  </div>
+  <table class="DescriptionTable">
+    <tr>
+      <td>Owner</td>
+      <td><%= split.owner %></td>
+    </tr>
+    <tr>
+      <td>Platform</td>
+      <td><%= split.platform %></td>
+    </tr>
+    <tr>
+      <td>Location</td>
+      <td><%= split.location %></td>
+    </tr>
+  </table>
 </article>

app/views/admin/splits/_experiment.html.erb

@@ -0,0 +1,63 @@
+<article class="InfoCard fs-SplitExperiment my-5">
+  <div class="InfoCard-header">
+    <h4>Experiment<%= " (decision: #{split.decided_variant})" if split.decided?%></h4>
+
+    <% if split.experiment_detail %>
+      <%= link_to "Edit", edit_admin_split_details_path(split), class: 'edit-experiment-details-link' %>
+    <% else %>
+      <%= link_to "Add Details", edit_admin_split_details_path(split), class: 'add-experiment-details-link' %>
+    <% end %>
+  </div>
+
+  <hr class="InfoCard-divider">
+  <% if split.experiment_detail %>
+    <table class="DescriptionTable">
+      <tr>
+        <td>Review Period</td>
+        <td><%= split.experiment_detail.start_date %> — <%= split.experiment_detail.end_date %></td>
+      </tr>
+      <tr>
+        <td>Control Variant</td>
+        <td><%= split.experiment_detail.control_variant %></td>
+      </tr>
+      <tr>
+        <td>Description</td>
+        <td><%= split.experiment_detail.description %></td>
+      </tr>
+      <tr>
+        <td>Hypothesis</td>
+        <td><%= split.experiment_detail.hypothesis %></td>
+      </tr>
+      <tr>
+        <td>Assignment Criteria</td>
+        <td><%= split.experiment_detail.assignment_criteria %></td>
+      </tr>
+      <tr>
+        <td>Takeaways</td>
+        <td>
+          <% if split.decided? %>
+            <%= split.experiment_detail.takeaways %>
+          <% else %>
+            (split not decided)
+          <% end %>
+        </td>
+      </tr>
+      <tr>
+        <td>Tests</td>
+        <td><%= split.experiment_detail.tests.join(', ') %></td>
+      </tr>
+      <tr>
+        <td>Segments</td>
+        <td><%= split.experiment_detail.segments.join(', ') %></td>
+      </tr>
+    </table>
+  <% else %>
+    <div class="EmptyTable EmptyTable--centered p-5">
+      <p>
+        <strong>Is this split a test? Add metadata about it.</strong>
+        <br>
+        This information is used to explain the context of what a user is experiencing.
+      </p>
+    </div>
+  <% end %>
+</article>

app/views/admin/splits/_info.html.erb

@@ -0,0 +1,55 @@
+<article class="InfoCard InfoCard--constrained my-5">
+  <div class="InfoCard-header">
+    <h4>Split Details</h4>
+    <%= link_to "Decide Split", new_admin_split_decision_path(@split), class: 'decide-split-link btn btn-primary btn-large' %>
+  </div>
+  <hr class="InfoCard-divider">
+  <div class="InfoCard-description">
+    <table class="BasicTable">
+      <tr class="population-row">
+        <td>Population Size</td>
+        <td>
+          <span class='population'>
+            <%= @split.assignments.count %><% if @split.feature_gate? %>* [<a href="#gate_population">feature gate</a>]<% end %>
+          </span>
+        </td>
+        <td>
+          <span><%= link_to "Edit", new_admin_split_bulk_assignment_path(@split), class: 'upload-new-assignments-link' %></span>
+        </td>
+      </tr>
+      <tr>
+        <td>Status</td>
+        <% if @split.finished? %>
+          <td class="u-status--finished">Finished (Retired)
+            <% if @split.decided? %>
+              (decision: <%= @split.decided_variant %>)
+           <% end %>
+           <span class="p-3">🏁</span></td>
+        <% elsif @split.decided? %>
+          <td class="u-status--decided">Decided (decision: <%= @split.decided_variant %>)</td>
+        <% else %>
+          <td class="u-status--active">Active</td>
+        <% end %>
+        <td>&nbsp;</td>
+      </tr>
+      <tr>
+        <td>App</td>
+        <td><%= @split.owner_app.name.capitalize %></td>
+        <td>&nbsp;</td>
+      </tr>
+      <tr>
+        <td>Creation</td>
+        <td>
+          <span><%= @split.created_at %></span>
+        </td>
+        <td>&nbsp;</td>
+      </tr>
+    </table>
+    <% if @split.feature_gate? %>
+      <p>
+      <a id="gate_population"></a>
+      * Feature gates no longer track assignment events and population reflects only visitors assigned to specific variants via the chrome extension or admin tool.
+      </p>
+    <% end %>
+  </div>
+</article>