Skip to content

Commit

Permalink
Merge pull request #1 from dtereshenko/fix/update-correlator
Browse files Browse the repository at this point in the history
Fix/update correlator
  • Loading branch information
johnsonreyliu authored Jan 15, 2020
2 parents 3e8d0db + 89a820a commit 72bc850
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 42 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ coverage/
npm-debug.log
yarn-error.log
.DS_Store
.idea
1 change: 1 addition & 0 deletions examples/apps/single-request/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class App extends Component {
</Button>
<div style={styles.lb}>
<Gpt
className="test-class"
adUnitPath={adUnitPath}
slotSize={[728, 90]}
style={styles.adBorder}
Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ticketmaster/react-gpt",
"version": "2.1.0",
"version": "2.2.0",
"description": "A react display ad component using Google Publisher Tag",
"main": "lib/index.js",
"jsnext:main": "es/index.js",
Expand Down Expand Up @@ -84,7 +84,7 @@
"prettier": "^1.9.2",
"prop-types": "^15.5.10",
"querystring": "^0.2.0",
"radium": "^0.18.1",
"radium": "^0.21.2",
"react": "^16.0.0",
"react-addons-test-utils": "^15.0.1",
"react-dom": "^16.0.0",
Expand Down Expand Up @@ -118,7 +118,8 @@
"pretest": "npm run build",
"prepublish": "npm run build && npm run build:es && npm run build:umd && npm run build:umd:min",
"test": "npm run lint && karma start",
"update-apilist": "node ./scripts/updateAPIList.js"
"update-apilist": "node ./scripts/updateAPIList.js",
"prepare": "npm run build"
},
"config": {
"commitizen": {
Expand All @@ -128,7 +129,7 @@
"bundlesize": [
{
"path": "./dist/react-gpt.min.js",
"maxSize": "8.5 kB"
"maxSize": "8.53 kB"
}
]
}
80 changes: 57 additions & 23 deletions src/Bling.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ class Bling extends Component {
* @property id
*/
id: PropTypes.string,
/**
* An optional string to be used as prefix to element id.
*
* @property idName
*/
idName: PropTypes.string,
/**
* An optional string to be used as container div class.
*
* @property className
*/
className: PropTypes.string,
/**
* An optional string indicating ad unit path which will be used
* to create an ad slot.
Expand Down Expand Up @@ -191,7 +203,17 @@ class Bling extends Component {
*
* @property style
*/
style: PropTypes.object
style: PropTypes.object,
/**
* An optional property to control non-personalized Ads.
* https://support.google.com/admanager/answer/7678538
*
* Set to `true` to mark the ad request as NPA, and to `false` for ad requests that are eligible for personalized ads
* It is `false` by default, according to Google's definition.
*
* @property npa
*/
npa: PropTypes.bool
};

/**
Expand All @@ -217,7 +239,13 @@ class Bling extends Component {
* @property reRenderProps
* @static
*/
static reRenderProps = ["adUnitPath", "slotSize", "outOfPage", "content"];
static reRenderProps = [
"adUnitPath",
"slotSize",
"outOfPage",
"content",
"npa"
];
/**
* An instance of ad manager.
*
Expand Down Expand Up @@ -348,15 +376,6 @@ class Bling extends Component {
static clear(slots) {
Bling._adManager.clear(slots);
}
/**
* Updates the correlator value for the next ad request.
*
* @method updateCorrelator
* @static
*/
static updateCorrelator() {
Bling._adManager.updateCorrelator();
}

static set testManager(testManager) {
invariant(testManager, "Pass in createManagerTest to mock GPT");
Expand Down Expand Up @@ -387,8 +406,9 @@ class Bling extends Component {
}

componentWillReceiveProps(nextProps) {
const {propsEqual} = Bling._config;
const {sizeMapping} = this.props;
const { propsEqual } = Bling._config;
const { sizeMapping } = this.props;

if (
(nextProps.sizeMapping || sizeMapping) &&
!propsEqual(nextProps.sizeMapping, sizeMapping)
Expand Down Expand Up @@ -603,10 +623,12 @@ class Bling extends Component {
}

defineSlot() {
const {adUnitPath, outOfPage} = this.props;
const {adUnitPath, outOfPage, npa} = this.props;
const divId = this._divId;
const slotSize = this.getSlotSize();

this.handleSetNpaFlag(npa);

if (!this._adSlot) {
if (outOfPage) {
this._adSlot = Bling._adManager.googletag.defineOutOfPageSlot(
Expand Down Expand Up @@ -701,12 +723,6 @@ class Bling extends Component {
if (content) {
Bling._adManager.googletag.content().setContent(adSlot, content);
} else {
if (
!Bling._adManager._disableInitialLoad &&
!Bling._adManager._syncCorrelator
) {
Bling._adManager.updateCorrelator();
}
Bling._adManager.googletag.display(divId);
if (
Bling._adManager._disableInitialLoad &&
Expand Down Expand Up @@ -740,7 +756,7 @@ class Bling extends Component {

render() {
const {scriptLoaded} = this.state;
const {id, outOfPage, style} = this.props;
const {id, idName, className, outOfPage, style} = this.props;
const shouldNotRender = this.notInViewport(this.props, this.state);

if (!scriptLoaded || shouldNotRender) {
Expand Down Expand Up @@ -777,9 +793,27 @@ class Bling extends Component {
Bling._adManager.googletag.destroySlots([this._adSlot]);
this._adSlot = null;
}
this._divId = id || Bling._adManager.generateDivId();
this._divId = id || Bling._adManager.generateDivId(idName);

return <div className={className} id={this._divId} style={style} />;
}

return <div id={this._divId} style={style} />;
/**
* Call pubads and set the non-personalized Ads flag, if it is not undefined.
*
* @param {boolean} npa
*/
handleSetNpaFlag(npa) {
if (npa === undefined) {
return;
}

Bling._adManager.pubadsProxy({
method: "setRequestNonPersonalizedAds",
args: [npa ? 1 : 0],
resolve: null,
reject: null
});
}
}

Expand Down
16 changes: 2 additions & 14 deletions src/createManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ export class AdManager extends EventEmitter {
this._syncCorrelator = value;
}

generateDivId() {
return `bling-${++this._adCnt}`;
generateDivId(idName = "bling") {
return `${idName}-${++this._adCnt}`;
}

getMountedInstances() {
Expand Down Expand Up @@ -428,9 +428,6 @@ export class AdManager extends EventEmitter {
// first instance updates correlator value and re-render each ad
const instances = this.getMountedInstances();
instances.forEach((instance, i) => {
if (i === 0) {
this.updateCorrelator();
}
instance.forceUpdate();
});

Expand All @@ -451,15 +448,6 @@ export class AdManager extends EventEmitter {
return this.googletag.pubads().getVersion();
}

updateCorrelator() {
if (!this.pubadsReady) {
return false;
}
this.googletag.pubads().updateCorrelator();

return true;
}

load(url) {
return (
this._loadPromise ||
Expand Down
1 change: 0 additions & 1 deletion src/utils/apiList.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export const pubadsAPI = [
["getVideoContent", "function"],
["getCorrelator", "function"],
["setCorrelator", "function"],
["updateCorrelator", "function"],
["isAdRequestFinished", "function"],
["collapseEmptyDivs", "function"],
["clear", "function"],
Expand Down
39 changes: 39 additions & 0 deletions test/Bling.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,45 @@ describe("Bling", () => {
);
});

it("call pubads API to set non-personalized Ads when npa prop is set", done => {
const spy = sinon.stub(Bling._adManager, "pubadsProxy");
const expectedParamTrue = {
method: "setRequestNonPersonalizedAds",
args: [1],
resolve: null,
reject: null
};
const expectedParamFalse = {
...expectedParamTrue,
args: [0]
};

Bling.once(Events.RENDER, () => {
expect(spy.calledWith(expectedParamTrue)).to.be.true;
expect(spy.calledWith(expectedParamFalse)).to.be.true;
spy.restore();
done();
});

// Render once to test with non-personalized ads
ReactTestUtils.renderIntoDocument(
<Bling
adUnitPath="/4595/nfl.test.open"
npa={true}
slotSize={["fluid"]}
/>
);

// Render a second time to test re-enable personalized ads
ReactTestUtils.renderIntoDocument(
<Bling
adUnitPath="/4595/nfl.test.open"
npa={false}
slotSize={["fluid"]}
/>
);
});

it("fires once event", done => {
const events = Object.keys(Events).map(key => Events[key]);

Expand Down

0 comments on commit 72bc850

Please sign in to comment.