Allow bootstrapped variants when client is disabled #138
Conversation
jdurkee-mdsol
changed the title
lib/unleash/feature_toggle.rb
Outdated
| return context.get_by_name(stickiness) unless stickiness == "default" || context.nil? | ||
| return context.user_id unless context&.user_id.to_s.empty? | ||
| return context.session_id unless context&.session_id.to_s.empty? | ||
| return context.remote_address unless context&.remote_address.to_s.empty? |
There was a problem hiding this comment.
Can you add an explicit test for this case?
I think it should be somewhere around:
- the end of
feature_toggle_spec.rband/or - the end of
client_spec.rb
There was a problem hiding this comment.
Note that since this is a private method, the test should be in the closest public method that calls this. Probably in get_variant() in FeatureToggle and/or Client.
There was a problem hiding this comment.
Do you mean tests that pass in a context missing one of user_id, session_id, remote_address additively each time?
There was a problem hiding this comment.
I am working on some extra tests in FeatureToggle, so it can make sense to call it out of scope for the PR.
But the fix in feature_toggle.rb that I mention could be addressed here right away. FYI: I found the issue when writing these extra tests locally.
Pull Request Test Coverage Report for Build 4983808189
💛 - Coveralls |
Co-authored-by: Renato Arruda <git@rarruda.org>
lib/unleash/feature_toggle.rb
Outdated
| return context.get_by_name(stickiness) unless stickiness == "default" || context.nil? | ||
| return context.user_id unless context&.user_id.to_s.empty? | ||
| return context.session_id unless context&.session_id.to_s.empty? | ||
| return context.remote_address unless context&.remote_address.to_s.empty? |
There was a problem hiding this comment.
I am working on some extra tests in FeatureToggle, so it can make sense to call it out of scope for the PR.
But the fix in feature_toggle.rb that I mention could be addressed here right away. FYI: I found the issue when writing these extra tests locally.
Co-authored-by: Renato Arruda <git@rarruda.org>
Your welcome. Thank you for taking the time to consider, and being so timely with the response. |
About the changes
Attempts to address #137
Variants in a bootstrap configuration are ignored with the client is disabled. These changes remove that limitation, so disabled clients can interact with bootstrapped variants.
Auxiliary changes:
Calling
get_variantallows passing a nil context, however attributes of this context are referenced when calculating the variant salt resulting inundefined method ... for nilerrors. Account for a nil context in this method which results in the default: random number.