PE-87: Implement Prebid Adapter #1

PavloMalashnyak · 2023-09-26T14:25:21Z

Story

PE-87

Implement BT Bid Adapter that send one request for each configured ad unit to the our prebid server

Testing

What tests did you perform to ensure that this code functions as expected?

Manual testing
Unit tests
Integration tests
A/B tests

lawrence-hoo · 2023-09-28T17:58:24Z

modules/BTBidAdapter.js

+      return [];
+    }
+
+    const seatBids = serverResponse?.body.seatbid || [];


if we already check for serverResponse on line 37, I don't think we need the conditional operator after serverResponse - maybe serverResponse.body?.seatbid though?

or we can combine this 2 checks:

const seatBids = serverResponse?.body?.seatbid || []; if (!seatBids.length) { return []; }

btw, why the server responses with seatbid while it seems to be an array?

lawrence-hoo · 2023-09-28T18:01:06Z

modules/BTBidAdapter.js

+    const newBids = seatBids.flatMap((seat) => {
+      return seat.bid.map((bid) => {
+        const bidResponse = getNewBidResponse(bid);
+        bidResponse.btBidderCode = seat.seat;


Would it make sense to pass in seat into getNewBidResponse and have bidResponse.btBidderCode = seat.seat be done inside the function instead?

Then I think you could do:

const newBids = seatBids.flatMap(seat => seat.bid.map(bid => getNewBidResponse(bid, seat)) );

I'm not super familiar with this, so feel free to leave it as is if you think it makes more sense to keep them separate

bidResponse.btBidderCode = seat.seat; was done this way, due to Relay implementation specific. so, if we can adjust this and probably rename (not to do seat.seat), it would be great! I guess it requires BE change as well.

lawrence-hoo · 2023-09-28T18:04:30Z

modules/BTBidAdapter.js

+  const { ab, siteId, ...bidderParams } = params;
+  const imps = [];
+
+  for (const mediaType in mediaTypes) {


Personally I'm more of a fan of:
const imps = Object.entries(mediaTypes).map(([mediaType, currentType]) => ...)

Not really a huge deal either way; if you keep the for ... in I wonder if you'd need to add a hasOwnProperty check for safety

do we expect mediaType to be anything other than a banner?
I mean, you specified supportedMediaTypes: [BANNER], but here if we get VIDEO, we won't error. am I missing something?

Rn we only support banner, but in future we plan to support other types. Might be worth adding check for supported media types

lawrence-hoo · 2023-09-28T18:13:26Z

test/spec/modules/BTBidAdapte_spec.js

+      expect(requests[0].bids).to.equal(validBidRequests);
+    });
+
+    it('should build post request ortb2 fields are not present', () => {


Should this be something like: "should build post request even when ortb2 fields are not present"?

lawrence-hoo · 2023-09-28T18:17:45Z

test/spec/modules/BTBidAdapte_spec.js

+      expect(bids.length).to.equal(0);
+    });
+
+    it('should return bids array', () => {


Would it better to have this say something like: "should return bids array when serverResponse is defined and seatbid array is not empty"? Feel free to leave it as-is, I'm not really sure what the convention should be.

MariiaKolisnyk

did you skip logging on purpose?

why do we use this repo? are we going to merge it to our Prebid instance?

MariiaKolisnyk · 2023-09-29T06:59:50Z

modules/BTBidAdapter.js

+      isStr(bid.params.siteId) &&
+      !!bid.params?.ab &&
+      isBoolean(bid.params.ab)
+    );


do you think we can add a log in case of invalid bidRequests and specify what is missing/wrong?

also, we can move out bid.params check to not repeat it:

bid.params && bid.params.siteId && ...

or even something like:

const { siteId, ab } = bid.params || {}; if (!siteId || !isStr(bid.params.siteId)) { logError('BT Bid Adapter: siteId must be string type.'); return false; }

MariiaKolisnyk · 2023-09-29T07:05:54Z

modules/BTBidAdapter.js

+  code: BIDDER_CODE,
+  gvlid: GVLID,
+  supportedMediaTypes: [BANNER],
+  isBidRequestValid: (bid) => {


might be worth adding JSDoc, e.g.:

it's up to you, however I prefer to declare methods separately to make export spec clearer:

function isBidRequestValid() { ... } function buildRequests() { ... } export const spec = { code: BIDDER_CODE, gvlid: GVLID, supportedMediaTypes: [BANNER], isBidRequestValid, buildRequests, ... }

MariiaKolisnyk · 2023-09-29T07:12:50Z

modules/BTBidAdapter.md

+# Params
+
+- `ab` required, whether AdBlock is enabled.
+- `siteId` required, site id.


would be nice to specify the types, table view might work better:

MariiaKolisnyk · 2023-09-29T07:13:35Z

modules/BTBidAdapter.md

+
+The BT service accepts industry standard OpenRTB 2.5 as its payload. The BT Bidder Adapter converts Prebid.js parameters into an OpenRTB 2.5 request.
+
+# Params


should this be Bid Params?

MariiaKolisnyk · 2023-09-29T07:15:48Z

modules/BTBidAdapter.md

+The BT Bidder Adapter provides an interface to the BT Service. The BT Bidder Adapter sends one request to the BT Service per ad unit, but the BT Service will then send one request to each configured exchange. This is functionally similar to Prebid Server. One client side request, bids from many exchanges.
+
+The BT service accepts industry standard OpenRTB 2.5 as its payload. The BT Bidder Adapter converts Prebid.js parameters into an OpenRTB 2.5 request.


so, this is just copy-pasted from Relay docs, we might be more creative, unless this is approved by Product

MariiaKolisnyk · 2023-09-29T07:36:08Z

modules/BTBidAdapter.js

+    const newBids = seatBids.flatMap((seat) => {
+      return seat.bid.map((bid) => {
+        const bidResponse = getNewBidResponse(bid);
+        bidResponse.btBidderCode = seat.seat;


bidResponse.btBidderCode = seat.seat; was done this way, due to Relay implementation specific. so, if we can adjust this and probably rename (not to do seat.seat), it would be great! I guess it requires BE change as well.

MariiaKolisnyk · 2023-09-29T07:37:10Z

modules/BTBidAdapter.js

+    if (!seatBids.length) {
+      return [];
+    }
+    const newBids = seatBids.flatMap((seat) => {


newBids is redundant, we can just return seatBids.flatMap((seat) ...

MariiaKolisnyk · 2023-09-29T07:42:21Z

modules/BTBidAdapter.js

+}
+
+function getBTRequest(validBidRequest, bidderRequest) {
+  let request = {


could be const

MariiaKolisnyk · 2023-09-29T07:44:20Z

modules/BTBidAdapter.js

+  const imps = [];
+
+  for (const mediaType in mediaTypes) {
+    let bidImp = {


could be const

MariiaKolisnyk · 2023-09-29T07:48:29Z

modules/BTBidAdapter.js

+  const { ab, siteId, ...bidderParams } = params;
+  const imps = [];
+
+  for (const mediaType in mediaTypes) {


do we expect mediaType to be anything other than a banner?
I mean, you specified supportedMediaTypes: [BANNER], but here if we get VIDEO, we won't error. am I missing something?

PavloMalashnyak · 2023-09-29T15:04:45Z

did you skip logging on purpose?

why do we use this repo? are we going to merge it to our Prebid instance?

Yes, I didn't log anything as it isn't a part of acceptance criteria. But I could add some logs.

We use this forked repo to contribute to Prebid.js in the future. I think we will merge this into up-prebid.js once they are merged to the Prebid.js

MariiaKolisnyk · 2023-10-06T07:24:33Z

modules/BTBidAdapter.js

I wonder if the file name should start with a lowercase letter? I did not check the convention, but I don't see any files in Prebid that start with a capital letter.

* Kimberlite bid adapter (#1) * initial: bid adapter * styling * Fix: lint (#2) * Fix: lint (#4) * review fixes (#6) * Change: filling request.ext.prebid section (#7) --------- Co-authored-by: Oleg <oleg.s@solta.io>

* PE-87: Implement Prebid Adapter (#1) * PE-87: implement BT Bid Adapter * PE-87: rework adapter to use ortbConverter lib, make requested changes * PE-87: update imports * PE-110: Add user sync logic to the Prebid Adapter (#3) * PE-110: add user sync logic * PE-110: update userSync url * PE-110: check if iframe is enabled before setting params * PE-111: BT Prebid Adapter can request AA ads or regular ads (#2) * PE-120: Send Prebid Bidder info to BT Server (#4) * PE-120: add btBidderCode to the bid object * PE-120: use single quotes for logs string * PE-123: Add More Metadata in site.ext.blockthrough (#5) * PE-123: send additional meta data * PE-123: send auctionID under imp.ext.prebid.blockthrough * PE-123: use ortb2 config to set site.ext params * PE-123: sent auctionId in ext.prebid.blockthrough.auctionID * PE-123: update logs for bidderConfig setup * PE-000: check if blockthrough is defined (#6) * PE-87: remove BT specific logic (#7) * Implement Blockthrough Prebid Adapter * PE-87: Implement Prebid Adapter - misc fixes (#9) * PE-87: rename test file, add bidder config * PE-87: increase ttl * PE-000: fix test * BP-74: Change the way we enable debug (#10) * BP-79: Send GPID as a part of `imp[].ext` (#11) * BP-79: send gpid in imp.ext * BP-79: add optional operator * BP-90: Update Cookie Sync Logic (#12) * BP-90: pass bidder to cookie sync * BP-90: update sync logic, fix typo * BP-90: use const for syncs variable * BP-55: Re-add endpoint URLs (#13) * BP-91: Add prebid JS version to auction request (#14)

PE-87: implement BT Bid Adapter

7de4057

PavloMalashnyak requested review from lawrence-hoo, MariiaKolisnyk, jhonnythrough, caitlinbt and elissa-bt September 26, 2023 14:25

lawrence-hoo reviewed Sep 28, 2023

View reviewed changes

MariiaKolisnyk reviewed Sep 29, 2023

View reviewed changes

PE-87: rework adapter to use ortbConverter lib, make requested changes

bbef4ea

PavloMalashnyak requested review from MariiaKolisnyk and lawrence-hoo October 4, 2023 17:07

lawrence-hoo approved these changes Oct 5, 2023

View reviewed changes

MariiaKolisnyk approved these changes Oct 9, 2023

View reviewed changes

PE-87: update imports

a2d35b2

PavloMalashnyak requested a review from zhongshixi October 13, 2023 18:26

PavloMalashnyak merged commit 79a0a94 into bt-master Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PE-87: Implement Prebid Adapter #1

PE-87: Implement Prebid Adapter #1

PavloMalashnyak commented Sep 26, 2023 •

edited by jira bot

Loading

lawrence-hoo Sep 28, 2023

MariiaKolisnyk Sep 29, 2023

lawrence-hoo Sep 28, 2023 •

edited

Loading

lawrence-hoo Sep 28, 2023

MariiaKolisnyk Sep 29, 2023

lawrence-hoo Sep 28, 2023

MariiaKolisnyk Sep 29, 2023

PavloMalashnyak Sep 29, 2023

lawrence-hoo Sep 28, 2023

lawrence-hoo Sep 28, 2023

MariiaKolisnyk left a comment •

edited

Loading

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

MariiaKolisnyk Sep 29, 2023

PavloMalashnyak commented Sep 29, 2023

MariiaKolisnyk Oct 6, 2023


		The BT service accepts industry standard OpenRTB 2.5 as its payload. The BT Bidder Adapter converts Prebid.js parameters into an OpenRTB 2.5 request.

		# Params

		The BT Bidder Adapter provides an interface to the BT Service. The BT Bidder Adapter sends one request to the BT Service per ad unit, but the BT Service will then send one request to each configured exchange. This is functionally similar to Prebid Server. One client side request, bids from many exchanges.

		The BT service accepts industry standard OpenRTB 2.5 as its payload. The BT Bidder Adapter converts Prebid.js parameters into an OpenRTB 2.5 request.

PE-87: Implement Prebid Adapter #1

PE-87: Implement Prebid Adapter #1

Conversation

PavloMalashnyak commented Sep 26, 2023 • edited by jira bot Loading

Story

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lawrence-hoo Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MariiaKolisnyk left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PavloMalashnyak commented Sep 29, 2023

Choose a reason for hiding this comment

PavloMalashnyak commented Sep 26, 2023 •

edited by jira bot

Loading

lawrence-hoo Sep 28, 2023 •

edited

Loading

MariiaKolisnyk left a comment •

edited

Loading