#2640 all bids in console #3022

jrosendahl · 2018-08-24T20:56:27Z

Type of change

Bugfix
Feature
New bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other

Description of change

Addressing #2640
Made changes so that all bids are shown in console when pbjs.getBidResponses() is called.

A link to a PR on the docs repo at https://github.com/prebid/prebid.github.io/

Other information

#2640

send 'iv' param if present and full url in `site.page`

Enhance location detection within utils

CR for enhanced-location-detection PR

Reorg gettopwindowlocation util & add tests.

set creativeId to crid if present in response

mike-chowla

I need to think about the effects of this change more but there's a bunch of items that need cleaned up.

mike-chowla · 2018-08-30T18:04:31Z

src/adapters/bidderFactory.js

@@ -1,15 +1,30 @@
 import Adapter from 'src/adapter';
 import adaptermanager from 'src/adaptermanager';
-import { config } from 'src/config';
+import {


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:04:36Z

src/adapters/bidderFactory.js

@@ -1,15 +1,30 @@
 import Adapter from 'src/adapter';
 import adaptermanager from 'src/adaptermanager';
-import { config } from 'src/config';
+import {
+  config


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:04:43Z

src/adapters/bidderFactory.js

+import {
+  userSync
+} from 'src/userSync';
+import {


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:04:53Z

src/adapters/bidderFactory.js

 import CONSTANTS from 'src/constants.json';
 import events from 'src/events';
 import includes from 'core-js/library/fn/array/includes';

-import { logWarn, logError, parseQueryStringParameters, delayExecution, parseSizesInput, getBidderRequest } from 'src/utils';
+import {


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:04:58Z

src/adapters/bidderFactory.js

+  logWarn,
+  logError,
+  parseQueryStringParameters,
+  delayExecution,


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:06:10Z

src/adapters/bidderFactory.js

+        // bidderRequest.end = timestamp();
+        let bidIds = bidsArray.map(bid => {
+          if (bid.requestId) {
+            return bid.requestId.toString();


Why are you converting bid ids to strings? Seems inefficient and unecessary

Existing unit tests do not consistently use strings or numeric values for id's. I was unsure if this was an issue with the tests or in place to make sure ids can be provided either as a string or int. In any case this is safer.

I could write this using a double equal, but then the cast would be implicit instead of explicit.

mike-chowla · 2018-08-30T18:06:30Z

src/adapters/bidderFactory.js

+
+        let missingBids = bidderRequest.bids.filter(bid => {
+          if (bid.bidId) {
+            return bidIds.indexOf(bid.bidId.toString()) === -1;


Why are you converting bid ids to strings? Seems inefficient and unnecessary

mike-chowla · 2018-08-30T18:07:14Z

src/adapters/bidderFactory.js

@@ -251,8 +294,7 @@ export function newBidder(spec) {
            break;
          case 'POST':
            ajax(
-              request.url,
-              {
+              request.url, {


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:07:20Z

src/adapters/bidderFactory.js

@@ -370,7 +412,7 @@ function validBidSize(adUnitCode, bid, bidRequests) {
  // if a banner impression has one valid size, we assign that size to any bid
  // response that does not explicitly set width or height
  if (parsedSizes.length === 1) {
-    const [ width, height ] = parsedSizes[0].split('x');
+    const [width, height] = parsedSizes[0].split('x');


Please revert all non-substantive formatting changes

mike-chowla · 2018-08-30T18:12:38Z

src/adapters/bidderFactory.js

@@ -210,9 +249,14 @@ export function newBidder(spec) {
        }
      });

+      if (validBidRequests.length === 0) {


I don't understand the logic why we only want to add no bids when there are no valid bids. What's the thinking here?

So if there is a bidderRequest.bids, but none of them are valid, this logs those bids as failed bids. This is consistent with what would currently happen if a subset of the bids are invalid.

If we want to only have no bids for valid requests we will have to use validBidRequests instead of bidderRequest.bids when looking for missing bids to maintain consistency.

Let me know what your preference is. I am fine with it either way.

src/adapters/bidderFactory.js

jaiminpanchal27 · 2018-09-12T19:28:12Z

src/adapters/bidderFactory.js

+        missingBids.forEach(bid => {
+          const bidRequest = bidRequestMap[bid.bidId];
+          bid.requestId = bid.bidId;
+          let prebidNoBid = Object.assign(bidfactory.createBid(CONSTANTS.STATUS.NO_BID, bidRequest), bid);


With Prebid 1.0 we are not adding NO_BID to auction.
To close aucton in legacy we used to match bidsRequested and bidsReceived and hence were adding a NO_BID.

Not needed here.

This is one of the main pieces to this ticket, so is definitely needed here. It’s adding back in “no bids” for all the missing analytics/data caused by the oversight of removing them in 1.*

mkendall07 · 2018-09-18T18:04:54Z

Since this is a API change it needs to go into a 2.x release.

mike-chowla · 2018-09-20T18:11:46Z

Given the feedback, I'm thinking the only way to resolve the debug snippet not showing zero bids in Prebid 1.x is to modify the debug snippet rather the change the core

andyblackwell · 2018-09-21T04:04:35Z

To be honest it’s not just this “debug” call, the removal of these no bids, and even the introduced ajax timeouts (separate topic) in 1.* has resulted in quite a loss of important analytics data. I’ve personally already patched these issues, it was a roadblock for us on the upgrade and we were not ok with losing that data.

Not sure what you have in mind with the debug snippet, but without changing core I’m not really sure it will cover all the cases though?

stale · 2018-10-05T04:08:43Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2018-10-25T22:59:34Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Rachel Joyce and others added 30 commits February 7, 2018 17:04

fix sovrn dealid

c0c28d7

Merge branch 'master' of https://github.com/prebid/Prebid.js

d7df108

Merge branch 'master' of https://github.com/prebid/Prebid.js

0700e91

send 'iv' param if present

cf0ca21

page field in site object sends full url

f89135c

Merge pull request #1 from sovrn/HS-303_Send_iv_param

be091fd

send 'iv' param if present and full url in `site.page`

Merge remote-tracking branch 'upstream/master'

af86376

Enhance location detection within util function.

c2f2350

Merge pull request #3 from sovrn/enhanced-location-detection

4867c53

Enhance location detection within utils

fix func call

28f3e0e

fix tagid var type

497723b

add test for converting tagid to string

f90b2e0

Merge branch 'master' of https://github.com/prebid/Prebid.js

2e9e413

merge upstream master

74ed7f9

CR

1269a33

Merge pull request #4 from sovrn/enhanced-location-detection

55284d5

CR for enhanced-location-detection PR

Reorg gettopwindowlocation util & add tests.

3c2f8bc

Merge branch 'master' into enhanced-location-detection

fee1789

Merge pull request #5 from sovrn/enhanced-location-detection

cd78968

Reorg gettopwindowlocation util & add tests.

lint fixes

68a8afa

Merge remote-tracking branch 'upstream/master'

35b3f4c

search param is string in window location objects

b2bc2a4

Merge branch 'master' into master

7da5620

Merge branch 'master' of https://github.com/prebid/Prebid.js

1f5f2a3

set creativeId to crid if present in response

6763f3a

Merge pull request #7 from sovrn/HS-368_Add_crid_if_present

ad9e191

set creativeId to crid if present in response

Merge remote-tracking branch 'upstream/master'

f83afca

Merge branch 'master' of https://github.com/prebid/Prebid.js

de65514

send gdpr info

bbcd0b9

Merge branch 'master' of https://github.com/prebid/Prebid.js

0d80a5e

John Rosendahl added 2 commits August 23, 2018 15:25

check for bid and request is existing before calling toString

de3a97f

type

4c276e2

snapwich requested a review from mike-chowla August 27, 2018 17:28

snapwich assigned snapwich and mike-chowla and unassigned snapwich Aug 27, 2018

mike-chowla requested changes Aug 30, 2018

View reviewed changes

mike-chowla mentioned this pull request Aug 30, 2018

New prebid version doesn't show all bids in the console #2640

Closed

John Rosendahl added 2 commits August 30, 2018 15:02

updated to remove fomrating changes

9be8e35

all formating changes removed

22afd4a

andyblackwell suggested changes Sep 4, 2018

View reviewed changes

src/adapters/bidderFactory.js Outdated Show resolved Hide resolved

src/adapters/bidderFactory.js Outdated Show resolved Hide resolved

John Rosendahl added 4 commits September 5, 2018 10:31

added bid accumulator

cf7f192

added timestamp

0745c24

Merged with upstream master

7d2f986

removed formating change

ed3b51d

jaiminpanchal27 requested changes Sep 12, 2018

View reviewed changes

mkendall07 added the API Impact label Sep 18, 2018

stale bot added the stale label Oct 5, 2018

bwoolcott added needs review and removed stale labels Oct 11, 2018

bwoolcott mentioned this pull request Oct 11, 2018

Debug log don't return all bids #2393

Closed

stale bot added the stale label Oct 25, 2018

stale bot closed this Nov 1, 2018

snapwich mentioned this pull request Nov 9, 2018

new NO_BID event and "no bids" available from auction #3286

Merged

2 tasks

s-nikulin-lineate deleted the HB-2_2640_AllBidsInConsole branch June 20, 2022 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2640 all bids in console #3022

#2640 all bids in console #3022

jrosendahl commented Aug 24, 2018

mike-chowla left a comment

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

jrosendahl Aug 30, 2018

mike-chowla Aug 30, 2018

jrosendahl Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

mike-chowla Aug 30, 2018

jrosendahl Aug 30, 2018

jaiminpanchal27 Sep 12, 2018

andyblackwell Sep 13, 2018

mkendall07 commented Sep 18, 2018

mike-chowla commented Sep 20, 2018

andyblackwell commented Sep 21, 2018 •

edited

Loading

stale bot commented Oct 5, 2018

stale bot commented Oct 25, 2018

#2640 all bids in console #3022

#2640 all bids in console #3022

Conversation

jrosendahl commented Aug 24, 2018

Type of change

Description of change

Other information

mike-chowla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkendall07 commented Sep 18, 2018

mike-chowla commented Sep 20, 2018

andyblackwell commented Sep 21, 2018 • edited Loading

stale bot commented Oct 5, 2018

stale bot commented Oct 25, 2018

andyblackwell commented Sep 21, 2018 •

edited

Loading