Migrating TrustX adapter to 1.0 #1709
Conversation
There was a problem hiding this comment.
@PWyrembak Left some comments.
With your test params, I see a bid response but not able to see an ad.
modules/trustxBidAdapter.js
Outdated
| * @return boolean True if this is a valid bid, and false otherwise. | ||
| */ | ||
| isBidRequestValid: function(bid) { | ||
| return !!(bid.params.uid && bid.adUnitCode); |
There was a problem hiding this comment.
bid.adUnitCode is not required here to validate bid reqeust. It is part of prebid-core and already tested there.
modules/trustxBidAdapter.js
Outdated
| hasEmptySeatbidArray: 'Response has empty seatbid array', | ||
| hasNoArrayOfBids: 'Seatbid from response has no array of bid objects - ' | ||
| }; | ||
| const bidsRequestMap = {}; |
There was a problem hiding this comment.
No global variables. You can create the same in spec.buildRequests scope and add it to request object.
spec.interpretResponse has request object as second argument.
For reference, check rubiconBidAdapter
modules/trustxBidAdapter.js
Outdated
| _responseProcessing(resp, auids, placementMap); | ||
| const payload = { | ||
| u: utils.getTopWindowUrl(), | ||
| pt: window.globalPrebidTrustxPriceType === 'gross' ? 'gross' : 'net', |
There was a problem hiding this comment.
you can pass window.globalPrebidTrustxPriceType via param. This is not allowed.
modules/trustxBidAdapter.js
Outdated
| } | ||
| } | ||
| } | ||
| // const payloadString = JSON.stringify(payload); |
There was a problem hiding this comment.
remove unwanted code.
|
@jaiminpanchal27 Thanks, we have updated the code and pulled the changes to the repo. Could you please review once more? Re: bid responses @ test params. Are you calling TX bidder in the browser? I should be definitely seeing bid responses coming from the bidder. |
There was a problem hiding this comment.
Changes looks good except you need to udpate markdown file.
Also i am not able to validate bids or ad. Using this test page. https://jsfiddle.net/1ajrfpvs/
modules/trustxBidAdapter.md
Outdated
|
|
||
| # Test Parameters | ||
| ``` | ||
| window.globalPrebidTrustxPriceType = 'gross'; // by default is 'net' |
There was a problem hiding this comment.
After update you don't need this in markdown. Update params as required.
There was a problem hiding this comment.
Are you sure you're calling the right prebid location on the page example you've provided? On the page it is "../../build/dev/prebid.js". So I can't see any calls to TX bidder going at all.
|
@PWyrembak The first argument to interpretResponse now looks like this: You'll have to pull master and update the spec so that it looks digs into that object as well now. |
* 'master' of https://github.com/prebid/Prebid.js: (22 commits) Update GetIntent adapter to 1.0 version (prebid#1721) Add `usePaymentRule` param to AN bidders (prebid#1778) New hooks API (replaces monkey-patching for currency) (prebid#1683) Change prebidServer to call client user syncs if they exist (prebid#1734) Fix Centro adapter to allow requests of the same units (prebid#1746) add vastUrl + media type for video bids Prebid Server (prebid#1739) Update adxcg adapter for prebid 1.0 (prebid#1741) Update yieldmoBid adapter request url (prebid#1771) Upgrade Quantcast adapter for Prebid 1.0 (prebid#1753) Fidelity Media Adapter update. Prebid v1.0 (prebid#1719) Kargo Adapter for Prebid 1.0 (prebid#1729) updated for prebid 1.0 api (prebid#1722) Add AdOcean adapter (prebid#1735) Update Conversant adapter to Prebid 1.0 (prebid#1711) Fix test-coverage bug (prebid#1765) Migrating TrustX adapter to 1.0 (prebid#1709) Update Improve Digital adapter for Prebid 1.0 (prebid#1728) Fixed the argument type on getUserSyncs. (prebid#1767) nanointeractive bid adapter (prebid#1627) Validating bid response params (prebid#1738) ...
Type of change
Description of change
Migrating TrustX adapter to 1.0