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

Video - create mastertagUrl from adservertag #614

Merged
merged 23 commits into from
Sep 15, 2016
Merged

Conversation

jaiminpanchal27
Copy link
Collaborator

No description provided.

var bidObject = $$PREBID_GLOBAL$$._bidsReceived.find(bid => bid.adUnitCode === that.code);
return bidObject;
};
return that;
Copy link
Collaborator

@matthewlane matthewlane Sep 9, 2016

Choose a reason for hiding this comment

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

If you call this function as a constructor with the new keyword, you can remove lines 5 and 12 as these steps will then happen implicitly. Then change the 4 other thats to this and capitalize adserver on line 4 to prevent jshint from giving a warning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@matthewlane thanks.

@matthewlane
Copy link
Collaborator

@jaiminpanchal27 left a few code notes. Also it'd be good to have unit tests for the new functions. Looking good!

@@ -0,0 +1,57 @@
import {formatQS} from './url';
Copy link
Member

Choose a reason for hiding this comment

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

I think this file should be named 'video.js' - since it's relating to video

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After discussion we decided to keep it adserver.js as of now. When more adservers are added it will be updated

return formatURL(dfpAdserverObj.urlComponents);
} else {
utils.logError('Only DFP adserver is supported');
return;
Copy link
Member

Choose a reason for hiding this comment

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

return adserverTag here

@mkendall07
Copy link
Member

@jaiminpanchal27
Please update so we can get this merged in. Thanks!

@mkendall07 mkendall07 merged commit 24d2fc2 into master Sep 15, 2016
@mkendall07 mkendall07 deleted the video-mastertag branch September 22, 2016 19:06
marian-r added a commit to aol/Prebid.js that referenced this pull request Sep 23, 2016
…3.0 to master

* commit '9d4bc7fda6bcc249544fb8636b98e90fdc8d474b': (44 commits)
  Added support for new adapters introduced in 0.13.0 into AOL analytics
  Updated CHANGELOG
  Fixed merge conflict
  Prebid 0.13.0 Release
  Resolves prebid#635 (prebid#640)
  Add IX Deal Support (prebid#638)
  Bug fix: accept custom timeout prebid#582 & prebid#604 (prebid#641)
  added timeouts to rubicon adapter that consider time-to-start costs (prebid#629)
  Modify handling of no-bids in Krux Link adapter (prebid#628)
  Allow bypassing ajax preflights with config options (prebid#630)
  validateIndentation: Invalid indentation character (prebid#631)
  use `splice` rather than `slice` to remove bids from array (prebid#637)
  delete the callback before calling clearAuction (prebid#636)
  Deal override fix to resolve prebid#618 (prebid#619)
  Update package.json
  Video - create mastertagUrl from adservertag (prebid#614)
  Prevent renderAd from rendering videos (prebid#623)
  Remove unused build depencencies (prebid#622)
  Add pull request template (prebid#615)
  Openx adaptor deal update (prebid#612)
  ...
Studnicky pushed a commit to sonobi/Prebid.js that referenced this pull request Oct 4, 2016
* Support video demand requests for video-enabled adUnits

* Refactor and write unit tests

* Enable adapters to register as supporting video

* Pass adUnit mediaType to adapters

* Place helper functions at module-level

* Master video tag from adservertag

* Add example page with setup for video placement

* Move helper functions to video module

* Clarify error message

* Updates

* master tag function updates

* Moved adserver code to separate file

* Video mastertag updates after code review and test cases

* Changed key in bid object (adUrl to vastUrl)

* update example file

* Update example
Studnicky pushed a commit to sonobi/Prebid.js that referenced this pull request Oct 4, 2016
* Support video demand requests for video-enabled adUnits

* Refactor and write unit tests

* Enable adapters to register as supporting video

* Pass adUnit mediaType to adapters

* Place helper functions at module-level

* Master video tag from adservertag

* Add example page with setup for video placement

* Move helper functions to video module

* Clarify error message

* Updates

* master tag function updates

* Moved adserver code to separate file

* Video mastertag updates after code review and test cases

* Changed key in bid object (adUrl to vastUrl)

* update example file

* Update example
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.

None yet

3 participants