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

AdSupply adapter #1162

Merged
merged 5 commits into from
May 2, 2017
Merged

AdSupply adapter #1162

merged 5 commits into from
May 2, 2017

Conversation

msadsupply
Copy link
Contributor

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
  • Other

Description of change

  • test parameters for validating bids
{
  bidder: 'adsupply',
  params: {
    clientId: 'b367CB268B1094004A3689751E7AC568F',
    siteId: '9e3ba26d-eaac-4004-9c7d-c0ac269d0cf2',
    zoneId: 28384,
    endpointUrl: 'engine.4dsply.com'
 }
}

Other information

matthewlane
matthewlane reviewed May 1, 2017
View reviewed changes
Copy link
Collaborator

@matthewlane matthewlane left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tests pass and was able to verify bid responses with the given params. We have an upcoming change to the linting system which will catch a few errors here, I've noted them below for your review. Thanks for the PR

test/spec/adapters/adsupply_spec.js Outdated
describe('adsupply adapter tests', function () {

const expect = require('chai').expect;
const assert = require('chai').assert;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't used, can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const assert = require('chai').assert; - REMOVED

const expect = require('chai').expect; - USED in the first test

src/adapters/adsupply.js Outdated
return false;
}

if (typeof params.zoneId !== "number" || params.zoneId <= 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Upcoming ESLint error: prefer single quotes around number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/adapters/adsupply.js Outdated

if (!media) return;

if (!media.Url || !media.Ecpm || typeof media.Ecpm !== "number" || media.Ecpm <= 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also prefer single quotes around number here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/adapters/adsupply.js
};
};

module.exports = AdSupplyAdapter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ESLint error: newline required at end of file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new line at the end. "Files changed" tab doesn't show it for some reason but if you try to edit the file with online editor you will see there's an empty line at the end of the file. Hope it helps to fix the ESLint error.

@protonate protonate added this to the Prebid 0.23.0 milestone May 2, 2017
@jaiminpanchal27 jaiminpanchal27 merged commit 4be50ea into prebid:master May 2, 2017
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request May 4, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
482c0ed 
…built

* 'master' of https://github.com/prebid/Prebid.js: (21 commits)
  add lodash as dependency (prebid#1174)
  fix size mapping for s2s (prebid#1175)
  Improve footer styling (prebid#1171)
  Bugfix: internal bids requested overwritten (prebid#1173)
  pre-release version bump
  Prebid 0.23.0 Release
  Yieldbot adapter - multiple requestBids per pageview (prebid#1146)
  Widespace adapter validate size fix (prebid#1140)
  Audience Network: bid when at least one valid slot size (prebid#1148)
  Quantcast adaptor (prebid#1063)
  AOL Adapter - ONE Mobile endpoint implemented. (prebid#1115)
  Prebid Server to Server (prebid#1165)
  Pubgears Header Bidding Adapter (prebid#953)
  remove old adloader#trackPixel (prebid#1159)
  added audit beacon to detect misuse of this bidder.  Detects auctions… (prebid#1134)
  Bidfluence CDN endpoint URL update (prebid#1163)
  AdSupply adapter (prebid#1162)
  Sonobi Adapter - Enable size overrides (prebid#1141)
  Added an editorconfig file to match jshint and jssrc files. (prebid#1147)
  force cpm to be a number (prebid#1161)
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@msadsupply @dluxemburg
AdSupply adapter (prebid#1162)
86f7623 
* Added adsupply adapter

* Added client Id

* Fixed code review comments
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

@matthewlane matthewlane matthewlane approved these changes

Assignees

@matthewlane matthewlane

Labels
LGTM
Projects
None yet
Milestone
Prebid 0.23.0
Development

Successfully merging this pull request may close these issues.
4 participants
@msadsupply @matthewlane @jaiminpanchal27 @protonate