Skip to content
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

Event stats #192

Merged
merged 8 commits into from
Aug 4, 2019
Merged

Event stats #192

merged 8 commits into from
Aug 4, 2019

Conversation

samparsky
Copy link
Contributor

@samparsky samparsky commented Jul 10, 2019

Fixes #190 and #191

@samparsky samparsky force-pushed the eventStats branch 3 times, most recently from 4f6c3ca to e804f1a Compare July 10, 2019 18:39
@samparsky samparsky requested a review from Ivshti July 18, 2019 12:59
const updateChannelPrice = async ev => {
const price = ev.type === 'UPDATE_IMPRESSION_PRICE' ? ev.price : '0'
await channelsCol.updateOne({ id: channelId }, { $set: { currentPricePerImpression: price } })
if (ev.type === eventTypes.IMPRESSION_PRICE_PER_CASE) {
Copy link
Member

Choose a reason for hiding this comment

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

later improvement: consider merging IMPRESSION_PRICE_PER_CASE and UPDATE_IMPRESSION_PRICE into one event (considering that this is still not frozen in spec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh okay, which event name should it be UPDATE_IMPRESSION_PRICE ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but start with a PR to the documentation with the proposed interface

Copy link
Member

Choose a reason for hiding this comment

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

see this: #203

// for the event stat
// it can be publisher prefixed for a specific publisher
const result = channel.pricePerImpressionCase.find(
priceCase => priceCase.stat === ev.stat || priceCase.stat === `${ev.publisher}:${ev.stat}`
Copy link
Member

Choose a reason for hiding this comment

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

this allows setting the price per-publisher, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@Ivshti Ivshti merged commit a3b59a4 into AmbireTech:master Aug 4, 2019
@Ivshti Ivshti mentioned this pull request Dec 13, 2019
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.

eventAggregates should save statistics
2 participants