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

fix several issues in appnexus video bids #4154

Merged
merged 1 commit into from
Sep 10, 2019
Merged

Conversation

jsnellbaker
Copy link
Collaborator

@jsnellbaker jsnellbaker commented Sep 4, 2019

Type of change

  • Bugfixes

Description of change

This PR addresses several issues that were present in how video bids were generated in the appnexusBidAdapter (and in cases when any appnexus alias was used, as reported under #3364 ).

Below is a short summary of the issues:

  1. Reported in EMXDigital outstream does not set VAST url #3364.

For outstream bids, the appnexus bidder did not properly populate the typical bid.vastXml or bid.vastUrl fields.

The bidder was coded to only look for the asset_url value in the raw bid response, but this value is only used by instream or adpod type bids. For outstream bids, the ad server returns vast XML code (via the rtbBid.rtb.video.content property) and should be set to the bid.vastXml field.

Fix: Added logic to look at the proper fields given the context used in the video bid object.

  1. Reported in Appnexus outstream should not check if renderer is defined server side #4025.

When publishers have the renderer defined in an adUnit, they may have cases where the placement in the appnexus adserver is not automatically configured to return a renderer asset.

Due to some logic used in the bidder (i.e. it assumed it had an outstream bid only if the renderer_url was present in the raw bid response from the AN adserver), this prevented some extra video bid information from being attached to the bid object (ie bid.adResponse).

This lack of data in the bid in turn hindered the bid from rendering with the adUnit's renderer when the above use-case was used.

Fix: Updated the logic we use to consider what is an outstream bid and moved some logic around so proper values are set for outstream bids.

  1. Observed internally.

When looking for the renderer_options for outstream bids, it was hard-coded to always look at the first bid in the bidderRequest object. When there are multiple bids of the same bidder, the desired video bid that should be referenced isn't always going to be first in the list.

Fix: Updated the logic to find the correct bid in the bidderRequest object and use that to read the renderer_options if they're defined.

Other information

Fixes #3364
Fixes #4025

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.

lgtm

@xavierleune
Copy link
Contributor

👍 thanks @jsnellbaker

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

LGTM. @jsnellbaker can you manually test against regressions for adPod / long form to make sure this is working as expected still?

@jsnellbaker
Copy link
Collaborator Author

Hi @mkendall07 I checked the longform test pages and they were working fine.

@jsnellbaker jsnellbaker merged commit 2d0b75c into master Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants