From 7c7e22cd21cbdea2c6e9a42ff52858084aa39121 Mon Sep 17 00:00:00 2001
From: dbemiller <dbemiller@appnexus.com>
Date: Fri, 21 Jul 2017 11:34:45 -0400
Subject: [PATCH] Fixing the BidAdjustmentEvent fire time (#1399)

* Fired the bid adjustment event at (what I think is) the appropriate time.

* More thorough tests.

* Better test description.

* Removed a stray console.log
---
 src/bidmanager.js                 | 11 ++++++-----
 test/spec/unit/bidmanager_spec.js | 29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/bidmanager.js b/src/bidmanager.js
index cd910c54524e..ff32aa3e9055 100644
--- a/src/bidmanager.js
+++ b/src/bidmanager.js
@@ -130,11 +130,6 @@ exports.addBidResponse = function (adUnitCode, bid) {
   // Postprocess the bids so that all the universal properties exist, no matter which bidder they came from.
   // This should be called before addBidToAuction().
   function prepareBidForAuction() {
-    // Let listeners know that now is the time to adjust the bid, if they want to.
-    //
-    // This must be fired first, so that we calculate derived values from the updates
-    events.emit(CONSTANTS.EVENTS.BID_ADJUSTMENT, bid);
-
     const bidRequest = getBidderRequest(bid.bidderCode, adUnitCode);
 
     Object.assign(bid, {
@@ -148,6 +143,12 @@ exports.addBidResponse = function (adUnitCode, bid) {
 
     bid.timeToRespond = bid.responseTimestamp - bid.requestTimestamp;
 
+    // Let listeners know that now is the time to adjust the bid, if they want to.
+    //
+    // CAREFUL: Publishers rely on certain bid properties to be available (like cpm),
+    // but others to not be set yet (like priceStrings). See #1372 and #1389.
+    events.emit(CONSTANTS.EVENTS.BID_ADJUSTMENT, bid);
+
     // a publisher-defined renderer can be used to render bids
     const adUnitRenderer =
       bidRequest.bids && bidRequest.bids[0] && bidRequest.bids[0].renderer;
diff --git a/test/spec/unit/bidmanager_spec.js b/test/spec/unit/bidmanager_spec.js
index d5b8c77c167b..c55a2ae5a2f7 100644
--- a/test/spec/unit/bidmanager_spec.js
+++ b/test/spec/unit/bidmanager_spec.js
@@ -70,6 +70,21 @@ describe('The Bid Manager', () => {
      */
     function prepAuction(adUnits, bidRequestTweaker) {
       function bidAdjuster(bid) {
+        if (bid.hasOwnProperty('cpm')) {
+          bid.hadCpmDuringBidAdjustment = true;
+        }
+        if (bid.hasOwnProperty('adUnitCode')) {
+          bid.hadAdUnitCodeDuringBidAdjustment = true;
+        }
+        if (bid.hasOwnProperty('timeToRespond')) {
+          bid.hadTimeToRespondDuringBidAdjustment = true;
+        }
+        if (bid.hasOwnProperty('requestTimestamp')) {
+          bid.hadRequestTimestampDuringBidAdjustment = true;
+        }
+        if (bid.hasOwnProperty('responseTimestamp')) {
+          bid.hadResponseTimestampDuringBidAdjustment = true;
+        }
         bid.cpm = adjustCpm(bid.cpm);
       }
       beforeEach(() => {
@@ -142,6 +157,20 @@ describe('The Bid Manager', () => {
           bidManager.addBidResponse('mock/code');
           expect($$PREBID_GLOBAL$$._bidsReceived.length).to.equal(0);
         });
+
+        it('should attach properties for analytics *before* the BID_ADJUSTMENT event listeners are called', () => {
+          const copy = Object.assign({}, bidResponse);
+          copy.getSize = function() {
+            return `${this.height}x${this.width}`;
+          };
+          delete copy.cpm;
+          bidManager.addBidResponse('mock/code', copy);
+          expect(copy).to.have.property('hadCpmDuringBidAdjustment', true);
+          expect(copy).to.have.property('hadAdUnitCodeDuringBidAdjustment', true);
+          expect(copy).to.have.property('hadTimeToRespondDuringBidAdjustment', true);
+          expect(copy).to.have.property('hadRequestTimestampDuringBidAdjustment', true);
+          expect(copy).to.have.property('hadResponseTimestampDuringBidAdjustment', true);
+        });
       });
 
       describe('when the auction has timed out', () => {