Skip to content

Commit 398d40a

Browse files
committed
Add a check for mixed priorities (some placement with and some without)
1 parent 6d1e120 commit 398d40a

File tree

4 files changed

+34
-21
lines changed

4 files changed

+34
-21
lines changed

dist/ethicalads.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/ethicalads.min.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

index.js

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,11 @@ export class Placement {
517517
static from_element(element) {
518518
// Get attributes from DOM node
519519
const publisher = element.getAttribute(ATTR_PREFIX + "publisher");
520+
if (!publisher) {
521+
logger.debug("Missing publisher value. Skipping...");
522+
return null;
523+
}
524+
520525
let ad_type = element.getAttribute(ATTR_PREFIX + "type");
521526
if (!ad_type) {
522527
ad_type = "image";
@@ -1186,17 +1191,26 @@ export function load_placements(force_load = false) {
11861191
}
11871192

11881193
// Group prioritized placements
1189-
let priority_group = [];
1194+
let has_priority = false;
11901195
placements.forEach((placement) => {
1191-
if (
1192-
placement &&
1193-
placement.priority !== null &&
1194-
(force_load || !placement.load_manually)
1195-
) {
1196-
priority_group.push(placement);
1196+
if (placement && placement.priority !== null) {
1197+
has_priority = true;
11971198
}
11981199
});
11991200

1201+
let priority_group = [];
1202+
if (has_priority) {
1203+
placements.forEach((placement) => {
1204+
if (placement && (force_load || !placement.load_manually)) {
1205+
if (placement.priority === null) {
1206+
// Set default priority for non-prioritized placements
1207+
placement.priority = 1;
1208+
}
1209+
priority_group.push(placement);
1210+
}
1211+
});
1212+
}
1213+
12001214
if (priority_group.length > 0) {
12011215
Placement.fetchGroup(priority_group);
12021216
}

tests/priority-placement.test.html

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
<html>
22
<body>
33
<!-- 3 placements, two with priorities -->
4-
<!-- There will be 2 calls to the ad server: one for ad1 and ad2, and one for ad3 -->
4+
<!-- There will be 1 call to the ad server for all ad1, ad2, and ad3 -->
55

66
<div id="ad1" data-ea-publisher="test" data-ea-priority="1"></div>
77
<div id="ad2" data-ea-publisher="test" data-ea-priority="2"></div>
8-
<!-- standalone ad -->
8+
<!-- standalone ad (gets default priority 1) -->
99
<div id="ad3" data-ea-publisher="test"></div>
1010

1111
<script type="module">
@@ -16,10 +16,10 @@
1616
import { wait, Placement } from "../index";
1717
import { mockAdDecision } from "./common.inc.js";
1818

19-
// Mock the regular fetch for ad3
19+
// Mock the regular fetch
2020
let fetchStub = mockAdDecision();
2121

22-
// Mock the fetchGroup for ad1 and ad2
22+
// Mock the fetchGroup for ad1, ad2, and ad3
2323
let groupStub = sinon
2424
.stub(Placement, "fetchGroup")
2525
.callsFake((placements) => {
@@ -43,26 +43,25 @@
4343
describe("EthicalAds library", () => {
4444
it("loads prioritized placements correctly", async () => {
4545
const placements = await wait;
46-
// Placements that resolved with ads: only ad2 and ad3
47-
expect(placements.length).to.equal(2);
46+
// Placements that resolved with ads: only ad2
47+
expect(placements.length).to.equal(1);
4848
expect(placements[0].target.id).to.equal("ad2");
49-
expect(placements[1].target.id).to.equal("ad3");
5049

5150
// Look at DOM directly
5251
const ad1 = document.getElementById("ad1");
5352
const ad2 = document.getElementById("ad2");
5453
const ad3 = document.getElementById("ad3");
5554

56-
// ad1 should NOT have the loaded class
55+
// ad1 and ad3 should NOT have the loaded class
5756
expect(ad1.className).to.not.include("loaded");
57+
expect(ad3.className).to.not.include("loaded");
5858

59-
// ad2 and ad3 should have the loaded class
59+
// ad2 should have the loaded class
6060
expect(ad2.className).to.include("loaded");
61-
expect(ad3.className).to.include("loaded");
6261

63-
// fetchGroup should have been called once with a group of 2
62+
// fetchGroup should have been called once with a group of 3
6463
expect(groupStub.calledOnce).to.be.true;
65-
expect(groupStub.firstCall.args[0].length).to.equal(2);
64+
expect(groupStub.firstCall.args[0].length).to.equal(3);
6665
});
6766
});
6867
});

0 commit comments

Comments
 (0)