fix(set_forced_variation): Treats empty variation key as invalid and does not reset forced variation. (#137)

Author: rashidsp

lib/optimizely/project_config.rb

@@ -329,20 +329,17 @@ def set_forced_variation(experiment_key, user_id, variation_key)
       #
       # Returns a boolean value that indicates if the set completed successfully.
 
-      return false unless Optimizely::Helpers::Validator.inputs_valid?(
-        {
-          user_id: user_id,
-          experiment_key: experiment_key
-        }, @logger, Logger::DEBUG
-      )
+      input_values = {user_id: user_id, experiment_key: experiment_key}
+      input_values[:variation_key] = variation_key unless variation_key.nil?
+      return false unless Optimizely::Helpers::Validator.inputs_valid?(input_values, @logger, Logger::DEBUG)
 
       experiment = get_experiment_from_key(experiment_key)
       experiment_id = experiment['id'] if experiment
       #  check if the experiment exists in the datafile
       return false if experiment_id.nil? || experiment_id.empty?
 
       #  clear the forced variation if the variation key is null
-      if variation_key.nil? || variation_key.empty?
+      if variation_key.nil?
         @forced_variation_map[user_id].delete(experiment_id) if @forced_variation_map.key? user_id
         @logger.log(Logger::DEBUG, "Variation mapped to experiment '#{experiment_key}' has been removed for user "\
                     "'#{user_id}'.")

spec/project_config_spec.rb

@@ -927,10 +927,11 @@
                                                      "Variation mapped to experiment '#{@valid_experiment[:key]}' has been removed for user '#{@user_id}'.")
     end
     # Variation key is an empty string
-    it 'should delete forced varaition maping, log a message and return true when variation_key is passed as empty string' do
-      expect(config.set_forced_variation(@valid_experiment[:key], @user_id, '')).to eq(true)
+    it 'should persist forced variation mapping, log a message and return false when variation_key is passed as empty string' do
+      expect(config.set_forced_variation(@valid_experiment[:key], @user_id, '')).to eq(false)
       expect(spy_logger).to have_received(:log).with(Logger::DEBUG,
-                                                     "Variation mapped to experiment '#{@valid_experiment[:key]}' has been removed for user '#{@user_id}'.")
+                                                     'Variation key is invalid')
+      expect(config.get_forced_variation(@valid_experiment[:key], @user_id)).to eq(nil)
     end
     # Variation key does not exist in the datafile
     it 'return false when variation_key is not in datafile' do
@@ -958,7 +959,8 @@
       expect(Optimizely::Helpers::Validator).to receive(:inputs_valid?).with(
         {
           user_id: @user_id,
-          experiment_key: @valid_experiment[:key]
+          experiment_key: @valid_experiment[:key],
+          variation_key: @valid_variation[:key]
         }, spy_logger, Logger::DEBUG
       )
       config.set_forced_variation(@valid_experiment[:key], @user_id, @valid_variation[:key])
@@ -985,9 +987,6 @@
       variation = config.get_forced_variation(@valid_experiment[:key], @user_id)
       expect(variation['id']).to eq(@valid_variation_2[:id])
       expect(variation['key']).to eq(@valid_variation_2[:key])
-
-      expect(config.set_forced_variation(@valid_experiment[:key], @user_id, '')).to eq(true)
-      expect(config.get_forced_variation(@valid_experiment[:key], @user_id)).to eq(nil)
     end
 
     # Set variation on multiple experiments for one user.