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

Update JCM Adapter to 1.0 #1715

Merged
merged 20 commits into from
Oct 24, 2017
Merged

Update JCM Adapter to 1.0 #1715

merged 20 commits into from
Oct 24, 2017

Conversation

JCarterGw
Copy link
Contributor

Type of change

-- [ ] Other

Description of change

Update JCM adapter for 1.0 compatability

  • test parameters for validating bids
var adUnits = [
  {
    code: 'test-div1',
    sizes: [[300, 250]], // display 300x250 
    bids: [
      {
      bidder: 'jcm',
      params: {
      siteId: '3608'
     }
  }
 ]
},{
  code: 'test-div2',
  sizes: [[728, 90]], // display 728x90
  bids: [
    {
    bidder: 'jcm',
    params: {
      siteId: '3608'
     }
   }
  ]
 }
];

- contact email of the adapter’s maintainer
[email protected]
- [ ] official adapter submission

dbemiller
dbemiller reviewed Oct 18, 2017
View reviewed changes
Copy link
Contributor

@dbemiller dbemiller left a comment

Choose a reason for hiding this comment

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

The code looks pretty good, barring a few things.

I'm not getting bids back, though. Screenshot to follow.

adapters.json Outdated
@@ -0,0 +1,34 @@
[
Copy link
Contributor

Choose a reason for hiding this comment

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

File committed accidentally?

package.json Outdated
@@ -11,13 +11,15 @@
"type": "git",
"url": "https://github.com/prebid/Prebid.js.git"
},

Copy link
Contributor

Choose a reason for hiding this comment

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

More unintended changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There should be no changes to package.json or adapters.json in this PR.

Copy link
Contributor

@dbemiller dbemiller Oct 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

ok. Please remove them when you update, then

test/spec/modules/jcmBidAdapter_spec.js Outdated
expect(bidObj).to.have.property('callbackId').and.to.equal('3c9408cdbf2f68');
it('sends bid request to ENDPOINT via GET', () => {
const request = spec.buildRequests(bidRequests);
expect(request.method).to.equal('GET');
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs some real assertions... barely tests buildRequests at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will look into this.

@dbemiller
Copy link
Contributor

screen shot 2017-10-18 at 10 11 48 am

@JCarterGw
Copy link
Contributor Author

Our servers required an update. The test ad request should now deliver an ad.

@dbemiller
Copy link
Contributor

nope... still getting the same error

@JCarterGw
Copy link
Contributor Author

Please retest the ad request. The cross-origin issue is resolved.

@jaiminpanchal27 jaiminpanchal27 merged commit 923adfb into prebid:master Oct 24, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Oct 25, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js
ec7c583 
* 'master' of https://github.com/prebid/Prebid.js: (414 commits)
  Make response headers available to the specs (prebid#1748)
  add option to run tests in a specific file (prebid#1727)
  Update JCM Adapter to 1.0  (prebid#1715)
  Finished an unfinished comment. (prebid#1749)
  Platform.io Bidder Adapter update.  Prebid v1.0. (prebid#1705)
  Fix window.top.host cross origin issue when in nested iframes. (prebid#1730)
  fix log message not displaying when referencing missing bidder (prebid#1737)
  Allow more than one placement from one page (prebid#1692)
  Justpremium Adapter bugfix (prebid#1716)
  Updating license (prebid#1717)
  realvuBidAdapter  (prebid#1571)
  Update JSDoc to call the module `pbjs` (prebid#1572)
  Update Beachfront adapter for v1.0 (prebid#1675)
  Update AdButler adapter for Prebid v1.0 (prebid#1664)
  Increment pre version
  Fix for prebid#1628 (allowing standard bidCpmAdjustment) (prebid#1645)
  Prebid 0.31.0 Release
  Support native click tracking (prebid#1691)
  Initial commit for video support for pbs (prebid#1706)
  Fixes: Immediate adapter response may end auction (prebid#1690)
  ...
mattpr pushed a commit to mattpr/Prebid.js that referenced this pull request Oct 31, 2017
@JCarterGw @mattpr
Update JCM Adapter to 1.0 (prebid#1715)
bae3886 
* Add files via upload

* Update package.json

* Add files via upload

Test for jcm adapter

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Delete jcm.js

* Add files via upload

* Add files via upload

* Add files via upload

* Delete jcm_spec.js

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Delete adapters.json

* Update package.json
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@JCarterGw @dluxemburg
Update JCM Adapter to 1.0 (prebid#1715)
69d6901 
* Add files via upload

* Update package.json

* Add files via upload

Test for jcm adapter

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Delete jcm.js

* Add files via upload

* Add files via upload

* Add files via upload

* Delete jcm_spec.js

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Add files via upload

* Delete adapters.json

* Update package.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbemiller dbemiller dbemiller approved these changes

@jaiminpanchal27 jaiminpanchal27 jaiminpanchal27 approved these changes

Assignees

@jaiminpanchal27 jaiminpanchal27

Labels
LGTM Prebid 1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JCarterGw @dbemiller @jaiminpanchal27 @mjacobsonny