Fixing the BidAdjustmentEvent fire time (prebid#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
Genius · Jul 17, 2018 · 7c7e22c · 7c7e22c
1 parent 6038938
commit 7c7e22c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 5 deletions.
diff --git 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
@@ -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', () => {