-
Notifications
You must be signed in to change notification settings - Fork 254
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
[TOPSORT] Action Definitions for Banner Events #2655
base: main
Are you sure you want to change the base?
Conversation
* feat(actions): initial template, types and presets
* tests & snapshots
* fix: repeated asserts in test
Co-authored-by: Thomas Gilbert <[email protected]>
Co-authored-by: Thomas Gilbert <[email protected]>
Co-authored-by: Thomas Gilbert <[email protected]>
* fix: correct partnerAction values
Co-authored-by: Thomas Gilbert <[email protected]>
Regarding the |
@agustinespildora appreciate for the thorough PR! Looks good to me.
Yeah this fails when coming from a ofrked branch, so no worries there. That said, we're going into a deploy freeze for the Xmas and New Years holidays, so this PR won't go out until the New Year. |
That's a pitty, no chance to be able to slip this into some hot-fix or something? |
return client.sendEvent({ | ||
impressions: [payload] | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @agustinespildora - how is this Action any different to the Impression Action? It seems to send the exact same payload to Topsort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @joe-ayoub-segment, this event sends the same fields as the Impression action, and thus to the API there is no difference. But the value of the resolvedBidId is decoded differently on our side, and there is a difference on the ecommerce side too, because it needs to detect a Banner impression and send a Banner resolvedBidId instead of a product resolvedBidId. That's why we decided to name it as a different event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is addressed and done
return client.sendEvent({ | ||
clicks: [payload] | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi again @agustinespildora (and @tcgilbert ) - why is the payload for this new Action identical to the payload send in the existing click Action?
If they are the same, then a new Action might not be the best way to do this. You could just add a new Preset which uses the 'click' Action but sets the subscription to be sets the subscription to 'type = "track" and event = "Banner Clicked"'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point now. I can do that change as I believe makes sense. Thanks for the tip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done
hi @agustinespildora our next deploy is on 7th Jan. |
@agustinespildora - Yes, @joe-ayoub-segment raises a great point. If there’s no functional difference between some of the actions (e.g., "Banner Click" vs. "Banner Impression"), it’s probably best to consolidate them into a single action. You could then create multiple presets to differentiate how the single action can be used |
@tcgilbert @joe-ayoub-segment comments are addressed. Sorry for the last minute change, but I have tested thorougly and it is all working. Thanks for the suggestion! |
Note
Ready to review.
Topsort Banner Events
This is a destination so that eCommerce sites that already use Segment (to track actions) can integrate Topsort events for sponsored listings seamlessly. With this PR we now add the events integration for banner ads too.
Intro to Topsort Banners
Topsort allows to monetize through banner auction ads served in your site. To learn more about Topsort you can visit our docs.
This destination already helps you track the 3 main events Topsort needs to serve the most relevant sponsored listing ads:
impressions
,clicks
andpurchases
. Here is the new events we map for the banner events:click
should be triggered with the bannerresolvedBidId
.purchase
event should be reported to Topsort. You can report a purchase whether it has promoted products or not. We will take care of filtering relevant promoted products inside the completed order given the information about promoted clicks. For more information on banner attribution with Topsort you can go to https://api.docs.topsort.com/api-reference/examples/sponsored-banners/attributionChanges
End to end tests
Click event with additional attribution in local Action Tester
Impression event with additional attribution in local Action Tester
Banner Impression event with local Action Tester (updated with last changes Jan 7th)
Banner Click event with local Action Tester (updated with last changes Jan 7th)