-
Notifications
You must be signed in to change notification settings - Fork 3
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
YL-3989: Accept NATIVE yieldprobe response #2
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import * as utils from '../src/utils.js' | ||
import { registerBidder } from '../src/adapters/bidderFactory.js' | ||
import find from 'core-js-pure/features/array/find.js' | ||
import { VIDEO, BANNER } from '../src/mediaTypes.js' | ||
import { VIDEO, BANNER, NATIVE } from '../src/mediaTypes.js' | ||
import { Renderer } from '../src/Renderer.js' | ||
|
||
const ENDPOINT = 'https://ad.yieldlab.net' | ||
|
@@ -14,7 +14,7 @@ const GVLID = 70 | |
export const spec = { | ||
code: BIDDER_CODE, | ||
gvlid: GVLID, | ||
supportedMediaTypes: [VIDEO, BANNER], | ||
supportedMediaTypes: [VIDEO, BANNER, NATIVE], | ||
|
||
isBidRequestValid: function (bid) { | ||
if (bid && bid.params && bid.params.adslotId && bid.params.supplyId) { | ||
|
@@ -142,6 +142,27 @@ export const spec = { | |
} | ||
} | ||
|
||
if (isNative(bidRequest, adType)) { | ||
const url = `${ENDPOINT}/d/${matchedBid.id}/${bidRequest.params.supplyId}/?ts=${timestamp}${extId}${gdprApplies}${gdprConsent}${pvId}` | ||
alex-ylb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bidResponse.adUrl = url | ||
bidResponse.mediaType = NATIVE | ||
const nativeImageAssetObj = matchedBid.native.assets.find(e => e.id === 2) | ||
const nativeImageAsset = nativeImageAssetObj ? nativeImageAssetObj.img : {url: '', w: 0, h: 0}; | ||
const nativeTitleAsset = matchedBid.native.assets.find(e => e.id === 1) | ||
const nativeBodyAsset = matchedBid.native.assets.find(e => e.id === 3) | ||
bidResponse.native = { | ||
title: nativeTitleAsset ? nativeTitleAsset.title.text : '', | ||
body: nativeBodyAsset ? nativeBodyAsset.data.value : '', | ||
image: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we support image assets only? What about video assets? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to Mareike we only support image assets. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thought, I did not spot in code where we return video native assets. |
||
url: nativeImageAsset.url, | ||
width: nativeImageAsset.w, | ||
height: nativeImageAsset.h, | ||
}, | ||
clickUrl: matchedBid.native.link.url, | ||
impressionTrackers: matchedBid.native.imptrackers, | ||
}; | ||
} | ||
|
||
bidResponses.push(bidResponse) | ||
} | ||
}) | ||
|
@@ -159,6 +180,16 @@ function isVideo (format, adtype) { | |
return utils.deepAccess(format, 'mediaTypes.video') && adtype.toLowerCase() === 'video' | ||
} | ||
|
||
/** | ||
* Is this a native format? | ||
* @param {Object} format | ||
* @param {String} adtype | ||
* @returns {Boolean} | ||
*/ | ||
function isNative(format, adtype) { | ||
return utils.deepAccess(format, 'mediaTypes.native') && adtype.toLowerCase() === 'native' | ||
} | ||
|
||
/** | ||
* Is this an outstream context? | ||
* @param {Object} format | ||
|
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.
Beside this I will need to create another small PR in https://github.com/prebid/prebid.github.io/blob/1174621dbc456ba79a44d4d2de4055fcab547f33/dev-docs/bidders/yieldlab.md#L6
media_types: video, banner, native
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.
Did you already create the pull request? Can you add a link to the other pull request then?
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.
Not yet. I thought to create two PRs in parallel against Prebid.js github. After we merge this PR to our repository I can create them together
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 think we should have a fork of the documentation repo as well and create pull requests in parallel in both forks. This would align best with our current workflow.
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.
@brushmate , I just create a PR: yieldlab/prebid.github.io#4