Making targeting keys configurable #3140

pm-harshad-mane · 2018-09-28T21:01:50Z

Type of change

Feature
Refactoring (no functional changes, no api changes)

Description of change

As of now the targeting keys in Prebid.js are hard-coded, starting with "hb_" , I have made changes to make these key names configurable. So now publishers will be able to change key-names.
This will help the wrapper solutions which are build using Prebid.

- Now you can change key names - Made changes in code at auction.js and targeting.js to handle the TARGETING_KEYS object than array

this will now replace partner specific keys as well as per config

pm-harshad-mane · 2018-09-28T21:05:50Z

Hello @jsnellbaker ,
Can you help me with this PR?
There are some conflicts we need to merge.

harpere

looks fine overall, just some recommendations about making the code cleaner.

harpere · 2018-09-28T21:54:25Z

test/spec/unit/core/targeting_spec.js

    'foobar': '300x250'
  },
  'netRevenue': true,
  'currency': 'USD',
  'ttl': 300
 };
+bid2['adserverTargeting'][CONSTANTS.TARGETING_KEYS.BIDDER] = 'rubicon';
+bid2['adserverTargeting'][CONSTANTS.TARGETING_KEYS.AD_ID] = '5454545';
+bid2['adserverTargeting'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '0.25';


same comment as above recommending using "computed" properties

Done, thanks for suggestion

harpere · 2018-09-28T21:56:15Z

test/spec/unit/core/targeting_spec.js

    'foobar': '300x600'
  },
  'netRevenue': true,
  'currency': 'USD',
  'ttl': 300
 };
+bid3['adserverTargeting'][CONSTANTS.TARGETING_KEYS.BIDDER] = 'rubicon';
+bid3['adserverTargeting'][CONSTANTS.TARGETING_KEYS.AD_ID] = '48747745';
+bid3['adserverTargeting'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '0.75';


same comment as above recommending using "computed" properties

Done, thanks for the suggestion

harpere · 2018-09-28T21:56:59Z

test/spec/unit/core/targeting_spec.js

    'foobar': '300x250'
  },
  'netRevenue': true,
  'currency': 'USD',
  'ttl': 300
 };
+bid1['adserverTargeting'][CONSTANTS.TARGETING_KEYS.BIDDER] = 'rubicon';
+bid1['adserverTargeting'][CONSTANTS.TARGETING_KEYS.AD_ID] = '148018fe5e';
+bid1['adserverTargeting'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '0.53';


rather than setting these separately, I think it would be cleaner to use "computed" properties when you first define the object. For example:
'adserverTargeting': { [CONSTANTS.TARGETING_KEYS.BIDDER]: 'rubicon', ... }

Done, thanks for suggestion :)

harpere · 2018-09-28T22:01:33Z

test/spec/unit/pbjs_api_spec.js

+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.SIZE] = '728x90';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '10.00';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.AD_ID] = '24bd938435ec3fc';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.BIDDER] = 'appnexus';


cleaner to use "computed" properties.

Done, thanks for the suggestion

harpere · 2018-09-28T22:01:46Z

test/spec/unit/pbjs_api_spec.js

+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.SIZE] = '728x90';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '10.00';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.AD_ID] = '24bd938435ec3fc';
+      expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.BIDDER] = 'appnexus';


Done, thanks for the suggestion

harpere · 2018-09-28T22:02:26Z

test/spec/unit/pbjs_api_spec.js

+        expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.AD_ID] = '24bd938435ec3fc';
+        expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '10.00';
+        expected['/19968336/header-bid-tag1'][CONSTANTS.TARGETING_KEYS.SIZE] = '728x90';
+        expected['/19968336/header-bid-tag1']['foobar'] = '728x90';


Done, thanks for the suggestion

harpere · 2018-09-28T22:02:40Z

test/spec/unit/pbjs_api_spec.js

+      $$PREBID_GLOBAL$$._bidsReceived[0]['adserverTargeting'][CONSTANTS.TARGETING_KEYS.AD_ID] = '233bcbee889d46d';
+      $$PREBID_GLOBAL$$._bidsReceived[0]['adserverTargeting'][CONSTANTS.TARGETING_KEYS.PRICE_BUCKET] = '10.00';
+      $$PREBID_GLOBAL$$._bidsReceived[0]['adserverTargeting'][CONSTANTS.TARGETING_KEYS.SIZE] = '300x250';
+      $$PREBID_GLOBAL$$._bidsReceived[0]['adserverTargeting'][CONSTANTS.TARGETING_KEYS.DEAL + '_appnexusDummyName'] = '1234';


Done, thanks for the suggestion

src/targeting.js

src/native.js

harpere

changes look LGTM, but there are merge conflicts that need to be fixed/resolved.

pm-harshad-mane · 2018-10-01T19:23:14Z

Hello @harpere ,

I have resolved conflicts and eslint suggestions.
I think we are good to go.

pm-harshad-mane · 2018-10-02T16:53:41Z

Hello @harpere and @jsnellbaker ,
Am I missing anything? is something pending from my side?

jsnellbaker

@pm-harshad-mane Thanks for putting together these changes. I gave them a test try and they seem to be working. I did see something that should be reviewed (mainly syntax thing). Please take a look when you can.

jsnellbaker · 2018-10-02T16:05:09Z

test/spec/unit/pbjs_api_spec.js

@@ -720,7 +719,7 @@ describe('Unit: Prebid Module', function () {
      ajaxStub.restore();
    });

-    it('should get correct hb_pb with cpm between 0 - 5', function () {
+    it('should get correct ' + CONSTANTS.TARGETING_KEYS.PRICE_BUCKET + ' with cpm between 0 - 5', () => {


Can you change back the various () => { references to the , function() { variant? These were implemented to make the unit tests more compliant with Mocha.

Hello @jsnellbaker ,
sure, let me check.

Hello @jsnellbaker ,
I have made these changes.

jsnellbaker

LGTM

@97

* Make size mapping mediaType aware (prebid#3134) * make sizeMapping mediaTypes aware * no sizeMapping if the only mediaType is not banner * better logging for size mapping and no auction when no bids * fix size mapping tests to ignore filter results (which are only for logging) * Making targeting keys configurable (prebid#3140) * moved NATIVE_KEYS to constants * Changed TARGETING_KEYS from array to an array - Now you can change key names - Made changes in code at auction.js and targeting.js to handle the TARGETING_KEYS object than array * fixed aauctionmanager test cases for dynamic targeting key names * fixed native test cases for dynamic targeting key names * fixed test cases of targeting for dynamic targeting key names * in-dev changes * replace old keys with new keys * fixed a typo * fixed a unit case in utils * updated convertTargetingsFromOldToNew in fixtures this will now replace partner specific keys as well as per config * removed a log * keep test case as it was * improved convertTargetingsFromOldToNew in fixtures * Capitalized key names in TARGETING_KEYS object; not values in object * using computed properties * using computed properties * fixed eslint suggestion * fixed eslint suggestions * keeping unit tests more compliant with Mocha * 4235 prebid endpoint return empty array instead of 204 when no bids returned (prebid#3136) * corrected user sync type * add alias pxyz add version to endpoint add validation for 204 bid response * add validation with serverResponse is undefined fix test for 204 (no bid response) * remove package lock * restore package-lock * update gulp 4 notes on README (prebid#3154) * Add teads bidder adapter (prebid#3135) * Add teads bidder adapter * Remove bidder code & tests arrow functions * Added the option to pass a deal id instead of a partnership id (prebid#3148) * increment Prebid version * Prebid 1.26.0 release * increment prebid version * implement find polyfill in unit test (prebid#3156) * InvibesBidAdapter - gdpr support (prebid#3151) * removed `cookieSyncDelay` from `config` since we have more configuration options now with the `userSync` object (prebid#3142) * Livewrapped bid and analytics adapter (prebid#3157) * Livewrapped bid and analytics adapter * Fixed some tests for browser compatibility * Fixed some tests for browser compatibility * Changed analytics adapter code name * Fix double quote in debug message * Adding appnexus debug via cookie/params (prebid#3152) * adding appnexus debug via cookie/params * removing nested object * added documentation link and removed useless conditional * fix lint error on documentation message * Add RSA validation to Criteo FastBid (prebid#3110) * Pass Prebid version to Criteo direct bidder * Update Criteo profile IDs * Add RSA verification to Criteo FastBid * Update package-lock with jsencrypt and crypto-js * Replacing all arrow functions in Mocha function calls * Update Adapter Version to 14 * Added support for user syncing pixel (prebid#3092) * Added Polymorph adapter * Cleaned up code * Updated var to let * Updated with mediaType * Updated tests * Fixed tests * Updated polymorph adapter to support cookie syncing and network key integration * Fixed parens for lint * Fixed a bug related to network_key approach * Fixed double negation warning * Updated tests for network_key * Added bidId as cache buster and updated tests * Fixed tests * Small bugfix and cleanup for Prebid Server OpenRTB code (prebid#3113) * change bid map to bid_id map and clean up openrtb in pbs * add src to bids in bid request when known * Revert "Adding appnexus debug via cookie/params" (prebid#3164) * Revert "Small bugfix and cleanup for Prebid Server OpenRTB code (prebid#3113)" This reverts commit a1f07e9. * Revert "Added support for user syncing pixel (prebid#3092)" This reverts commit 8fdf61d. * Revert "Add RSA validation to Criteo FastBid (prebid#3110)" This reverts commit e72e2dc. * Revert "Adding appnexus debug via cookie/params (prebid#3152)" This reverts commit 4797ea2. * add bid ttl to cache call (prebid#3163) * Render outstream safeframe (prebid#3159) * render outstream safeframe * move code to Renderer module * some more logic to move * Prebid 1.27.0 Release * increment Prebid version * rubiconBidAdapter - Checking FPD values are defined before toString() (prebid#3165) * rubiconBidAdapter - Checking FPD values are defined before toString() - Added two tests for this behavior * Removing unused variable * changed to compare against null added some null params to the tests to verify * Adding mediaType param to parseSizes in order to ALWAYS get the correct parse Size method correct. (prebid#3166) * PubMatic to support DigiTrust Id passing (prebid#3160) * in-dev changes * included config * Unit test cases for DigitrustId passing in PubMatic bid adapter * eslint fixes * removed a comment * replaced "() => {" with "() => {" * OpenX Adapter: Added support for pubcid (prebid#3158) * use version replace rather than package.json in widespace (prebid#3143) * Trafficroots Resubmit (prebid#3141) circleci is still failing, but I cannot find a workaround for this PR at the moment * Rubicon skip video request in mutlti format when video is not setup (prebid#3167) * Update rubiconBidAdapter.js * OpenXOutstream Bid Adapter (prebid#3153) * adds openxoutstreamadapter files * cleans up url, adds bidder config and version, removes unused code * updates template ad response * remove final unused custom param code * add openrtb to list of params again. * add payload back so we can read bidId * extra checks for cpm (pub_rev) * adds unit tests * adds unit tests * update md page * undo openx adapter space changes from autosave * undo openx adapter space changes from autosave * test cleanup * test cleanup * update md file * update example page params * remove sneaky console.log * return false * adds yieldmo to the md. remove useless docEl assignment * remove useless docEl assignments * move crid and adid to constants * fix test * remove another useless variable assignment caught by LGTM * changes CR_ID. moves tdoc to within if scope * updates crid on test * use more constants and use crid for lfid * remove superfluous trailing argument * adds viewport meta tag to header. removes unnecessary string interpolation. * move ad_id to a string * undo changes for pacakge-lock * fixes lfid to lfId, pID to pId * fixes lfid to lfId, pID to pId * remove rti:1 * adds openx maintainer email * improves additional data passed to the handler in AuctionInit and AuctionEnd events (prebid#3168) * removing cookieSet (prebid#3175) * removing cookieSet * removing blank line * not including src/cookie anymore * Removed deprecated priceType option (+tests) (prebid#3170) * Improve Digital adapter: set dealID based on buying type (prebid#3182) * Adding GDPR support * Always drop user syncs when available * Set dealID based on buying type * disabling tests that are failing in safari (prebid#3186) * Prebid 1.28.0 Release * Increment pre version * RVR-1124 Setup initial skeleton analytics adapter that can send something. Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Formatted auction/events data to fit needed schema. * RVR-1135 fetched device data. * Applied feedback. * Applied feedback. * Fetched core. * Added click handler for reporting banners click events. * Applied analyzer for reporting displayed impressions. * Applied feedback. * Merged in RVR-1214-invoke-handlers-on-rendering (pull request #7) RVR-1214 Invoke handlers on rendering * RVR-1214 Invoked handlers right after ad is displayed. * Applied feedback. Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Merged in RVR-1192-configuration-global-parameters (pull request #8) RVR-1192 Configuration/Global parameters Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Merged in RVR-1181-Prebid-js-unit-tests-setup (pull request #6) RVR-1181 Prebid.js Unit tests setup Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Merged in RVR-1247-additional-data-to-impression-records (pull request #9) RVR-1247 Additional data to impression records Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Merged in RVR-1249-add-requestedbids-to-auction (pull request #10) RVR-1249 Add requested bids to auction object request. Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Merged in RVR-1261-fix-tests (pull request prebid#11) RVR-1261 fix tests * RVR-1261 Secured adapter from no containers configuration. And changed fetching URL. * RVR-1261 Added event check for request and changed some names. * Applied feedback. Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * RVR-1352 analytics adapter bugs Approved-by: Alessandro Di Giovanni <alessandro.digiovanni@gamegenetics.de> * Fixed bug with geolocation notification. * fixed missing bracket. * one more fix. * RVR-1357 Different optimisation responses & tracking into auction event * RVR-1852 - Add content type and hardcoded auth headers (cherry picked from commit 4def881) * RVR-1852 - Change tracker host * RVR-1852 - Override content type instead of adding header * districtmDMX new adapter (prebid#2765) * adding DMX test @97%, two files added one updated * Update districtm_spec.js * Update districtmDMX.js * adding all districtm needed file * remove legacy file * remove typo || 0 in the test method * force default to return a valid width and height * update unit test code for failing test * changed class for an object * remove package-lock.json * upgrade to gulp 4 (prebid#2930) * upgrade to gulp 4 * update circleci config * removed some tasks and added notest flag * update lint dependency for test task * RVR-1914 Consistent data types in events Also removes undefined and null properties in audience events * Merged in RVR-1883-Add-Basic-Access-Authentication (pull request prebid#17) RVR-1883 Add Basic Access Authentication * RVR-1914 - Rename functions * RVR-1914 - Set default total_duration to null in bid response * RVR-1883 - Use RIVR_CLIENT_AUTH_TOKEN global variable for Auth token * RVR-1883 - Restore stub after every test not just at the end * RVR-1883 - Remove commented code * Increase code coverage * Fix for IE 11.0.0 and Safari 8.0.8 - includes() Use core-js includes function for array

* moved NATIVE_KEYS to constants * Changed TARGETING_KEYS from array to an array - Now you can change key names - Made changes in code at auction.js and targeting.js to handle the TARGETING_KEYS object than array * fixed aauctionmanager test cases for dynamic targeting key names * fixed native test cases for dynamic targeting key names * fixed test cases of targeting for dynamic targeting key names * in-dev changes * replace old keys with new keys * fixed a typo * fixed a unit case in utils * updated convertTargetingsFromOldToNew in fixtures this will now replace partner specific keys as well as per config * removed a log * keep test case as it was * improved convertTargetingsFromOldToNew in fixtures * Capitalized key names in TARGETING_KEYS object; not values in object * using computed properties * using computed properties * fixed eslint suggestion * fixed eslint suggestions * keeping unit tests more compliant with Mocha

pm-harshad-mane added 14 commits September 24, 2018 23:15

moved NATIVE_KEYS to constants

32a1f91

Changed TARGETING_KEYS from array to an array

97c27e1

- Now you can change key names - Made changes in code at auction.js and targeting.js to handle the TARGETING_KEYS object than array

fixed aauctionmanager test cases for dynamic targeting key names

95bfcf6

fixed native test cases for dynamic targeting key names

b2087b8

fixed test cases of targeting for dynamic targeting key names

54be63d

in-dev changes

9dc1d08

replace old keys with new keys

f17f7f6

fixed a typo

c3c65a6

fixed a unit case in utils

730df24

updated convertTargetingsFromOldToNew in fixtures

1ca800c

this will now replace partner specific keys as well as per config

removed a log

aae80e6

keep test case as it was

74965af

improved convertTargetingsFromOldToNew in fixtures

047cf30

Capitalized key names in TARGETING_KEYS object; not values in object

7e86bd3

harpere requested changes Sep 28, 2018

View reviewed changes

harpere added needs review needs update needs 2nd review Core module updates require two approvals from the core team labels Sep 28, 2018

harpere assigned jsnellbaker Sep 28, 2018

pm-harshad-mane added 2 commits September 28, 2018 19:27

using computed properties

20008f2

using computed properties

5ff6c25

harpere approved these changes Oct 1, 2018

View reviewed changes

harpere requested changes Oct 1, 2018

View reviewed changes

pm-harshad-mane added 3 commits October 1, 2018 10:58

Merge branch 'master' into configurable_key_names

fd0ec4c

fixed eslint suggestion

fa16c52

fixed eslint suggestions

f66b209

jsnellbaker requested changes Oct 2, 2018

View reviewed changes

keeping unit tests more compliant with Mocha

27f36d6

jsnellbaker approved these changes Oct 2, 2018

View reviewed changes

jsnellbaker assigned harpere and unassigned jsnellbaker Oct 2, 2018

jsnellbaker removed the needs review label Oct 2, 2018

harpere approved these changes Oct 2, 2018

View reviewed changes

harpere added LGTM and removed needs 2nd review Core module updates require two approvals from the core team needs update labels Oct 2, 2018

harpere merged commit 4e8955e into prebid:master Oct 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making targeting keys configurable #3140

Making targeting keys configurable #3140

pm-harshad-mane commented Sep 28, 2018

pm-harshad-mane commented Sep 28, 2018

harpere left a comment

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere Sep 28, 2018

pm-harshad-mane Sep 29, 2018

harpere left a comment

pm-harshad-mane commented Oct 1, 2018

pm-harshad-mane commented Oct 2, 2018

jsnellbaker left a comment

jsnellbaker Oct 2, 2018

pm-harshad-mane Oct 2, 2018

pm-harshad-mane Oct 2, 2018

jsnellbaker left a comment

Making targeting keys configurable #3140

Making targeting keys configurable #3140

Conversation

pm-harshad-mane commented Sep 28, 2018

Type of change

Description of change

pm-harshad-mane commented Sep 28, 2018

harpere 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

harpere left a comment

Choose a reason for hiding this comment

pm-harshad-mane commented Oct 1, 2018

pm-harshad-mane commented Oct 2, 2018

jsnellbaker 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

jsnellbaker left a comment

Choose a reason for hiding this comment