jenkins-bot has submitted this change and it was merged.
Change subject: Improve impression diet state machine
......................................................................
Improve impression diet state machine
This fixes two bugs which were causing extra waitdate hides.
* Even when readers have a waitUntil set, increasing the campaign maximum
impressions seen per cycle should result in the additional banners.
* Got rid of an unnecessary error condition. It's clear that every code
path in the main switch will assign a value to the hide variable.
Also gets rid of the waitnorestart state. This was equivalent to waitdate,
just check the single-shot campaign configuration to tell if waitdate is
going to restart or not.
Bug: T121178
Change-Id: I7d1c2b6672d5ef12405d1f43a36ea3f5e01da21c
---
M resources/subscribing/ext.centralNotice.display.state.js
M resources/subscribing/ext.centralNotice.impressionDiet.js
2 files changed, 47 insertions(+), 48 deletions(-)
Approvals:
AndyRussG: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/subscribing/ext.centralNotice.display.state.js
b/resources/subscribing/ext.centralNotice.display.state.js
index bee9e41..ced7b90 100644
--- a/resources/subscribing/ext.centralNotice.display.state.js
+++ b/resources/subscribing/ext.centralNotice.display.state.js
@@ -42,7 +42,7 @@
'close': 1,
'waitdate': 2,
'waitimps': 3,
- 'waiterr': 4,
+ 'waiterr': 4, // Deprecated
'belowMinEdits': 5,
'viewLimit': 6,
'seen-fullscreen': 7,
@@ -51,7 +51,7 @@
'cookies': 10,
'seen': 11,
'empty': 12,
- 'waitnorestart': 13,
+ 'waitnorestart': 13, // Deprecated
'waitnostorage': 14,
'namespace': 15
};
diff --git a/resources/subscribing/ext.centralNotice.impressionDiet.js
b/resources/subscribing/ext.centralNotice.impressionDiet.js
index 9badccc..06e4c49 100644
--- a/resources/subscribing/ext.centralNotice.impressionDiet.js
+++ b/resources/subscribing/ext.centralNotice.impressionDiet.js
@@ -52,8 +52,7 @@
mixin.setPreBannerHandler( function( mixinParams ) {
var forceFlag = mw.util.getParamValue( 'force' ),
- hide = null,
- pastDate, waitForHideImps, waitForShowImps;
+ hide;
// Only use cookies for certain campaigns on the legacy track
// Do this here since it's needed for storageAvailable() (below)
@@ -64,8 +63,8 @@
return;
}
- // Also just hide a banner if we have no way to store counts
if ( !storageAvailable() ) {
+ // Hide the banner if we have no way to store counts.
hide = 'waitnostorage';
} else {
@@ -75,61 +74,61 @@
identifier = mixinParams.cookieName;
counts = getCounts();
- // Compare counts against campaign settings and decide
whether to
- // show a banner
- pastDate = counts.waitUntil < new Date().getTime();
- waitForHideImps = counts.waitCount <
mixinParams.skipInitial;
- waitForShowImps = counts.waitSeenCount <
mixinParams.maximumSeen;
+ if ( counts.waitUntil < new Date().getTime()
+ && counts.waitSeenCount >=
mixinParams.maximumSeen
+ ) {
+ // We're beyond the wait period, and have
nothing to do except
+ // maybe start a new cycle.
- if ( !pastDate ) {
- // We're still waiting for the next cycle to
begin.
- hide = 'waitdate';
- counts.waitCount += 1;
- } else if ( pastDate && waitForHideImps ) {
- // We're still skipping initial impressions.
- hide = 'waitimps';
- counts.waitCount += 1;
- } else if ( pastDate && !waitForHideImps &&
waitForShowImps ) {
- // Show a banner!
- hide = false;
- counts.waitSeenCount += 1;
- counts.seenCount += 1;
-
- // For restartCycleDelay, 0 is a magic number
that means, never
- // restart. TODO Use a checkbox instead of a
magic number.
- if ( ( mixinParams.restartCycleDelay !== 0) &&
- ( counts.waitSeenCount >=
mixinParams.maximumSeen ) ) {
-
- // We just completed a cycle. Wait to
restart.
+ if ( mixinParams.restartCycleDelay !== 0 ) {
+ // Begin a new cycle by clearing
counters.
counts.waitCount = 0;
counts.waitSeenCount = 0;
- counts.waitUntil = new Date().getTime()
+
- ( mixinParams.restartCycleDelay
* 1000 );
}
-
- } else if ( ( mixinParams.restartCycleDelay === 0 ) &&
- ( pastDate && !waitForHideImps &&
!waitForShowImps ) ) {
-
- hide = 'waitnorestart';
}
- if ( hide === null ) {
- // All bets are off!
- hide = 'waiterr';
- counts.waitCount = 0;
- counts.waitSeenCount = 0;
- counts.waitUntil = new Date().getTime() +
- ( mixinParams.restartCycleDelay * 1000
);
- }
+ // Compare counts against campaign settings and decide
whether to
+ // show a banner
- // Bookkeeping.
- storeCounts( counts );
+ if ( counts.waitSeenCount < mixinParams.maximumSeen ) {
+ // You haven't seen the maximum count of
banners per cycle!
+
+ if ( counts.waitCount < mixinParams.skipInitial
) {
+ // Skip initial impressions.
+ hide = 'waitimps';
+ // TODO: rename skippedThisCycle
+ counts.waitCount += 1;
+ } else {
+ // Show a banner--you win!
+ hide = false;
+ }
+ } else {
+ // Wait for the next cycle to begin.
+ hide = 'waitdate';
+ }
}
- // Hide based on the results.
if ( hide ) {
+ // Hide based on the results.
cn.internal.state.cancelBanner( hide );
+ } else {
+ // Count shown impression.
+ // TODO: rename seenThisCycle
+ counts.waitSeenCount += 1;
+ counts.seenCount += 1;
+
+ // Reset the wait timer on every impression. The
configured delay
+ // is the minumum amount of time allowed between the
final impression
+ // and the start of the next cycle.
+ //
+ // TODO: rename: nextCycleAt
+
+ counts.waitUntil = new Date().getTime() +
+ ( mixinParams.restartCycleDelay * 1000 );
}
+
+ // More bookkeeping.
+ storeCounts( counts );
} );
function storageAvailable() {
--
To view, visit https://gerrit.wikimedia.org/r/258414
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7d1c2b6672d5ef12405d1f43a36ea3f5e01da21c
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Eileen <emcnaugh...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Katie Horn <kh...@wikimedia.org>
Gerrit-Reviewer: Pcoombe <pcoo...@wikimedia.org>
Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org>
Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
