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

Bidfluence Adapter #1023

Merged
merged 10 commits into from
Mar 10, 2017
Merged

Bidfluence Adapter #1023

merged 10 commits into from
Mar 10, 2017

Conversation

francescocristallo
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: 'bidfluence',
sizes: [[300, 250]],
  params: {
     adunitId: "747efe9c-5f8a-4b6e-872b-8e9939816298",
     pubId: "c4bbd807-7d22-485f-80f1-ba008ef1c619"
  }
}

-integrations@bidfluence.com

  • official adapter submission

Other information

@jaiminpanchal27 jaiminpanchal27 self-assigned this Mar 2, 2017
@rmloveland rmloveland mentioned this pull request Mar 2, 2017
Closed
jaiminpanchal27
jaiminpanchal27 requested changes Mar 6, 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.

Verified the bids. Please address the comments. Thanks

src/adapters/bidfluence.js Outdated
window.bfPbjsCB = function (bfr) {
var bidObject = null;
if (bfr.cpm > 0) {
bidObject = bidfactory.createBid(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add bid request as second param to match request response pair.

src/adapters/bidfluence.js Outdated
bidObject.width = bfr.width;
bidObject.height = bfr.height;
} else {
bidObject = bidfactory.createBid(2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add bid request as second param to match request response pair.

src/adapters/bidfluence.js Outdated
placementCode: bid.placementCode
};
/* jshint ignore:end */
var s = document.createElement('script');
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is recommended to use loadscript function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't use it because I need to set the Id of the new script element as per line 43, seems the loadscript function does not allow it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, one question, why would you need an id on script element

Copy link
Contributor Author

@francescocristallo francescocristallo Mar 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It was for legacy code, I spent some time today to fix that. The adapter now loadscript function as requested, all the update requested are complete. Thanks

jaiminpanchal27
jaiminpanchal27 reviewed Mar 7, 2017
View reviewed changes
src/adapters/bidfluence.js Outdated
var BidfluenceAdapter = function BidfluenceAdapter() {

//Callback
window.bfPbjsCB = function (bfr) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add callback to $$PREBID_GLOBAL$$

@francescocristallo
Addressed change request (all but the loader one)
7951311 
-Changed window with $$PREBID_GLOBAL$$
-Added bid request as second param to match request response pair
-Didn't change the loader since does not support custom Id to script element, necessary for our code to work.
@francescocristallo
Update reflecting the adapter changes.
510cc0f
francescocristallo
francescocristallo commented Mar 8, 2017
View reviewed changes
Copy link
Contributor Author

@francescocristallo francescocristallo 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.

Changes according to requests. Thanks

@jaiminpanchal27
Copy link
Collaborator

Looks good. Thanks @francescocristallo for the changes

@jaiminpanchal27 jaiminpanchal27 merged commit 6d6fb7e into prebid:master Mar 10, 2017
@jaiminpanchal27
Copy link
Collaborator

This is merged into master. Please submit a PR to the docs repo to add a file for your adapter to the bidders directory so your adapter's params will appear on the bidders page. Thank you for contributing

outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Mar 20, 2017
@francescocristallo
Bidfluence Adapter (prebid#1023)
2078607 
* Bidfluence Adapter

* Bidfluence adapter

* Fixed callback name

* Final review

* First commit, Test passed

* Addressed change request (all but the loader one)

-Changed window with $$PREBID_GLOBAL$$
-Added bid request as second param to match request response pair
-Didn't change the loader since does not support custom Id to script element, necessary for our code to work.

* Update reflecting the adapter changes.

* Final review, completed update requests.

* Fixed Indentation

* Fixed global variable causing validation error.
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Mar 20, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
cc9fa44 
…built

* 'master' of https://github.com/prebid/Prebid.js:
  Add GourmetAds AppNexus Alias (prebid#1057)
  fix issue calling `requestBids();` (prebid#1058)
  explicit win url response format as pixel (prebid#1001)
  OpenX Adapter: Correctly gets the page domain for cross-domain iframes (prebid#1027)
  better http/s support (prebid#1010)
  Add a new generated field transactionId to each adunits. (prebid#1040)
  Update readme (prebid#1053)
  PulsePoint Lite adapter (prebid#1016)
  Add new adapter ServerBid (by Adzerk) (prebid#1024)
  Fix Mantis tests in negative timezone (prebid#1049)
  Add deal id handling (prebid#1044)
  sanitize bidderRequest to rubicon adapter to ensure accountId is sent (prebid#1030)
  Bidfluence Adapter (prebid#1023)
  Update uglify-js version (prebid#1041)
  Add dev dependencies.
  hb_adid should be uppercase in all cases (prebid#1037)
  Add TapSense Header Bidding Adapter and tests (prebid#1004)
  iOS Referrer fix (prebid#996)
  Change identification of JavaScript user matching (prebid#1022)
  Fixed mixed tabs/spaces in wideorbit adapter (prebid#1031)
Allen-37F added a commit to Allen-37F/Prebid.js that referenced this pull request Mar 21, 2017
@Allen-37F
Merge commit '0cea31cd294f380b3b7cf46dd7a4000316b71ac1'
da9de35 
* commit '0cea31cd294f380b3b7cf46dd7a4000316b71ac1':
  Fix Mantis tests in negative timezone (prebid#1049)
  Add deal id handling (prebid#1044)
  sanitize bidderRequest to rubicon adapter to ensure accountId is sent (prebid#1030)
  Bidfluence Adapter (prebid#1023)
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@francescocristallo @dluxemburg
Bidfluence Adapter (prebid#1023)
fdd52ce 
* Bidfluence Adapter

* Bidfluence adapter

* Fixed callback name

* Final review

* First commit, Test passed

* Addressed change request (all but the loader one)

-Changed window with $$PREBID_GLOBAL$$
-Added bid request as second param to match request response pair
-Didn't change the loader since does not support custom Id to script element, necessary for our code to work.

* Update reflecting the adapter changes.

* Final review, completed update requests.

* Fixed Indentation

* Fixed global variable causing validation error.
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

Assignees

@jaiminpanchal27 jaiminpanchal27

Labels
needs update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@francescocristallo @jaiminpanchal27