Skip to content

Commit 6c55a68

Browse files
davidfischerdavidfischer
committed
Rework the group handling a bit to simplify
1 parent 398d40a commit 6c55a68

File tree

5 files changed

+94
-71
lines changed

5 files changed

+94
-71
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.html‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
<html>
1+
<!doctype html>
2+
<html lang="en">
23

34
<head>
5+
<meta charset="UTF-8" />
46
<script async src="dist/ethicalads.js"></script>
57
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/skeleton/2.0.4/skeleton.min.css" />
68
<style>
@@ -26,12 +28,13 @@
2628
<div class="container">
2729
<h1>EthicalAds</h1>
2830

29-
<div data-ea-publisher="ethicaladsio" data-ea-type="text" id="ad"></div>
31+
<div data-ea-publisher="ethicaladsio" data-ea-campaign-types="house" data-ea-type="text" data-ea-verbosity="verbose"
32+
data-ea-priority="1" id="ad-top"></div>
3033

3134
<script>
3235
var fn = (placements) => {
3336
if (!placements.length) {
34-
var elem = document.querySelector("#ad");
37+
var elem = document.querySelector("#ad-top");
3538
elem.innerHTML =
3639
"Requests to ethicalads.io are blocked. Check your ad blocker";
3740
}
@@ -241,9 +244,13 @@ <h3>Large format ad</h3>
241244
</div>
242245
</div>
243246

247+
<!--
244248
<div class="container">
245-
<p></p>
249+
<h3>Large format footer ad</h3>
250+
<div data-ea-publisher="ethicaladsio" data-ea-campaign-types="house" data-ea-type="logo-large-v1"
251+
data-ea-priority="10" id="ad-footer"></div>
246252
</div>
253+
-->
247254
</body>
248255

249256
</html>

‎index.js‎

Lines changed: 64 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@ export class Placement {
494494
this.div_id =
495495
target.id ||
496496
"ad_" + Date.now() + "_" + Math.floor(Math.random() * 1000000000);
497-
this.fetchPromise = null;
498497

499498
// Initialized and will be used in the future
500499
this.view_time = 0;
@@ -599,11 +598,11 @@ export class Placement {
599598
*
600599
* @returns {Promise}
601600
*/
602-
load() {
601+
load(fetchPromise = null) {
603602
// Detect the keywords
604603
this.keywords = this.keywords.concat(this.detectKeywords());
605604

606-
let fetchPromise = this.fetchPromise || this.fetch();
605+
fetchPromise = fetchPromise || this.fetch();
607606

608607
return fetchPromise
609608
.then((element) => {
@@ -773,6 +772,8 @@ export class Placement {
773772
if (!this.canRotate()) {
774773
return;
775774
}
775+
776+
logger.debug("Rotating ad");
776777
this.clearListeners();
777778

778779
this.view_time = 0;
@@ -873,10 +874,12 @@ export class Placement {
873874
*
874875
* @param {Array<Placement>} placements - Placements to fetch
875876
*/
876-
static fetchGroup(placements) {
877-
if (!placements || !placements.length) return;
877+
static loadGroup(placements) {
878+
if (!placements || !placements.length) return Promise.resolve([]);
878879

879880
const publisher = placements[0].publisher;
881+
882+
// Make sure all placements are from the same publisher
880883
const all_same_publisher = placements.every(
881884
(p) => p.publisher === publisher
882885
);
@@ -948,27 +951,40 @@ export class Placement {
948951
document.getElementsByTagName("head")[0].appendChild(script);
949952
});
950953

951-
placements.forEach((placement, idx) => {
952-
placement.fetchPromise = promise.then((response) => {
953-
let is_winner = false;
954-
if (response && response.html && response.view_url) {
955-
if (response.div_id) {
956-
is_winner = response.div_id === placement.div_id;
954+
return Promise.all(
955+
placements.map((placement, idx) => {
956+
const fetchPromise = promise.then((response) => {
957+
let is_winner = false;
958+
if (response && response.html && response.view_url) {
959+
if (response.div_id) {
960+
is_winner = response.div_id === placement.div_id;
961+
} else {
962+
// This should only happen if there are no ads for this request
963+
// In this case, we'll assign the first placement to be the winner
964+
is_winner = idx === 0;
965+
}
966+
}
967+
968+
if (is_winner) {
969+
placement.response = response;
970+
const node_convert = document.createElement("div");
971+
node_convert.innerHTML = response.html;
972+
return node_convert.firstChild;
957973
} else {
958-
is_winner = idx === 0;
974+
return null;
959975
}
960-
}
976+
});
961977

962-
if (is_winner) {
963-
placement.response = response;
964-
const node_convert = document.createElement("div");
965-
node_convert.innerHTML = response.html;
966-
return node_convert.firstChild;
967-
} else {
978+
return placement.load(fetchPromise).catch((err) => {
979+
if (err instanceof EthicalAdsWarning) {
980+
logger.warn(err.message);
981+
} else {
982+
logger.error(err.message);
983+
}
968984
return null;
969-
}
970-
});
971-
});
985+
});
986+
})
987+
);
972988
}
973989

974990
/* Sends the view time of the ad to the server
@@ -1191,15 +1207,10 @@ export function load_placements(force_load = false) {
11911207
}
11921208

11931209
// Group prioritized placements
1194-
let has_priority = false;
1195-
placements.forEach((placement) => {
1196-
if (placement && placement.priority !== null) {
1197-
has_priority = true;
1198-
}
1199-
});
1210+
const has_priority = placements.some((p) => p && p.priority !== null);
12001211

1201-
let priority_group = [];
12021212
if (has_priority) {
1213+
let priority_group = [];
12031214
placements.forEach((placement) => {
12041215
if (placement && (force_load || !placement.load_manually)) {
12051216
if (placement.priority === null) {
@@ -1209,30 +1220,32 @@ export function load_placements(force_load = false) {
12091220
priority_group.push(placement);
12101221
}
12111222
});
1212-
}
1213-
1214-
if (priority_group.length > 0) {
1215-
Placement.fetchGroup(priority_group);
1216-
}
12171223

1218-
// Create main promise. Iterator `all()` Promise will surround array of
1219-
// placements.
1220-
return Promise.all(
1221-
placements.map((placement) => {
1222-
if (placement && (force_load || !placement.load_manually)) {
1223-
return placement.load().catch((err) => {
1224-
if (err instanceof EthicalAdsWarning) {
1225-
logger.warn(err.message);
1226-
} else {
1227-
logger.error(err.message);
1228-
}
1224+
if (priority_group.length > 0) {
1225+
return Placement.loadGroup(priority_group);
1226+
} else {
1227+
return Promise.resolve([]);
1228+
}
1229+
} else {
1230+
// Create main promise. Iterator `all()` Promise will surround array of
1231+
// placements.
1232+
return Promise.all(
1233+
placements.map((placement) => {
1234+
if (placement && (force_load || !placement.load_manually)) {
1235+
return placement.load().catch((err) => {
1236+
if (err instanceof EthicalAdsWarning) {
1237+
logger.warn(err.message);
1238+
} else {
1239+
logger.error(err.message);
1240+
}
1241+
return null;
1242+
});
1243+
} else {
12291244
return null;
1230-
});
1231-
} else {
1232-
return null;
1233-
}
1234-
})
1235-
);
1245+
}
1246+
})
1247+
);
1248+
}
12361249
}
12371250

12381251
export function unload_placements() {

‎tests/priority-placement.test.html‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,27 @@
1919
// Mock the regular fetch
2020
let fetchStub = mockAdDecision();
2121

22-
// Mock the fetchGroup for ad1, ad2, and ad3
22+
// Mock the loadGroup for ad1, ad2, and ad3
2323
let groupStub = sinon
24-
.stub(Placement, "fetchGroup")
24+
.stub(Placement, "loadGroup")
2525
.callsFake((placements) => {
2626
// Let's declare the second one as the winner, as if its priority won
2727
let winner = placements.find((p) => p.target.id === "ad2");
28-
placements.forEach((p) => {
29-
p.fetchPromise = Promise.resolve().then(() => {
30-
if (p === winner) {
31-
const response_html =
32-
"<div class='ad-rendered'><!-- A real ad would be here normally --></div>";
33-
const elem_placement = document.createElement("div");
34-
elem_placement.innerHTML = response_html;
35-
return elem_placement.firstChild;
36-
}
37-
return null;
38-
});
39-
});
28+
return Promise.all(
29+
placements.map((p) => {
30+
const fetchPromise = Promise.resolve().then(() => {
31+
if (p === winner) {
32+
const response_html =
33+
"<div class='ad-rendered'><!-- A real ad would be here normally --></div>";
34+
const elem_placement = document.createElement("div");
35+
elem_placement.innerHTML = response_html;
36+
return elem_placement.firstChild;
37+
}
38+
return null;
39+
});
40+
return p.load(fetchPromise).catch(() => null);
41+
})
42+
);
4043
});
4144

4245
runTests(async () => {

0 commit comments

Comments
 (0)