Appnexus adaptor - Added App parameters for hybrid apps. #2973

aneuway2 · 2018-08-13T15:51:08Z

Type of change

Feature

Description of change

Added App parameters for hybrid apps. This allows developers running Prebid in a webview with prebid to pass the app related parameters. (Note that this is not the same as Prebid Mobile)

test parameters for validating bids

{
  bidder: '<bidder name>',
  placementId: '10433394',
  app: {
    id: "B1O2W3M4AN.com.prebid.webview",
    geo: {
      lat: 40.0964439,
      lng: -75.3009142
    },
    device_id: {
      idfa: "4D12078D-3246-4DA4-AD5E-7610481E7AE", // Apple advertising identifier
      aaid: "38400000-8cf0-11bd-b23e-10b96e40000d", // Android advertising identifier
      md5udid: "5756ae9022b2ea1e47d84fead75220c8", // MD5 hash of the ANDROID_ID
      sha1udid: "4DFAA92388699AC6539885AEF1719293879985BF", // SHA1 hash of the ANDROID_ID
      windowsadid: "750c6be243f1c4b5c9912b95a5742fc5" // Windows advertising identifier
    }
  }
}

mkendall07

Thanks @aneuway2
Can you remove the debug params from this PR? Also you have some test failures you need to resolve (run gulp test to see them )

jaiminpanchal27 · 2018-08-16T14:38:28Z

modules/appnexusBidAdapter.js

+    let appDeviceObj;
+    if (appDeviceObjBid) {
+      appDeviceObj = {};
+      Object.keys(appDeviceObjBid.params.app)


This will throw error if params is undefined. Please add a check on line 66 to check that it exists.

jaiminpanchal27 · 2018-08-16T14:40:00Z

modules/appnexusBidAdapter.js

+    let appIdObj;
+    if (appIdObjBid) {
+      appIdObj = {
+        appid: appIdObjBid.params.app.id


This will throw error if params and app are undefined. To access nested property you need to add check that it exist.

jaiminpanchal27 · 2018-08-16T14:46:41Z

modules/appnexusBidAdapter.js

+            debugObj['dongle'] = debugObjParams[param];
+          }
+          if (param == 'ast_debug_member'){
+            debugObj['member_id'] = parseInt(debugObjParams[param]);


Provide a second argument 10 which specifies the radix - a base 10 number. If you do not pass this and first argument starts with 0 than it would be considered as octal.
Same of other usage of parseInt

removed from this PR, but will keep in mind for the next

jaiminpanchal27 · 2018-08-16T14:59:14Z

modules/appnexusBidAdapter.js

+  return false
+}
+
+function getURLparams() {


I think you can use utils.getWindowLocation() and url.parse instead of adding this function.

jaiminpanchal27 · 2018-08-16T14:59:53Z

modules/appnexusBidAdapter.js

@@ -381,6 +431,36 @@ function hasMemberId(bid) {
  return !!parseInt(bid.params.member, 10);
 }

+function hasAppDeviceInfo(bid) {
+  return !!bid.params.app


Add check for nested access

jaiminpanchal27 · 2018-08-16T15:00:00Z

modules/appnexusBidAdapter.js

+}
+
+function hasAppId(bid) {
+  if (!!bid.params.app) {


Add check for nested access

aneuway2 · 2018-08-16T15:53:32Z

@mkendall07 - removed debug URL params from this PR (will follow up in future PR)
@jaiminpanchal27 - added changes per your review

jsnellbaker

@aneuway2

There appear to be a number of linting errors that need to be addressed (can see if you run gulp lint).

Below is a copy of the errors for reference:

/Users/jsnellbaker/git/Prebid.js/modules/appnexusBidAdapter.js
   13:50  error  Multiple spaces found before '// appid is co...'  no-multi-spaces
  410:8   error  Redundant double negation                         no-extra-boolean-cast
  416:8   error  Redundant double negation                         no-extra-boolean-cast

/Users/jsnellbaker/git/Prebid.js/test/spec/modules/appnexusBidAdapter_spec.js
  337:19  error  Strings must use singlequote  quotes
  343:23  error  Strings must use singlequote  quotes
  344:23  error  Strings must use singlequote  quotes
  345:26  error  Strings must use singlequote  quotes
  346:27  error  Strings must use singlequote  quotes
  347:30  error  Strings must use singlequote  quotes
  357:16  error  Strings must use singlequote  quotes
  361:15  error  Strings must use singlequote  quotes
  362:15  error  Strings must use singlequote  quotes
  363:18  error  Strings must use singlequote  quotes
  364:19  error  Strings must use singlequote  quotes
  365:22  error  Strings must use singlequote  quotes

…out the prebid 1.x versus the legacy ast/jpt adaptors. Added disclaimer on

aneuway2 · 2018-08-16T18:49:13Z

Fixed linting errors @jsnellbaker

jsnellbaker · 2018-08-16T19:14:28Z

modules/appnexusBidAdapter.js

@@ -59,6 +60,23 @@ export const spec = {
        .forEach(param => userObj[param] = userObjBid.params.user[param]);
    }

+    const appDeviceObjBid = find(bidRequests, hasAppDeviceInfo);


@aneuway2 Just to confirm a use-case - do you foresee the chance that multiple adUnits for appnexus would ever have differing values for this device/geo fields?

This find grabs the first matching result and uses that for the value. If you had multiple adUnits with differing sets of values, only the first set of those values would be used - potentially skewing the request that gets generated.

I'm not sure this would happen, but I wanted to clarify just in case.

@jsnellbaker device and geo are both related to the current user, so these should remain the same between adUnits

…932) * Adding addiional info on onEvent handlers, and PBS timeout * Appnexus adaptor parameters. Added for prebid/Prebid.js#2973, sorted out the prebid 1.x versus the legacy ast/jpt adaptors. Added disclaimer on * Updating sample native adserver creative to add in click tracking * made minor changes from review. Also added video params tabel from AST to 1.x version and info about keywords to the upgrade section * Fix to typos

Added hybrid app parameters and debug url parameters

9a2ec5f

jsnellbaker requested review from jaiminpanchal27 and jsnellbaker August 13, 2018 16:11

jsnellbaker assigned jaiminpanchal27 and jsnellbaker Aug 13, 2018

jsnellbaker added the needs review label Aug 13, 2018

mkendall07 requested changes Aug 14, 2018

View reviewed changes

jaiminpanchal27 requested changes Aug 16, 2018

View reviewed changes

jaiminpanchal27 added needs update and removed needs review labels Aug 16, 2018

aneuway2 added 2 commits August 16, 2018 11:04

Fix test for app parameters. removed debugging

879d079

added defensive code per review from @jaiminpanchal27

8571aae

aneuway2 changed the title ~~Added App parameters for hybrid apps. Also added AST debug URL params~~ Appnexus adaptor - Added App parameters for hybrid apps. Aug 16, 2018

simplified if/then statements

9a20e4b

jaiminpanchal27 added LGTM and removed needs update labels Aug 16, 2018

jaiminpanchal27 approved these changes Aug 16, 2018

View reviewed changes

jsnellbaker requested changes Aug 16, 2018

View reviewed changes

fix JS lint issue

d9f0681

aneuway2 added a commit to aneuway2/prebid.github.io that referenced this pull request Aug 16, 2018

Appnexus adaptor parameters. Added for prebid/Prebid.js#2973, sorted …

8e04619

…out the prebid 1.x versus the legacy ast/jpt adaptors. Added disclaimer on

aneuway2 mentioned this pull request Aug 16, 2018

Update Appnexus Bid Parameters for app and Prebid 1.x v.s. legacy prebid/prebid.github.io#932

Merged

jsnellbaker reviewed Aug 16, 2018

View reviewed changes

jsnellbaker approved these changes Aug 16, 2018

View reviewed changes

jaiminpanchal27 assigned mkendall07 and unassigned jaiminpanchal27 and jsnellbaker Aug 20, 2018

mkendall07 approved these changes Aug 21, 2018

View reviewed changes

mkendall07 merged commit 65df3bb into prebid:master Aug 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Appnexus adaptor - Added App parameters for hybrid apps. #2973

Appnexus adaptor - Added App parameters for hybrid apps. #2973

Uh oh!

aneuway2 commented Aug 13, 2018 •

edited

Loading

Uh oh!

mkendall07 left a comment

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

aneuway2 Aug 16, 2018

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

jaiminpanchal27 Aug 16, 2018

Uh oh!

aneuway2 commented Aug 16, 2018

Uh oh!

jsnellbaker left a comment

Uh oh!

aneuway2 commented Aug 16, 2018

Uh oh!

jsnellbaker Aug 16, 2018

Uh oh!

aneuway2 Aug 16, 2018

Uh oh!

Uh oh!

Appnexus adaptor - Added App parameters for hybrid apps. #2973

Appnexus adaptor - Added App parameters for hybrid apps. #2973

Uh oh!

Conversation

aneuway2 commented Aug 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Description of change

Uh oh!

mkendall07 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aneuway2 commented Aug 16, 2018

Uh oh!

jsnellbaker left a comment

Choose a reason for hiding this comment

Uh oh!

aneuway2 commented Aug 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aneuway2 commented Aug 13, 2018 •

edited

Loading