8000 refactor: Changed the way reasons are being returned from decide APIs by zashraf1985 · Pull Request #279 · optimizely/ruby-sdk · GitHub
[go: up one dir, main page]

Skip to content

refactor: Changed the way reasons are being returned from decide APIs #279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jan 7, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixed a few more methods
  • Loading branch information
zashraf1985 committed Dec 16, 2020
commit 7cbf993c014e6d1fd094644804555d1b1251afc9
40 changes: 20 additions & 20 deletions lib/optimizely/decision_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,17 @@ def get_variation(project_config, experiment_key, user_id, attributes = nil, dec
return forced_variation['id'] if forced_variation

# Check if user is in a white-listed variation
whitelisted_variation_id = get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons)
whitelisted_variation_id, reasons_received = get_whitelisted_variation_id(project_config, experiment_key, user_id)
decide_reasons&.push(*reasons_received)
return whitelisted_variation_id if whitelisted_variation_id

should_ignore_user_profile_service = decide_options.include? Optimizely::Decide::OptimizelyDecideOption::IGNORE_USER_PROFILE_SERVICE
# Check for saved bucketing decisions if decide_options do not include ignoreUserProfileService
unless should_ignore_user_profile_service
user_profile = get_user_profile(user_id, decide_reasons)
saved_variation_id = get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons)
user_profile, reasons_received = get_user_profile(user_id)
decide_reasons&.push(*reasons_received)
saved_variation_id, reasons_received = get_saved_variation_id(project_config, experiment_id, user_profile)
decide_reasons&.push(*reasons_received)
if saved_variation_id
message = "Returning previously activated variation ID #{saved_variation_id} of experiment '#{experiment_key}' for user '#{user_id}' from user profile."
@logger.log(Logger::INFO, message)
Expand Down Expand Up @@ -363,7 +366,7 @@ def get_forced_variation(project_config, experiment_key, user_id)

private

def get_whitelisted_variation_id(project_config, experiment_key, user_id, decide_reasons = nil)
def get_whitelisted_variation_id(project_config, experiment_key, user_id)
# Determine if a user is whitelisted into a variation for the given experiment and return the ID of that variation
#
# project_config - project_config - Instance of ProjectConfig
Expand All @@ -374,52 +377,49 @@ def get_whitelisted_variation_id(project_config, experiment_key, user_id, decide

whitelisted_variations = project_config.get_whitelisted_variations(experiment_key)

return nil unless whitelisted_variations
return nil, nil unless whitelisted_variations

whitelisted_variation_key = whitelisted_variations[user_id]

return nil unless whitelisted_variation_key
return nil, nil unless whitelisted_variation_key

whitelisted_variation_id = project_config.get_variation_id_from_key(experiment_key, whitelisted_variation_key)

unless whitelisted_variation_id
message = "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}', which is not in the datafile."
@logger.log(Logger::INFO, message)
decide_reasons&.push(message)
return nil
return nil, message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok to return a string instead of a string array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if you notice, i am using splat operator * when pushing into the array. it flattens the messages if its an array or uses it as is if its a single message.

end

message = "User '#{user_id}' is whitelisted into variation '#{whitelisted_variation_key}' of experiment '#{experiment_key}'."
@logger.log(Logger::INFO, message)
decide_reasons&.push(message)

whitelisted_variation_id
[whitelisted_variation_id, message]
end

def get_saved_variation_id(project_config, experiment_id, user_profile, decide_reasons = nil)
def get_saved_variation_id(project_config, experiment_id, user_profile)
# Retrieve variation ID of stored bucketing decision for a given experiment from a given user profile
#
# project_config - project_config - Instance of ProjectConfig
# experiment_id - String experiment ID
# user_profile - Hash user profile
#
# Returns string variation ID (nil if no decision is found)
return nil unless user_profile[:experiment_bucket_map]
return nil, nil unless user_profile[:experiment_bucket_map]

decision = user_profile[:experiment_bucket_map][experiment_id]
return nil unless decision
return nil, nil unless decision

variation_id = decision[:variation_id]
return variation_id if project_config.variation_id_exists?(experiment_id, variation_id)
return variation_id, nil if project_config.variation_id_exists?(experiment_id, variation_id)

message = "User '#{user_profile['user_id']}' was previously bucketed into variation ID '#{variation_id}' for experiment '#{experiment_id}', but no matching variation was found. Re-bucketing user."
@logger.log(Logger::INFO, message)
decide_reasons&.push(message)

nil
[nil, message]
end

def get_user_profile(user_id, decide_reasons = nil)
def get_user_profile(user_id)
# Determine if a user is forced into a variation for the given experiment and return the ID of that variation
#
# user_id - String ID for the user
Expand All @@ -431,17 +431,17 @@ def get_user_profile(user_id, decide_reasons = nil)
experiment_bucket_map: {}
}

return user_profile unless @user_profile_service
return user_profile, nil unless @user_profile_service

message = nil
begin
user_profile = @user_profile_service.lookup(user_id) || user_profile
rescue => e
message = "Error while looking up user profile for user ID '#{user_id}': #{e}."
@logger.log(Logger::ERROR, message)
decide_reasons&.push(message)
end

user_profile
[user_profile, message]
end

def save_user_profile(user_profile, experiment_id, variation_id)
Expand Down
0