Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adxcg adapter #1554

Merged
merged 2 commits into from
Sep 5, 2017
Merged

Adxcg adapter #1554

merged 2 commits into from
Sep 5, 2017

Conversation

adxcgcom
Copy link
Contributor

@adxcgcom adxcgcom commented Sep 1, 2017

Type of change

  • [x ] New bidder adapter

Description of change

Adding adxcg adapter and its tests

  • test parameters for validating bids
{
  bidder: 'adxcg',
  params: {
    adzoneid: '1'
  }
}
  • test parameters for validating video bids
{
  bidder: 'adxcg',
  params: {
    adzoneid: '20'
  }
}
  • test parameters for validating native bids
{
  bidder: 'adxcg',
  params: {
    adzoneid: '2379'
  }
}

@adxcgcom adxcgcom changed the title added adxcg adapter and associated tests Adxcg adapter Sep 1, 2017
@jaiminpanchal27 jaiminpanchal27 self-assigned this Sep 1, 2017
@jaiminpanchal27 jaiminpanchal27 self-requested a review September 1, 2017 12:59
jaiminpanchal27
jaiminpanchal27 requested changes Sep 1, 2017
View reviewed changes
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adxcgcom Thanks for submitting adapter. Left some comments.

modules/adxcgBidAdapter.js Outdated
});
};

function removeUrlParameters(oldURL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can be removed and you can use functions from url.js

modules/adxcgBidAdapter.js Outdated
sizes.push(parsedSizes);
});

let secure = window.location.protocol == 'https:';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils.getTopWindowLocation() get location object and also handles cross domain environments. Would recommend using that one.

modules/adxcgBidAdapter.js Outdated
return newURL;
}

function parseWidthAndHeight(bid) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if utils.parseSizesInput() fits your needs. If it does you can remove this function too.

modules/adxcgBidAdapter.js Outdated
bid.width = adxcgBidReponse.width;
bid.height = adxcgBidReponse.height;

utils.logMessage('submitting bid[' + bidRequest.placementCode + ']:' + JSON.stringify(bid));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals at all places.

Removed unnecessary string concatenations and similar utilitiy functi…
52a00a6 
…ons and replaced them with those from prebid utils.js and url.js.

Enabled credentials in the ajax call.
Replaced string concatenations with template literals where possible.
Escaped native click url in order to correctly handle redirects.
@adxcgcom
Copy link
Contributor Author

adxcgcom commented Sep 4, 2017

Thank you for the review, all issues should be addressed in the latest commit.

@jaiminpanchal27
Copy link
Collaborator

LGTM

mkendall07
mkendall07 reviewed Sep 5, 2017
View reviewed changes
modules/adxcgBidAdapter.js
};

adaptermanager.registerBidAdapter(new AdxcgAdapter(), 'adxcg', {
supportedMediaTypes: ['video', 'native']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@jaiminpanchal27 jaiminpanchal27 merged commit 34fbe9e into prebid:master Sep 5, 2017
philipwatson pushed a commit to mbrtargeting/Prebid.js that referenced this pull request Sep 18, 2017
@adxcgcom @philipwatson
Adxcg adapter (prebid#1554)
a7335be 
* added adxcg adapter and associated tests

* Removed unnecessary string concatenations and similar utilitiy functions and replaced them with those from prebid utils.js and url.js.
Enabled credentials in the ajax call.
Replaced string concatenations with template literals where possible.
Escaped native click url in order to correctly handle redirects.
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Sep 18, 2017
Merge tag '0.28.0' of https://github.com/prebid/Prebid.js into prebuilt
da960cc 
* tag '0.28.0' of https://github.com/prebid/Prebid.js: (27 commits)
  Prebid 0.28.0 Release
  Revert "Upgrade sinon to 3.x (prebid#1491)" (prebid#1563)
  add () for correct order of operations in scaling increments for currency (prebid#1559)
  AppnexusAst adapter update: Added source and version to request payload (prebid#1555)
  remove unnecessary spread operator (prebid#1561)
  Adxcg adapter (prebid#1554)
  Upgrade sinon to 3.x (prebid#1491)
  Rename vastPayload to vastXml (prebid#1556)
  Single-size sizes array now can be taken, too (prebid#1535)
  Updated the istanbul-instrumenter-loader (prebid#1550)
  Add AerServ Adapter (prebid#1538)
  Fixed imports and made adform support aliasing (prebid#1518)
  Custom granularity fix (prebid#1546)
  Fix `documentation lint` issues (prebid#1544)
  Yieldbot adunit bidder params slot name usage fix (prebid#1394)
  Update serverbid adapter to use smartsync (prebid#1324)
  Add improvedigitalBidAdapter (prebid#1381)
  Fix prebid#1533 spring server typo (prebid#1542)
  userSync is off by default (prebid#1543)
  currency module (prebid#1374)
  ...
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017
@adxcgcom @jbAdyoulike
Adxcg adapter (prebid#1554)
577bfb1 
* added adxcg adapter and associated tests

* Removed unnecessary string concatenations and similar utilitiy functions and replaced them with those from prebid utils.js and url.js.
Enabled credentials in the ajax call.
Replaced string concatenations with template literals where possible.
Escaped native click url in order to correctly handle redirects.
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request Oct 12, 2017
@vzhukovsky
Merge pull request prebid#121 in AOLP_ADS_JS/prebid.js from release/1…
e797d03 
….28.0 to aolgithub-master

* commit '4d9ade3df767750743f8888ed9efd6c77f8d0050': (26 commits)
  Added changelog entry.
  Added new aol partners ids.
  Prebid 0.28.0 Release
  Revert "Upgrade sinon to 3.x (prebid#1491)" (prebid#1563)
  add () for correct order of operations in scaling increments for currency (prebid#1559)
  AppnexusAst adapter update: Added source and version to request payload (prebid#1555)
  remove unnecessary spread operator (prebid#1561)
  Adxcg adapter (prebid#1554)
  Upgrade sinon to 3.x (prebid#1491)
  Rename vastPayload to vastXml (prebid#1556)
  Single-size sizes array now can be taken, too (prebid#1535)
  Updated the istanbul-instrumenter-loader (prebid#1550)
  Add AerServ Adapter (prebid#1538)
  Fixed imports and made adform support aliasing (prebid#1518)
  Custom granularity fix (prebid#1546)
  Fix `documentation lint` issues (prebid#1544)
  Yieldbot adunit bidder params slot name usage fix (prebid#1394)
  Update serverbid adapter to use smartsync (prebid#1324)
  Add improvedigitalBidAdapter (prebid#1381)
  Fix prebid#1533 spring server typo (prebid#1542)
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@adxcgcom @dluxemburg
Adxcg adapter (prebid#1554)
dbe6f1f 
* added adxcg adapter and associated tests

* Removed unnecessary string concatenations and similar utilitiy functions and replaced them with those from prebid utils.js and url.js.
Enabled credentials in the ajax call.
Replaced string concatenations with template literals where possible.
Escaped native click url in order to correctly handle redirects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaiminpanchal27 jaiminpanchal27 jaiminpanchal27 approved these changes

@mkendall07 mkendall07 mkendall07 approved these changes

Assignees

@jaiminpanchal27 jaiminpanchal27

Labels
LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adxcgcom @jaiminpanchal27 @mkendall07