* Moving Experiments into AbExperiment namespace This commit entails two major concepts: 1. Extracting logic out of a worker. 2. Cleaning the conditions in which we convert experiments. In addition, I revisited some spec names to tidy them up for clarification and structure. **Extracting Logic...** For workers, my preference is that they be a simple router to another object. A non-worker object, with it's initialize method, can both better manage instance variables. Further, workers are somewhat "flat" in hierarchy, In this case, having the AbExperiment be knowledgeable both in setting the experiment and handling conversions makes (to me) organizational sense. For example, the AbExperiment is constructed as a wrapper to the FieldTest gem. But the RecordFieldTestEventWorker had knowledge of FieldTest. With this refactor, it does not have that knowledge. This also provided an opportunity to replace magic strings with constants. **Cleaning the conditions...** This relates directly to and closes forem/forem#17530. I was looking at the experiments after 2 days, and realized that there shouldn't be results for `user_views_pages_on_at_least_nine_different_days_within_two_weeks`; after all the experiment started 2 days ago, how can we have results that look at 2 weeks. By adding the "max" function calls, we ensure a clear boundary of "before the experiment began" versus "while the experiment is running". Closes forem/forem#17530 * Update spec/models/ab_experiment_spec.rb Co-authored-by: Mac Siri <krairit.siri@gmail.com> * Apply suggestions from code review Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Mac Siri <krairit.siri@gmail.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
139 lines
5.5 KiB
Ruby
139 lines
5.5 KiB
Ruby
# This object wraps the FieldTest logic to provide a bit of insulation
|
|
# between the FieldTest implementation and the application logic.
|
|
#
|
|
# This is a SimpleDelegator because the field_test method requires
|
|
# controller context when the participating user is nil.
|
|
#
|
|
# Use the AbExperiment.get method to interact with this class.
|
|
class AbExperiment < SimpleDelegator
|
|
# Don't allow someone to circumvent the .get class method. I want
|
|
# to follow the Law of Demeter and require that we us the @api
|
|
# public
|
|
private_class_method :new
|
|
|
|
# @see ./config/feed-variants/README.md
|
|
ORIGINAL_VARIANT = "original".freeze
|
|
|
|
CURRENT_FEED_STRATEGY_EXPERIMENT = FieldTest.config["experiments"]&.keys
|
|
&.detect { |e| e.start_with? "feed_strategy" }.freeze
|
|
|
|
# Sometimes we might want to repurpose the same AbExperiment logic
|
|
# for different experiments. This provides the tooling for that
|
|
# exact thing.
|
|
EXPERIMENT_TO_METHOD_NAME_MAP = {
|
|
CURRENT_FEED_STRATEGY_EXPERIMENT => :feed_strategy
|
|
}.freeze
|
|
|
|
# This method helps us leverage existing methods for different
|
|
# experiments.
|
|
#
|
|
# @param experiment [Symbol] the name of the experiment
|
|
# @return [Symbol] the method name associated with this experiment.
|
|
#
|
|
# @see EXPERIMENT_TO_METHOD_NAME_MAP
|
|
def self.method_name_for(experiment)
|
|
EXPERIMENT_TO_METHOD_NAME_MAP.fetch(experiment, experiment)
|
|
end
|
|
|
|
# A convenience method for retrieving the feed variant
|
|
# @param controller [ApplicationController] the request context of
|
|
# the experiment.
|
|
# @param user [User] who are we running the experiment with
|
|
#
|
|
# @return [Object] a variant that the experimenter should know what it wants
|
|
#
|
|
# @see .get
|
|
def self.get_feed_variant_for(controller:, user:)
|
|
get(
|
|
controller: controller,
|
|
user: user,
|
|
default_value: ORIGINAL_VARIANT,
|
|
experiment: CURRENT_FEED_STRATEGY_EXPERIMENT,
|
|
)
|
|
end
|
|
|
|
def self.variants_for_experiment(experiment)
|
|
configured_experiment = FieldTest.config["experiments"][experiment]
|
|
configured_experiment["variants"] if configured_experiment
|
|
end
|
|
|
|
# @api public
|
|
#
|
|
# A convenience method to insulate against the implementation
|
|
# details of the field_test gem.
|
|
#
|
|
# @param experiment [Symbol] the named method we'll call for an
|
|
# experiment. It should be a method name defined on this
|
|
# object.
|
|
# @param controller [ApplicationController] the request context of
|
|
# the experiment.
|
|
# @note We need the controller object due to the implementation of
|
|
# the `field_test` method. The `field_test` method is defined
|
|
# in the FieldTest::Helpers module. If we have a user, the
|
|
# field_test method works great with just the
|
|
# FieldTest::Helpers methods. However, if we don't have a
|
|
# user, then the field_test method calls
|
|
# `field_test_participant` which is defined in the
|
|
# `FieldTest::Controller` (which calls the `request` and
|
|
# `cookies`).
|
|
# @param user [User] who are we running the experiment with
|
|
# @param config [Hash] container for possible ENV override of strategy.
|
|
# @param default_value [Object] the caller is making decisions based
|
|
# on a configured set of values. They know which one they
|
|
# likely want. Let them give us a hint.
|
|
#
|
|
# @return [Object] a variant that the experimenter should know what it wants
|
|
#
|
|
# @see config/field_test.yml file for configured experiments.
|
|
#
|
|
# @note You can force a named strategy by setting an ENV variable.
|
|
# This forced strategy might be super useful for anyone performing
|
|
# QA testing on AB Testing scenarios.
|
|
#
|
|
# @todo If we make heavy use of this class, consider guarding for
|
|
# valid experiment methods.
|
|
def self.get(experiment:, controller:, user:, default_value:, config: ApplicationConfig)
|
|
method_name = method_name_for(experiment)
|
|
new(controller: controller)
|
|
.public_send(method_name, user: user, default_value: default_value, experiment: experiment, config: config)
|
|
end
|
|
|
|
# Responsible for checking if a given :user has "accomplished" the state :goal for any of the
|
|
# active :experiments. We only consider events that occur on or after each experiment's given
|
|
# start_date.
|
|
#
|
|
# @param user [User]
|
|
# @param goal [String]
|
|
# @param experiments [Hash<String, Object>]
|
|
#
|
|
# @see AbExperiment::GoalConversionChecker
|
|
# @see FieldTest.config
|
|
def self.register_conversions_for(user:, goal:, experiments: FieldTest.config["experiments"])
|
|
GoalConversionHandler.call(user: user, goal: goal, experiments: experiments)
|
|
end
|
|
|
|
# @api private
|
|
# @param controller [ApplicationController] the current controller
|
|
# that's handling the current request.
|
|
def initialize(controller:)
|
|
super(controller)
|
|
end
|
|
|
|
# @api private
|
|
# @note Called via AbExperiment.get
|
|
def feed_strategy(user:, config:, default_value:, experiment: :feed_strategy)
|
|
return default_value.inquiry unless FeatureFlag.accessible?(:ab_experiment_feed_strategy)
|
|
|
|
(config["AB_EXPERIMENT_FEED_STRATEGY"] || field_test(experiment, participant: user)).inquiry
|
|
rescue FieldTest::ExperimentNotFound
|
|
# rubocop:disable Layout/LineLength
|
|
Rails.logger.warn do
|
|
"Upstream request #{experiment.inspect} experiment. There are no registered #{experiment.inspect} experiments. Using the default value of #{default_value.inspect} for #{experiment.inspect} experiment."
|
|
end
|
|
# rubocop:enable Layout/LineLength
|
|
|
|
# Because we should have a fall back plan in case the field test
|
|
# has an odd configuration.
|
|
default_value.inquiry
|
|
end
|
|
end
|