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

PBS Bid Adapter: Pass video.placement when context is instream (#5687) #5716

Merged
merged 1 commit into from
Sep 10, 2020

Conversation

robertrmartinez
Copy link
Collaborator

#5687

Type of change

  • Other

Description of change

If adUnit is video and context is instream and placement is not defined, set it to 1.

@robertrmartinez robertrmartinez changed the title implement issue #5687 PBS Bid Adapter: Pass video.placement when context is instream (#5687) Sep 8, 2020
@robertrmartinez robertrmartinez linked an issue Sep 8, 2020 that may be closed by this pull request
@bretg bretg requested a review from Fawke September 8, 2020 21:45
@@ -509,6 +509,22 @@ describe('S2S Adapter', function () {
expect(requestBid.imp[0].video).to.not.exist;
});

it('should default video placement if not defined and instream', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the test description be, "should set default video placement if "placement" not defined in instream".
What happens if context is Outstream? I understand, in case of instream, the value of mediaTypes.video.placement should be 1, but, in the case of Outstream, what should be the value ofmediaTypes.video.placement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in the case of Outstream, what should be the value ofmediaTypes.video.placement

It should not default / add anything if context is outstream

@bretg
Copy link
Collaborator

bretg commented Sep 9, 2020

in the case of Outstream, what should be the value of mediaTypes.video.placement be?

The Video Taskforce is going to review all video params (including placement) and document those that need to be added to the current list of required fields. ^^ @mike-chowla

In the meantime, I don't think any docs are necessary for this PR.

@bretg bretg merged commit c01cab1 into master Sep 10, 2020
@bretg bretg deleted the pbsPlacement branch September 10, 2020 13:41
@bretg bretg removed the needs docs label Sep 11, 2020
BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass video.placement through to Prebid Server
3 participants