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 5 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
63 changes: 55 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,54 @@ 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';
}
}

setDeviceAcronym () {
if (window.isMobile.any) {
return 'm';
} else {
return 'd';
}
}

getSiteName () {
return window.location.host.split('.')[1];
Copy link
Contributor Author

@spra85 spra85 Oct 11, 2016

Choose a reason for hiding this comment

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

We actually want this to be the target_channel set as dimension1 further up in Revealed.js. It's meant to be the channel code of the video you're playing, not necessarily the site it is playing on. For instance, if we're playing an labs video on The Onion, we wouldn't want the theonion passed up as the site name.

I'm not sure we actually need this method at all, since we should be able to pull this value direct from the video props

Copy link
Contributor

@MelizzaP MelizzaP Oct 12, 2016

Choose a reason for hiding this comment

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

some videos have their target_channel set to onion-labs, do we want that value or should I just make it default to theonion?

}

getDfpSection () {
if (window.TARGETING.dfp_section) {
return window.TARGETING.dfp_section;
} else if (window.TARGETING.dfp_specialcoverage) {
let slug = window.TARGETING.dfp_specialcoverage;
return `specialcoverage_${slug}`;
} else {
return 'video';
}
}

getCsidValue (videoMeta) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should try to make the method name for this much more descriptive than getCsidValue because csid is sort of cryptic. Maybe getSiteSection?

let deviceAcronym = this.setDeviceAcronym();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we make this getDeviceAcronym to stay consistent with those other accessor functions?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was trying to differentiate from values we are pulling in from other places the vs values we are setting.

Copy link
Contributor

Choose a reason for hiding this comment

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

but I suppose its really 6 of one half a dozen of the other

let siteName = this.getSiteName();
let dfpSection = this.getDfpSection();


return `${deviceAcronym}.${siteName}_${dfpSection}_${videoMeta.hostChannel}`;
}

vastUrl (videoMeta) {
let baseUrl = `http://${SETTINGS.FREEWHEEL_NETWORK_ID}.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=' + 'vmap1';
baseUrl += '&prof=' + prof;

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



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

Expand Down
127 changes: 117 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,106 @@ describe('<bulbs-video> <Revealed>', () => {
});
});

describe('getProfValue', () => {
it('returns different values on desktop and mobile', () => {
// desktop
window.isMobile.any = false;
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('getCsidValue', () => {
let response;
let getSiteNameStub;
let setDeviceAcronymStub;
let getDfpSectionStub;
let videoMeta;

beforeEach(() => {
getSiteNameStub = sinon.stub().returns('website');
setDeviceAcronymStub = sinon.stub().returns('d');
getDfpSectionStub = sinon.stub().returns('fun');
videoMeta = {
hostChannel: 'host_channel',
};
response = Revealed.prototype.getCsidValue.call({
getSiteName: getSiteNameStub,
setDeviceAcronym: setDeviceAcronymStub,
getDfpSection: getDfpSectionStub,
}, videoMeta);
});

// csid format: <device acronym>.<site name>_<dfp_section>_<host channel>
// example: d.theonion_specialcoverage_boldness_main
it('#setDeviceAcronym device acronym ', () => {
// desktop
window.isMobile.any = false;
response = Revealed.prototype.setDeviceAcronym.call();
expect(response).to.equal('d');

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

it('sets site name', () => {
expect(response.includes('website')).to.be.true;
});

context('getDfpSection', () => {
it('window.TARGETING.dfp_section is set', () => {
window.TARGETING = { dfp_section: 'sunshine' };
response = Revealed.prototype.getDfpSection.call();
expect(response).to.eql(window.TARGETING.dfp_section);
});

it('special coverage page', () => {
window.TARGETING = { dfp_specialcoverage: 'forest-walk' };
response = Revealed.prototype.getDfpSection.call();
let expected = `specialcoverage_${window.TARGETING.dfp_specialcoverage}`;
expect(response).to.eql(expected);
});

it('not special coverage or dfp_section', () => {
window.TARGETING = {};
response = Revealed.prototype.getDfpSection.call();
expect(response).to.eql('video');
});
});

it('populates csid', () => {
response = Revealed.prototype.getCsidValue.call({
getSiteName: getSiteNameStub,
setDeviceAcronym: setDeviceAcronymStub,
getDfpSection: getDfpSectionStub,
}, videoMeta);
let expected = 'd.website_fun_host_channel';
expect(response).to.eql(expected);
});
});

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

context('default', () => {
beforeEach(() => {
cacheBusterStub = sinon.stub().returns('456');
vastTestStub = sinon.stub().returns(null);
getProfValueStub = sinon.stub().returns('testy');
getSiteNameStub = sinon.stub().returns('website');
videoMeta = {
tags: ['clickhole', 'main', '12345'],
category: 'main/clickhole',
Expand All @@ -547,27 +640,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('111976.v.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 +687,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
30 changes: 30 additions & 0 deletions lib/bulbs-elements/settings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
let settings = {
base: {
BULBS_ELEMENTS_ONIONSTUDIOS_GA_ID: 'UA-223393-14',
BULBS_ELEMENTS_COMSCORE_ID: '6036328',
},
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: '112214',
}
}

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;