Deals not working as documented

[github-nilsson]

Type of issue

I'm having troubles deploying deals as documented on http://prebid.org/adops/deals.html
This may very well be a basic misunderstanding, but there doesn't appear to be any test cases either way so at the very least that is something to address.

Description

On an already successfully deployed prebid integration we add an adapter that returns a dealID in its bids. These are not automatically added to the targeting parameters unless prebid is configured with pbjs.enableSendAllBids().

Steps to reproduce

Given the documentation I would expect the following test case to be correct. The hb_deal_triplelift is however missing.

it('Deal adserver tagering', () => {
      const pre = $$PREBID_GLOBAL$$._bidsReceived;
      $$PREBID_GLOBAL$$._bidsReceived = [];
      const bid_a = Object.assign({},
        bidfactory.createBid(2),
        fixtures.getBidResponses()[0]
      );

      bid_a.dealID = 'test deal';
      bidmanager.addBidResponse(bid_a.adUnitCode, bid_a);

      const bid_b = Object.assign({},
        bidfactory.createBid(2),
        fixtures.getBidResponses()[1]
      );
      bidmanager.addBidResponse(bid_b.adUnitCode, bid_b);

      const result = {
        hb_size: '300x250',
        hb_pb: '10.00',
        hb_adid: '233bcbee889d46d',
        hb_bidder: 'appnexus',
        hb_deal_triplelift : 'test deal'
      };

      assert.equal($$PREBID_GLOBAL$$.getAdserverTargeting()[bid_a.adUnitCode], result);
      $$PREBID_GLOBAL$$._bidsReceived = pre;
    });

[bretg]

Agreed that there is a misleading statement in the doc: "Prebid.js will generate the deal key-values for every bidder".

The "alwaysUseBid" bidderSettings option would be the way around this. Will speak with the team about whether we change the doc or the code -- community input either way welcome.

[github-nilsson]

The documentation is probably made for how the code worked before the "refactoring" in
#935

I would expect prebid without any special configuration to add hb_deal_suffix targeting for every adapter that returns a deal. If the publisher do not want it, they can override that with adserverTargeting in bidderSettings (or, more probably, talk to the partner placing bids with deals).

[github-nilsson]

In addition to the documentation mismatch on hb_deal_suffix, most of the time you would want hb_adid_suffix in addition so the deal specific creative can call renderAd directly with %%PATTERN:hb_adid_suffix%%. That's a different issue, but something to consider at the same time.

[bretg]

@mkendall07 and @mjacobsonny - if Prebid sends hb_deal_ and hb_adid_ everywhere, not clear to me what function alwaysUseBid has. If we go this way, which I like, I think 1.0 could eliminate alwaysUseBid.

[mjacobsonny]

@bretg -- Agreed. I would definitely like to get rid of alwaysUseBid in 1.0.

[mkendall07]

what does that mean exactly? You want alwaysUseBid default to true? Does this mean we should drop this flag?
I'd recommend dropping the flag completely, and setting enableSendAllBids=true as the default (in setConfig).

[mjacobsonny]

+1

Any reason we can't also get rid of enableSendAllBids and just make it true always?

[mkendall07]

Ok, so here's the behavior I'm suggesting for 1.0.

1. hb_deal_{bidder_code} will be added into the "standard keys" to be set on every bidder
2. alwaysUseBid in pbjs.bidderSettings.adserverTargeting will be removed/always be true
3. enableSendAllBids will default to true.
4. hb_adid_{bidder_code} is also needed as a standard key. It's needed to display if the deal wins.
5. hb_deal_{bidder_code} should be overridable in bidderSettings -- if it's defined there, the standard value is ignored.

@mjacobsonny @bretg for comment ^

I think there is use cases were pubs won't want to send all bids - so leaving that as an option is good.

[bretg]

This is good for me with two additions:

1. hb_adid_{bidder_code} is also needed as a standard key. It's needed to display if the deal wins.
2. hb_deal_{bidder_code} should be overridable in bidderSettings -- if it's defined there, the standard value is ignored.

[jaiminpanchal27]

@mkendall07 @bretg
Since we are removing alwaysUseBid. What should happen if some pub has following in bidderSettings

pbjs.bidderSettings = {
          rubi: {
            adserverTargeting: [{
                key: "foo",
                val: function(bidResponse) {
                    return 'bar';
                }
            }]
          },
          appnexusAst: {
            adserverTargeting: [{
                key: "foo",
                val: function(bidResponse) {
                    return 'baz';
                }
            }]
          },
        };

Google's slot.setTargeting method supports string and array both.
So when we call dfp, what targeting key values should be appended as query param ? foo=baz or foo=bar,baz ?

As of now it only sends foo=baz. Test page https://jsfiddle.net/rbfxL353/

[mkendall07]

foo=bar,baz seems correct.

[bretg]

foo=bar,baz seems correct.

Agreed.