Skip to content
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

Spotx video Adapter #1326

Merged
merged 10 commits into from
Jul 10, 2017
Merged

Spotx video Adapter #1326

merged 10 commits into from
Jul 10, 2017

Conversation

npeceniak
Copy link
Contributor

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

Add Spotx video bidder adapter

  • test parameters for validating bids
{
              bidder: 'spotx',
              params: {
                placementId: "123456789",
                video: {
                  channel_id: 85394,
                  video_slot: 'contentElement',
                  slot: 'content',
                  hide_skin: false,
                  autoplay: true,
                  ad_mute: false,
                }
              }
}

Be sure to test the integration with your adserver using the Hello World sample page.

  • contact email of the adapter’s maintainer
    npeceniak@spotx.tv
  • official adapter submission

Other information

Nick Peceniak and others added 9 commits June 23, 2017 07:08
	- If passed an id set the slot and video_slot element when initializing the directsdk
Addressing code review comments for spotx adapter.

Add spotx_spec.js file for unit tests.

Increase test coverage

Add check for directSDK slot and video_slot
	- If passed an id set the slot and video_slot element when initializing the directsdk

Undo bracket notation change
@npeceniak npeceniak changed the title Spotx adapter eric Spotx video Adapter Jun 27, 2017
@jaiminpanchal27 jaiminpanchal27 self-assigned this Jun 28, 2017
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@npeceniak
I am getting error "adloader.js:loadScript: Error executing callback" Error: Invalid options: 'video_slot' is required. when testing with hello_world test page.

Also left some minor comments.

// Load the SpotX Direct AdOS SDK onto the page
function loadDSDK()
{
var channelId = bidReq.params.video.channel_id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a function to validate your params.


bid.cpm = KVP_Object.spotx_bid;
bid.vastUrl = url;
bid.descriptionUrl = url;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove descriptionUrl. vastUrl will suffice

	- Added parameter validation function
	- removed descriptionUrl
@npeceniak
Copy link
Contributor Author

I have pushed a new commit addressing your two comments. The error you were getting on the hello_world test page is likely because our adapter requires the slot and video_slot to exist on the page. Something like the following would need to be added.

<div id="mainContainer"> <div id="content"> <video id="contentElement"> <source src="http://rmcdn.2mdn.net/Demo/vast_inspector/android.mp4"></source> <source src="http://rmcdn.2mdn.net/Demo/vast_inspector/android.webm"></source> </video> </div> <div id="adContainer"></div> </div>

I created a test page for use while I was developing the adapter but I did not want to include that test page in the pull request as it was really just for testing. I have attached my working test page here which should be using the same bid parameters as the test parameters above.
IMA_Prebid_Test_html_page.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants