feat(decision-listener): Incorporated new decision notification listener changes. #166
Conversation
| @@ -137,6 +137,9 @@ def get_variation(experiment_key, user_id, attributes = nil) | |||
| }, @logger, Logger::ERROR | |||
| ) | |||
|
|
|||
| experiment = @config.get_experiment_from_key(experiment_key) | |||
There was a problem hiding this comment.
@rashidsp Can we remove this check from here now?
https://github.com/optimizely/ruby-sdk/blob/master/lib/optimizely/decision_service.rb#L60
lib/optimizely/project_config.rb
Outdated
| @@ -451,6 +460,16 @@ def get_rollout_from_id(rollout_id) | |||
| nil | |||
| end | |||
|
|
|||
| def feature_experiment?(experiment_id) | |||
| # Determines if a given experiment ID belongs to any Feature. | |||
There was a problem hiding this comment.
nit. Determines if a given experiment ID is related to a feature. or Determines if given experiment is a feature test.
spec/project_config_spec.rb
Outdated
| @@ -59,6 +59,21 @@ | |||
| 'test_event_not_running' => config_body['events'][3] | |||
| } | |||
|
|
|||
| expected_experiment_feature_map = { | |||
| '133331' => [ | |||
There was a problem hiding this comment.
An experiment having 2+ feature flags associated with it is an unexpected state. This points to me that the data we are testing with is incorrect / unexpected. Experiment to feature relationship is a many to one mapping and not many to many mapping.
cc @mikeng13
There was a problem hiding this comment.
I just checked and these are mutex experiments, for which we do allow multiple features to be associated. So I think this data representation is correct.
There was a problem hiding this comment.
Just synced offline with @aliabbasrizvi and turns out I had it backwards. An experiment can only be associated to one feature at a time. We should definitely fix this test data as it would otherwise suggest an erroneous case.
spec/project_config_spec.rb
Outdated
| describe '#feature_experiment' do | ||
| let(:config) { Optimizely::ProjectConfig.new(config_body_JSON, logger, error_handler) } | ||
|
|
||
| it 'should return true if the experiment belongs to any feature' do |
There was a problem hiding this comment.
nit. .... if the experiment is a feature test
spec/project_config_spec.rb
Outdated
| expect(config.feature_experiment?(experiment['id'])).to eq(true) | ||
| end | ||
|
|
||
| it 'should return false if the experiment does not belong to any feature' do |
There was a problem hiding this comment.
nit. .... if the experiment is not a feature test
Summary
Test plan
feature-testand project config method.