Synapse HX Bid Adapter : initial release - #15331
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8a3686751
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀 |
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
|
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
Barecheck - Code coverage reportTotal: 96.67%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀 |
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
|
This PR includes an adapter whose code does not match its file name. Bid adapter modules should be named |
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 935f135f3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (bid.nurl) { | ||
| const url = new URL(bid.nurl); | ||
| url.searchParams.set('cpm', String(bid.cpm)); | ||
| triggerPixel(url.toString()); |
There was a problem hiding this comment.
Preserve win notice URLs before onBidWon
When Synapse returns a normal ORTB nurl (the fixture does this), converter.fromORTB does not leave that field on the Prebid bid; the default banner/video processors consume it into render assets instead. As a result this onBidWon branch never sees bid.nurl for bids produced by this adapter, so the intended win notice with the CPM is not sent on bid win. Preserve bid.nurl in the converter's bidResponse customizer (and avoid also consuming it there if necessary) before relying on this handler.
Useful? React with 👍 / 👎.
| function makeUrl(bidRequests) { | ||
| return `${ENDPOINT_URL}?${formatQS({ pid: bidRequests[0].params.tenantId })}`; |
There was a problem hiding this comment.
Split batched requests by tenant
When a page has multiple Synapse HX bids with different tenantId values, bidderFactory passes them together to buildRequests, but this URL is built only from bidRequests[0] and the ORTB impressions do not carry the other tenants. Those later impressions are therefore sent under the first tenant's pid, causing misattribution or rejection for multi-tenant pages; split requests by tenantId or enforce a single tenant per request.
Useful? React with 👍 / 👎.
Type of change
Description of change
New bidder adapter for Synapse HX
Maintainer: prebid@compas-inc.com
Other information
Docs PR - prebid/prebid.github.io#6657