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.

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

adding Essens adapter #1353

Merged
merged 6 commits into from
Aug 14, 2017
Merged

adding Essens adapter #1353

merged 6 commits into from
Aug 14, 2017

Conversation

navneetpandey
Copy link
Contributor

Type of change

  • Bugfix
  • Feature
  • [x ] 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

New Essens bidder

  • test parameters for validating bids
{
  bidder: 'essens',
  params: {
           placementId: 'n4xv71a9' 
   }
}

Be sure to test the integration with your adserver using the Hello World sample page.

  • contact email of the adapter’s maintainer
    Navneet Kumar Pandey navneet@essens.no
  • official adapter submission

Other information

prebid/prebid.github.io#289 For documentation.

if (bid.bidId && bid.params && bid.params.placementId) {
return true
} else {
utils.logError('bid requires missing essential params for essens')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should register a NO_BID as well


function getEssesnBidRequests () {
return $$PREBID_GLOBAL$$
._bidsRequested.find(bidSet => bidSet.bidderCode === 'essens')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's better that you keep tabs on the bid request in the adapter (it's passed to callbids) rather than poking around at private Prebid.js variables which could change.

stubLoadScript.restore()
);

it('invalid case: missing bid', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't think of any case where bids would be missing. But there should be cases where bad bids are sent, i.e. bad bid.params. So I'm not sure this is a good test.


afterEach(() => {
stubAddBidResponse.restore()
pbjs._bidsRequested.pop()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't be reliant on prebid private vars. When you stub addBidRespone you can grab the bids. This is important as it can mess with other tests that may be doing this same thing (but shouldn't be). In fact, since you are stubbing addBidResponse

Also should be $$PREBID_GLOBAL$$ instead of pbjs

'id': '1234'
}

pbjs._bidsRequested.push(bidderRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing. don't use pbjs private vars.

snapwich
snapwich previously approved these changes Jul 20, 2017
@dbemiller
Copy link
Contributor

Hey @navneetpandey, can you pull from master and fix the lint errors?

There were also some minor changes to the format of adapters in #1422 and #1459. Check out any of the other adapters for an example of how it's done now.

@navneetpandey
Copy link
Contributor Author

Hi @dbemiller I made changes and fixed the lint error. The built is failing on other adapter (Rubicon). Let me know if I need to make more changes in our adapter.

@dbemiller
Copy link
Contributor

Looks like some bad unit tests got merged in #1320. It's being worked on in #1482.

Tests on your branch do pass for me locally, though, so everything looks good on your branch as far as I'm concerned.

@dbemiller dbemiller merged commit 4dd8896 into prebid:master Aug 14, 2017
@dbemiller
Copy link
Contributor

Looks like the CI passed on that last changeset anyway... so I just merged it now.

You'll also want to submit a PR to the docs repo to get your bidder added to the bidders params page, which many publishers use.

@navneetpandey
Copy link
Contributor Author

I already made PR for the doc repo. See prebid/prebid.github.io#289. Let me know if I need to do something more there.

@dbemiller
Copy link
Contributor

aha! ok, cool. Sorry I didn't see that.

Our docs guy, @rmloveland, will look it over and let you know there.

philipwatson pushed a commit to mbrtargeting/Prebid.js that referenced this pull request Sep 18, 2017
* adding Essens adapter

* Resloved issues from pull request submition

* Lint, createNew constructor related fix

* reverting yarn changes, and fixing missed modification at essensAdapter
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017
* adding Essens adapter

* Resloved issues from pull request submition

* Lint, createNew constructor related fix

* reverting yarn changes, and fixing missed modification at essensAdapter
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* adding Essens adapter

* Resloved issues from pull request submition

* Lint, createNew constructor related fix

* reverting yarn changes, and fixing missed modification at essensAdapter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants