Skip to content

[FSSDK-11578] Ruby: Update impression event handling and send notification for global holdout #376

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.

Sign up for GitHub

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 6 commits into from
Oct 22, 2025
Merged

[FSSDK-11578] Ruby: Update impression event handling and send notification for global holdout #376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2959432
[FSSDK-11578] Ruby: Update impression event handling and send notific…
esrakartalOpt Oct 17, 2025
67f2e13
Merge branch 'master' of https://github.com/optimizely/ruby-sdk into …
esrakartalOpt Oct 17, 2025
1712fc7
Fix lint issues
esrakartalOpt Oct 17, 2025
c446e69
Fix lint issue
esrakartalOpt Oct 18, 2025
fbe1cfb
Fix lint and test case errors
esrakartalOpt Oct 20, 2025
ee1c8fb
Update the "get_holdouts_for_flag" comment
esrakartalOpt Oct 22, 2025
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
2 changes: 1 addition & 1 deletion lib/optimizely.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
if decision.is_a?(Optimizely::DecisionService::Decision)
variation = decision['variation']
feature_enabled = variation['featureEnabled']
if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']
if decision.source == Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'] || decision.source == Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']
source_string = Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']
source_info = {
experiment_key: decision.experiment['key'],
Expand Down
9 changes: 6 additions & 3 deletions lib/optimizely/config/datafile_project_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ def initialize(datafile, logger, error_handler)
return unless @holdouts && [email protected]?

@holdouts.each do |holdout|
next unless holdout['status'] == 'Running'

holdout_key = holdout['key']
holdout_id = holdout['id']

Expand Down Expand Up @@ -633,16 +635,17 @@ def rollout_experiment?(experiment_id)
@rollout_experiment_id_map.key?(experiment_id)
end

def get_holdouts_for_flag(flag_key)
def get_holdouts_for_flag(flag_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

HAven't done event handling yet for holdouts myself, but I wonder here, it could be a potential bug if flag_key is used accidentally instead of flag_id in this PR. Are there any checks needed to make sure we use flag_id?
I'm not 100% sure, just pointing to this in case a check is needed. Does swift sdk has any additional guards maybe etc

Copy link
Contributor

Choose a reason for hiding this comment

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

We can a comment for better clarification. For Swift flag_id is none optional, so there is no way to pass null value.

# Helper method to get holdouts from an applied feature flag
#
# flag_key - Key of the feature flag
# flag_id - (REQUIRED) ID of the feature flag
# This parameter is required and should not be null/nil
#
# Returns the holdouts that apply for a specific flag

return [] if @holdouts.nil? || @holdouts.empty?

@flag_holdouts_map[flag_key] || []
@flag_holdouts_map[flag_id] || []
end

def get_holdout(holdout_id)
Expand Down
7 changes: 4 additions & 3 deletions lib/optimizely/decision_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide
# user_context - Optimizely user context instance
#
# Returns DecisionResult struct.
holdouts = project_config.get_holdouts_for_flag(feature_flag['key'])
holdouts = project_config.get_holdouts_for_flag(feature_flag['id'])

if holdouts && !holdouts.empty?
# Has holdouts - use get_decision_for_flag which checks holdouts first
Expand All @@ -194,7 +194,8 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
user_id = user_context.user_id

# Check holdouts
holdouts = project_config.get_holdouts_for_flag(feature_flag['key'])
holdouts = project_config.get_holdouts_for_flag(feature_flag['id'])

holdouts.each do |holdout|
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
reasons.push(*holdout_decision.reasons)
Expand Down Expand Up @@ -275,7 +276,7 @@ def get_variation_for_holdout(holdout, user_context, project_config)
variation, bucket_reasons = @bucketer.bucket(project_config, holdout, bucketing_id, user_id)
decide_reasons.push(*bucket_reasons)

if variation
if variation && !variation['key'].nil? && !variation['key'].empty?
message = "The user '#{user_id}' is bucketed into variation '#{variation['key']}' of holdout '#{holdout['key']}'."
@logger.log(Logger::INFO, message)
decide_reasons.push(message)
Expand Down
Loading