8000 feat(epmodel): Event Processor datamodel by msohailhussain · Pull Request #184 · optimizely/ruby-sdk · GitHub
[go: up one dir, main page]

Skip to content

feat(epmodel): Event Processor datamodel #184

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 6 commits into from
Jul 24, 2019
Merged

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0