feat(api): Accepting all types for attributes values #125
Conversation
|
Going to wait on this until we resolve and merge the Python PR, then we should make sure this matches the requirements over there as well @msohailhussain |
|
build |
lib/optimizely/event_builder.rb
Outdated
|
|
||
| attributes&.keys&.each do |attribute_key| | ||
| # Omit null attribute values | ||
| # Omit invalid attribute values. |
There was a problem hiding this comment.
correct comment is "Omit attribute values that are not supported by the log endpoint."
we need to check attribute key is string or not?
mikeproeng37
left a comment
mikeproeng37
left a comment
There was a problem hiding this comment.
Mostly good. Just have a few comments
spec/condition_spec.rb
Outdated
| expect(@condition_evaluator.evaluate(condition)).to be true | ||
| end | ||
|
|
||
| it 'should evaluate to true for evaluate when user attributes evaluate true' do |
There was a problem hiding this comment.
Can you re-word this: should evaluate to true when user attributes match the audience conditions
spec/event_builder_spec.rb
Outdated
| end | ||
|
|
||
| it 'should create a valid Event when create_impression_event is called with attributes of different valid types' do | ||
| @expected_impression_params[:visitors][0][:attributes].unshift( |
There was a problem hiding this comment.
Can you copy this into a new variable and mutate that one for use instead?
spec/event_builder_spec.rb
Outdated
| end | ||
|
|
||
| it 'should create a valid Event and exclude attributes of invalid types' do | ||
| @expected_impression_params[:visitors][0][:attributes].unshift( |
mikeproeng37
left a comment
mikeproeng37
left a comment
There was a problem hiding this comment.
lgtm, just have a small suggestion
lib/optimizely/decision_service.rb
Outdated
| bucketing_id = attributes[Optimizely::Helpers::Constants::CONTROL_ATTRIBUTES['BUCKETING_ID']] | ||
| @config.logger.log(Logger::DEBUG, "Setting the bucketing ID '#{bucketing_id}'") | ||
| end | ||
| attributes ||= {} |
There was a problem hiding this comment.
Let's return the user_id if attributes is not defined or null
|
@rashidsp looks like the test failures are legit. There are some lint errors. |
aliabbasrizvi
left a comment
aliabbasrizvi
left a comment
There was a problem hiding this comment.
Setting to revise so that rebase issues could be resolved and @mikeng13 's feedback be addressed
5 participants
Note:
ruby-sdk/lib/optimizely/helpers/validator.rb
Line 43 in 0e9980a
ATTRIBUTE_VALID_TYPES.include?(attribute_value.class) can't be used as Fixnum and Bignum are deprecated in ruby 2.4 and Rubocop fails: Lint/UnifiedInteger: Use Integer instead of Fixnum.