-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add video support in Grid Bid Adapter #3545
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
57210b5
Added Grid Bid Adapter
TheMediaGrid 2a0bad2
remove priceType from TheMediaGrid Bid Adapter
TheMediaGrid 12b9f52
Merge remote-tracking branch 'upstream/master'
TheMediaGrid f3a8ffe
Add video support in Grid Bid Adapter
TheMediaGrid 8e2083c
Added test parameter for video slot
TheMediaGrid bc9d9d4
Merge remote-tracking branch 'upstream/master'
TheMediaGrid 14e9ed7
update Grid Bid Adapter to set size in response bid
TheMediaGrid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ Maintainer: grid-tech@themediagrid.com | |
# Description | ||
|
||
Module that connects to Grid demand source to fetch bids. | ||
Grid bid adapter supports Banner and Video (instream and outstream). | ||
|
||
# Test Parameters | ||
``` | ||
|
@@ -35,6 +36,19 @@ Module that connects to Grid demand source to fetch bids. | |
} | ||
} | ||
] | ||
} | ||
}, | ||
{ | ||
code: 'test-div', | ||
sizes: [[728, 90]], | ||
mediaTypes: { video: {} }, | ||
bids: [ | ||
{ | ||
bidder: "grid", | ||
params: { | ||
uid: 11 | ||
} | ||
} | ||
] | ||
} | ||
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. Thanks for good test parameters which return valid bid responses! This is great. |
||
]; | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,7 @@ describe('TheMediaGrid Adapter', function () { | |
|
||
describe('interpretResponse', function () { | ||
const responses = [ | ||
{'bid': [{'price': 1.15, 'adm': '<div>test content 1</div>', 'auid': 1, 'h': 250, 'w': 300}], 'seat': '1'}, | ||
{'bid': [{'price': 1.15, 'adm': '<div>test content 1</div>', 'auid': 1, 'h': 250, 'w': 300, dealid: 11}], 'seat': '1'}, | ||
{'bid': [{'price': 0.5, 'adm': '<div>test content 2</div>', 'auid': 2, 'h': 90, 'w': 728}], 'seat': '1'}, | ||
{'bid': [{'price': 0, 'auid': 3, 'h': 250, 'w': 300}], 'seat': '1'}, | ||
{'bid': [{'price': 0, 'adm': '<div>test content 4</div>', 'h': 250, 'w': 300}], 'seat': '1'}, | ||
|
@@ -157,12 +157,13 @@ describe('TheMediaGrid Adapter', function () { | |
'requestId': '659423fff799cb', | ||
'cpm': 1.15, | ||
'creativeId': 1, | ||
'dealId': undefined, | ||
'dealId': 11, | ||
'width': 300, | ||
'height': 250, | ||
'ad': '<div>test content 1</div>', | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'banner', | ||
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. Thank you for > 80% Test Coverage! 👍 |
||
'netRevenue': false, | ||
'ttl': 360, | ||
} | ||
|
@@ -214,25 +215,27 @@ describe('TheMediaGrid Adapter', function () { | |
'requestId': '300bfeb0d71a5b', | ||
'cpm': 1.15, | ||
'creativeId': 1, | ||
'dealId': undefined, | ||
'dealId': 11, | ||
'width': 300, | ||
'height': 250, | ||
'ad': '<div>test content 1</div>', | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'banner', | ||
'netRevenue': false, | ||
'ttl': 360, | ||
}, | ||
{ | ||
'requestId': '5703af74d0472a', | ||
'cpm': 1.15, | ||
'creativeId': 1, | ||
'dealId': undefined, | ||
'dealId': 11, | ||
'width': 300, | ||
'height': 250, | ||
'ad': '<div>test content 1</div>', | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'banner', | ||
'netRevenue': false, | ||
'ttl': 360, | ||
}, | ||
|
@@ -246,6 +249,7 @@ describe('TheMediaGrid Adapter', function () { | |
'ad': '<div>test content 2</div>', | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'banner', | ||
'netRevenue': false, | ||
'ttl': 360, | ||
} | ||
|
@@ -255,6 +259,87 @@ describe('TheMediaGrid Adapter', function () { | |
expect(result).to.deep.equal(expectedResponse); | ||
}); | ||
|
||
it('should get correct video bid response', function () { | ||
const bidRequests = [ | ||
{ | ||
'bidder': 'grid', | ||
'params': { | ||
'uid': '1' | ||
}, | ||
'adUnitCode': 'adunit-code-1', | ||
'sizes': [[300, 250], [300, 600]], | ||
'bidId': '659423fff799cb', | ||
'bidderRequestId': '5f2009617a7c0a', | ||
'auctionId': '1cbd2feafe5e8b', | ||
'mediaTypes': { | ||
'video': { | ||
'context': 'instream' | ||
} | ||
} | ||
}, | ||
{ | ||
'bidder': 'grid', | ||
'params': { | ||
'uid': '2' | ||
}, | ||
'adUnitCode': 'adunit-code-1', | ||
'sizes': [[300, 250], [300, 600]], | ||
'bidId': '2bc598e42b6a', | ||
'bidderRequestId': '5f2009617a7c0a', | ||
'auctionId': '1cbd2feafe5e8b', | ||
'mediaTypes': { | ||
'video': { | ||
'context': 'instream' | ||
} | ||
} | ||
} | ||
]; | ||
const response = [ | ||
{'bid': [{'price': 1.15, 'adm': '<VAST version=\"3.0\">\n<Ad id=\"21341234\"><\/Ad>\n<\/VAST>', 'auid': 1, content_type: 'video', w: 300, h: 600}], 'seat': '2'}, | ||
{'bid': [{'price': 1.00, 'adm': '<VAST version=\"3.0\">\n<Ad id=\"21331274\"><\/Ad>\n<\/VAST>', 'auid': 2, content_type: 'video'}], 'seat': '2'} | ||
]; | ||
const request = spec.buildRequests(bidRequests); | ||
const expectedResponse = [ | ||
{ | ||
'requestId': '659423fff799cb', | ||
'cpm': 1.15, | ||
'creativeId': 1, | ||
'dealId': undefined, | ||
'width': 300, | ||
'height': 600, | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'video', | ||
'netRevenue': false, | ||
'ttl': 360, | ||
'vastXml': '<VAST version=\"3.0\">\n<Ad id=\"21341234\"><\/Ad>\n<\/VAST>', | ||
'adResponse': { | ||
'content': '<VAST version=\"3.0\">\n<Ad id=\"21341234\"><\/Ad>\n<\/VAST>' | ||
} | ||
}, | ||
{ | ||
'requestId': '2bc598e42b6a', | ||
'cpm': 1.00, | ||
'creativeId': 2, | ||
'dealId': undefined, | ||
'width': 300, | ||
'height': 250, | ||
'bidderCode': 'grid', | ||
'currency': 'USD', | ||
'mediaType': 'video', | ||
'netRevenue': false, | ||
'ttl': 360, | ||
'vastXml': '<VAST version=\"3.0\">\n<Ad id=\"21331274\"><\/Ad>\n<\/VAST>', | ||
'adResponse': { | ||
'content': '<VAST version=\"3.0\">\n<Ad id=\"21331274\"><\/Ad>\n<\/VAST>' | ||
} | ||
} | ||
]; | ||
|
||
const result = spec.interpretResponse({'body': {'seatbid': response}}, request); | ||
expect(result).to.deep.equal(expectedResponse); | ||
}); | ||
|
||
it('handles wrong and nobid responses', function () { | ||
const bidRequests = [ | ||
{ | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 this all looks great,
I have a question as to why when it is video there is no
bidResponse.width
orbidResponse.height
added?I see it is not sent or returned by your adserver.
You can see in the below screenshot that the KVP that gets set for the size turns out to be
undefinedxundefined
I have a feeling this may cause problems for certain publishers ad servers maybe?
Do you think it is a good idea to set the width and height based on the bidRequest object?
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.
According to Prebid Docs:
http://prebid.org/dev-docs/bidder-adaptor.html#interpreting-the-response
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.
width and height are required.
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, thank you for the catch, we have updated code, please review again.