Skip to content
This repository has been archived by the owner on Feb 13, 2019. It is now read-only.

Player integration for Freewheel video ad server #177

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions elements/bulbs-video/components/revealed.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import Comscore from '../plugins/comscore';

import React, { PropTypes } from 'react';
import invariant from 'invariant';
import SETTINGS from 'bulbs-elements/settings';

// FIXME: where should this be defined? Per-app?
// Or in some better sort of settings file here?
global.BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID = 'UA-223393-14';
global.BULBS_ELEMENTS_COMSCORE_ID = '6036328';
global.BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID = SETTINGS.BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID;
global.BULBS_ELEMENTS_COMSCORE_ID = SETTINGS.BULBS_ELEMENTS_COMSCORE_ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than dump this on settings, I think we should just read it from SETTINGS every time we use these values.


let prefixCount = 0;
function makeGaPrefix () {
Expand Down Expand Up @@ -40,6 +39,11 @@ export default class Revealed extends React.Component {
'`<bulbs-video>` requires `jwplayer` to be in global scope.'
);

invariant(
window.isMobile,
'`<bulbs-video>` requires `isMobile()` to be set on window.'
);

let gaPrefix = makeGaPrefix();
ga('create', BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID, 'auto', { name: gaPrefix });

Expand Down Expand Up @@ -156,13 +160,23 @@ export default class Revealed extends React.Component {
return false;
}

vastUrl (videoMeta) {
let baseUrl = 'http://us-theonion.videoplaza.tv/proxy/distributor/v2?rt=vast_2.0';
getProfValue () {
if (window.isMobile.any) {
return 'theonion_mobileweb_html5';
} else {
return 'theonion_desktop_html5';
}
}

vastUrl (videoMeta) {
let baseUrl = `http://${SETTINGS.FREEWHEEL_NETWORK_ID}v.fwmrm.net/ad/g/1?`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MelizzaP missing a . between the network id and the v. Example (http://1b656.v.fwmrm.net/ad/g/1?...)

let vastTestId = this.vastTest(window.location.search);

// AD_TYPE: one of p (preroll), m (midroll), po (postroll), o (overlay)
baseUrl += '&tt=p';

let prof = this.getProfValue();
baseUrl += '&resp=' + SETTINGS.RESP;
baseUrl += '&prof=' + prof;

videoMeta.tags.push('html5'); // Force HTML 5
// Tags
baseUrl += '&t=' + videoMeta.tags;
Expand All @@ -184,6 +198,8 @@ export default class Revealed extends React.Component {
return baseUrl;
}



extractTrackCaptions (sources, defaultCaptions) {
let captions = [];

Expand Down
53 changes: 43 additions & 10 deletions elements/bulbs-video/components/revealed.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ describe('<bulbs-video> <Revealed>', () => {

describe('componentDidMount globalsCheck', () => {
global.BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID = 'a-ga-id';
window.isMobile = () => {};
window.isMobile.any = false;
global.ga = () => {};

const globals = [
Expand Down Expand Up @@ -529,15 +531,32 @@ describe('<bulbs-video> <Revealed>', () => {
});
});

describe('getProfValue', () => {
it('returns different values on desktop and mobile', () => {
// desktop
let response = Revealed.prototype.getProfValue.call();
expect(response).to.equal('theonion_desktop_html5');

// mobile
window.isMobile.any = true;
response = Revealed.prototype.getProfValue.call();
expect(response).to.equal('theonion_mobileweb_html5');
});
});

describe('vastUrl', () => {
let videoMeta;
let cacheBusterStub;
let vastTestStub;
let vastUrl;
let getProfValueStub;
let parsed;

context('default', () => {
beforeEach(() => {
cacheBusterStub = sinon.stub().returns('456');
vastTestStub = sinon.stub().returns(null);
getProfValueStub = sinon.stub().returns('testy');
videoMeta = {
tags: ['clickhole', 'main', '12345'],
category: 'main/clickhole',
Expand All @@ -547,27 +566,40 @@ describe('<bulbs-video> <Revealed>', () => {
});

it('returns the vast url', function () {
let vastUrl = Revealed.prototype.vastUrl.call({
vastUrl = Revealed.prototype.vastUrl.call({
cacheBuster: cacheBusterStub,
vastTest: vastTestStub,
getProfValue: getProfValueStub,
}, videoMeta);
let parsed = url.parse(vastUrl, true);
parsed = url.parse(vastUrl, true);
expect(parsed.protocol).to.eql('http:');
expect(parsed.host).to.eql('us-theonion.videoplaza.tv');
expect(parsed.pathname).to.eql('/proxy/distributor/v2');
expect(Object.keys(parsed.query)).to.eql(['rt', 'tt', 't', 's', 'rnd']);
expect(parsed.query.rt).to.eql('vast_2.0');
expect(parsed.query.tt).to.eql('p');
expect(parsed.query.t).to.eql('clickhole,main,12345,html5');
expect(parsed.query.s).to.eql('host_channel/channel_slug');
expect(parsed.query.rnd).to.eql('456');
expect(parsed.host).to.eql('111976v.fwmrm.net');
expect(parsed.pathname).to.eql('/ad/g/1');
});

it('populates keys for required global params', () => {
let expectedQueryKeys = [
'resp', 'prof', 'csid', 'caid', 'pvrn', 'vprn', 'cana'
];
let queryKeys = Object.keys(parsed.query);
expect(queryKeys).to.eql(expectedQueryKeys);
});

context('populates values for required global params', () => {
it('resp', function () {
expect(parsed.query.resp).to.eql('vmap1');
});
it('prof', function () {
expect(parsed.query.prof).to.eql('testy');
});
});
});

context('when series_slug is given', () => {
beforeEach(() => {
cacheBusterStub = sinon.stub().returns('456');
vastTestStub = sinon.stub().returns(null);
getProfValueStub = sinon.stub().returns('testy');
videoMeta = {
tags: ['clickhole', 'main', '12345'],
category: 'main/clickhole',
Expand All @@ -581,6 +613,7 @@ describe('<bulbs-video> <Revealed>', () => {
let vastUrl = Revealed.prototype.vastUrl.call({
cacheBuster: cacheBusterStub,
vastTest: vastTestStub,
getProfValue: getProfValueStub,
}, videoMeta);
let parsed = url.parse(vastUrl, true);
expect(parsed.query.s).to.eql('host_channel/channel_slug/series_slug');
Expand Down
31 changes: 31 additions & 0 deletions lib/bulbs-elements/settings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
let settings = {
base: {
BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID: 'UA-223393-14',
BULBS_ELEMENTS_COMSCORE_ID: '6036328',
RESP: 'vmap1',
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 think we can just hardcode this instead of making it a setting, it'll never change.

},
development: {
FREEWHEEL_NETWORK_ID: '111976',
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 think these network IDs should actually be stored in each property's python settings and then passed into the the bulbs-element somehow, maybe even as a namespaced variable on window. I don't believe the NODE_ENV gives you what you need to integrate into each of the bulbs properties, but I'd want to @collin to weigh in.

Copy link
Contributor

Choose a reason for hiding this comment

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

my logic behind it was that they were the same for each site, so we wouldn't have to repeat them. And I just felt weird about putting them on the window.

Copy link
Contributor

Choose a reason for hiding this comment

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

So far we've been kicking the can down the road on some of these bulbs-elements settings.

I think this is a good step to have a module that exports settings.

Maybe we could add a settings object to the properties header (before bulbs-elements is loaded).

<script>
window.BULBS_ELEMENTS_SETTINGS = {};
</script>

And in settings.js

if (window.BULBS_ELEMENTS_SETTINGS) {
  Object.assign(settings, window.BULBS_ELEMENTS_SETTINGS);
}

Then we can decide where a setting should live on a case-by-case basis.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the long-term I'd like to get bulbs-elements into omni and find some way to access the python settings at build time. Then we could have one settings file for everything.

But 1 settings.py location and 1 settings.js location beats having these settings strewn about the individual js files.

Copy link
Contributor

@MelizzaP MelizzaP Oct 11, 2016

Choose a reason for hiding this comment

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

if the end goal is to have all the settings in one settings.py file and we are pushing these settings to window anyway. I'd argue, it might make sense to just put them in each sites' settings.py file. Having a settings.js and settings.py file just seems to add an unnecessary and possibly confusing extra layer of complication.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

},
test: {
FREEWHEEL_NETWORK_ID: '111976',
},
production: {
FREEWHEEL_NETWORK_ID: '123456',
}
}

function getEnvironment () {
let envValues;
if (process.env.NODE_ENV === 'production') {
envValues = settings.production;
} else if ( process.env.NODE_ENV === 'test') {
envValues = settings.test;
} else {
envValues = settings.development;
}
return Object.assign(settings.base, envValues);
};

let SETTINGS = getEnvironment();
export default SETTINGS;