Skip to content

Conversation

msohailhussain
Copy link
Contributor

@msohailhussain msohailhussain commented Jul 19, 2019

Summary

  • Data models to represent Impression/Conversion event on client/server side.

Test plan

  • Unit tests to verify after serialization datamodel is converted into expected server payload
    Impression Event
    Conversion Event
@msohailhussain msohailhussain added the WIP Work in progress label Jul 19, 2019
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 99.537% when pulling 53deab0 on rashid/EP-Datamodel into ba7d0cc on master.

@coveralls
Copy link

coveralls commented Jul 19, 2019

Coverage Status

Coverage increased (+0.02%) to 99.492% when pulling 20b7226 on rashid/EP-Datamodel into c51968f on master.

#
require_relative 'user_event'
module Optimizely
class ConversionEvent < UserEvent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

where are other attributes?
bot filter?

require_relative 'user_event'
module Optimizely
class ConversionEvent < UserEvent
attr_accessor :event, :event_tags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't we just define attr_reader and pass it in a constructor?

}
end

class Builder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we really need builder classes in ruby ?


module Optimizely
class EventContext
attr_reader :account_id, :project_id, :revision, :client_name,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

how you will set these values?

Copy link
Contributor

Choose a reason for hiding this comment

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

initialized

require_relative 'user_event'
module Optimizely
class ImpressionEvent < UserEvent
attr_reader :user_id, :user_attributes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not user_attributes, it's visitor_attributes

module Optimizely
class ImpressionEvent < UserEvent
attr_reader :user_id, :user_attributes
attr_accessor :experiment, :variation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why accessor?

Copy link
Contributor

Choose a reason for hiding this comment

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

@timestamp = timestamp
@revenue = revenue
@value = value
@event_tags = event_tags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are just tags?

#
module Optimizely
class UserEvent
attr_reader :context, :uuid, :timestamp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

write it as event_context

@rashidsp rashidsp changed the title feat(epmodel): Event Processor datamodel - Do not review feat(epmodel): Event Processor datamodel Jul 22, 2019
@rashidsp rashidsp removed the WIP Work in progress label Jul 22, 2019
@rashidsp rashidsp requested a review from a team July 22, 2019 09:03
@optimizely optimizely deleted a comment from rashidsp Jul 23, 2019
Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

lgtm

@mikeproeng37 mikeproeng37 merged commit eeea135 into master Jul 24, 2019
@rashidsp rashidsp deleted the rashid/EP-Datamodel branch July 24, 2019 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants