Phuedx has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/327194 )
Change subject: Redefine when Page Previews are enabled
......................................................................
Redefine when Page Previews are enabled
The Page Previews A/B test was disabled in T144490. After the changes
for that task had been deployed we discovered that the old definition
for when previews are enabled was invalid.
Previews are enabled:
* If the beta feature is enabled and the user has enabled the beta
feature.
* The user hasn't disabled previews via the settings modal.
* And soon, if the beta feature is disabled and the user hasn't disabled
previews via their user preferences (see T146889).
Changes:
* mw.popups#createExperiment -> mw.popups#isEnabled
* Make mw.popups#isEnabled act like a factory to maintain backwards
compatibility.
* Update the associated tests.
Bug: T152687
Change-Id: I713a90dfb29866b27738e7d19e8a22518a12d417
---
M extension.json
M resources/ext.popups/boot.js
D resources/ext.popups/experiment.js
A resources/ext.popups/isEnabled.js
D tests/qunit/ext.popups/experiment.test.js
A tests/qunit/ext.popups/isEnabled.test.js
6 files changed, 81 insertions(+), 144 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/94/327194/1
diff --git a/extension.json b/extension.json
index 52b8d92..777cb03 100644
--- a/extension.json
+++ b/extension.json
@@ -60,7 +60,7 @@
"resources/ext.popups/index.js",
"resources/ext.popups/wait.js",
"resources/ext.popups/userSettings.js",
- "resources/ext.popups/experiment.js",
+ "resources/ext.popups/isEnabled.js",
"resources/ext.popups/actions.js",
"resources/ext.popups/processLinks.js",
"resources/ext.popups/counts.js",
diff --git a/resources/ext.popups/boot.js b/resources/ext.popups/boot.js
index 4767055..c171953 100644
--- a/resources/ext.popups/boot.js
+++ b/resources/ext.popups/boot.js
@@ -87,13 +87,14 @@
gateway = createGateway(),
userSettings,
settingsDialog,
- isUserInCondition,
+ isEnabled,
schema;
userSettings = mw.popups.createUserSettings( mw.storage,
mw.user );
settingsDialog = mw.popups.createSettingsDialogRenderer();
- isUserInCondition = mw.popups.createExperiment( mw.config,
mw.user, userSettings );
schema = mw.popups.createSchema( mw.config, window );
+
+ isEnabled = mw.popups.isEnabled( mw.user, userSettings );
// If debug mode is enabled, then enable Redux DevTools.
if ( mw.config.get( 'debug' ) === true ) {
@@ -110,7 +111,7 @@
registerChangeListeners( store, actions, schema, userSettings,
settingsDialog );
actions.boot(
- isUserInCondition,
+ isEnabled,
mw.user,
userSettings,
generateToken,
diff --git a/resources/ext.popups/experiment.js
b/resources/ext.popups/experiment.js
deleted file mode 100644
index 52c1c1e..0000000
--- a/resources/ext.popups/experiment.js
+++ /dev/null
@@ -1,49 +0,0 @@
-( function ( mw ) {
-
- /**
- * Given the global state of the application, creates a function that
gets
- * whether or not the user should have Page Previews enabled, i.e.
whether
- * they are in the experiment condition.
- *
- * The user is in the experiment condition if:
- * * They've enabled by Page Previews via UI interactions.
- * * They've enabled Page Previews as a beta feature.
- * * They aren't in the control bucket of the experiment.
- *
- * @param {mw.Map} config
- * @param {mw.user} user The `mw.user` singleton instance
- * @param {Object} userSettings An object returned by
- * `mw.popups.createUserSettings`, from which the user's token will be
- * retrieved
- *
- * @return {Function}
- */
- mw.popups.createExperiment = function ( config, user, userSettings ) {
- return function () {
- var experimentConfig = config.get(
'wgPopupsExperimentConfig' );
-
- if ( userSettings.hasIsEnabled() ) {
- return userSettings.getIsEnabled();
- }
-
- if ( config.get( 'wgPopupsExperiment', false ) ===
false ) {
- return false;
- }
-
- if ( user.isAnon() ) {
- if ( !experimentConfig ) {
- return false;
- }
-
- // FIXME: mw.experiments should expose the
CONTROL_BUCKET constant, e.g.
- // `mw.experiments.CONTROL_BUCKET`.
- return mw.experiments.getBucket(
experimentConfig, userSettings.getToken() ) !== 'control';
- } else {
- // Logged in users are in condition depending
on the beta feature flag
- // instead of bucketing
- return config.get(
'wgPopupsExperimentIsBetaFeatureEnabled', false );
- }
- };
- };
-
-}( mediaWiki ) );
diff --git a/resources/ext.popups/isEnabled.js
b/resources/ext.popups/isEnabled.js
new file mode 100644
index 0000000..a82f3ab
--- /dev/null
+++ b/resources/ext.popups/isEnabled.js
@@ -0,0 +1,33 @@
+( function ( mw ) {
+
+ /**
+ * Given the global state of the application, creates a function that
gets
+ * whether or not the user should have Page Previews enabled.
+ *
+ * The user has previews enabled if:
+ * * The beta feature is available (see `$wgPopupsBetaFeature`) and
they've
+ * enabled the beta feature.
+ * * They *haven't disabled* it via the settings modal.
+ *
+ * The first case covers the enabled by default case: if
+ * `$wgPopupsBetaFeature` is `false` and the user hasn't disabled
previews via
+ * their preferences, then previews are enabled.
+ *
+ * @param {mw.user} user The `mw.user` singleton instance
+ * @param {Object} userSettings An object returned by
+ * `mw.popups.createUserSettings`
+ *
+ * @return {Function}
+ */
+ mw.popups.isEnabled = function ( user, userSettings ) {
+ return function () {
+ if ( user.isAnon() ) {
+ return false;
+ }
+
+ return !userSettings.hasIsEnabled() ||
+ ( userSettings.hasIsEnabled() &&
userSettings.getIsEnabled() );
+ };
+ };
+
+}( mediaWiki ) );
diff --git a/tests/qunit/ext.popups/experiment.test.js
b/tests/qunit/ext.popups/experiment.test.js
deleted file mode 100644
index 793da46..0000000
--- a/tests/qunit/ext.popups/experiment.test.js
+++ /dev/null
@@ -1,91 +0,0 @@
-( function ( mw ) {
-
- function createStubUserSettings( hasIsEnabled ) {
- return {
- getToken: function () {
- return '1234567890';
- },
- hasIsEnabled: function () {
- return hasIsEnabled;
- },
- getIsEnabled: function () {
- return true;
- }
- };
- }
-
- QUnit.module( 'ext.popups/experiment', {
- setup: function () {
- this.config = new mw.Map();
- this.user = mw.popups.tests.stubs.createStubUser( /*
isAnon = */ true );
- this.userSettings = createStubUserSettings( /*
hasIsEnabled = */ false );
- this.isUserInCondition = mw.popups.createExperiment(
this.config, this.user, this.userSettings );
- }
- } );
-
- QUnit.test( 'is should return false if the experiment is disabled', 1,
function ( assert ) {
- this.config.set( 'wgPopupsExperiment', false );
-
- assert.notOk( this.isUserInCondition() );
- } );
-
- QUnit.test( '#isUserInCondition', 2, function ( assert ) {
- var user = mw.popups.tests.stubs.createStubUser( /* isAnon = */
false ),
- isUserInCondition = mw.popups.createExperiment(
this.config, user, this.userSettings );
-
- this.config.set( {
- wgPopupsExperiment: true,
- wgPopupsExperimentIsBetaFeatureEnabled: true
- } );
-
- assert.ok(
- isUserInCondition(),
- 'If the user has the beta feature enabled, then they
are in the condition.'
- );
-
- // ---
-
- this.config.set( 'wgPopupsExperimentIsBetaFeatureEnabled',
false );
-
- assert.notOk(
- isUserInCondition(),
- 'If the user has the beta feature disabled, then they
aren\'t in the condition.'
- );
- } );
-
- QUnit.test( 'it should bucket the user', 2, function ( assert ) {
- var experimentConfig = {
- name: 'Popups A/B Test - May, 2016',
- enabled: true,
- buckets: {
- control: 0.5,
- A: 0.5
- }
- },
- getBucketSpy = this.sandbox.stub( mw.experiments,
'getBucket' ).returns( 'A' );
-
- this.config.set( {
- wgPopupsExperiment: true,
- wgPopupsExperimentConfig: experimentConfig
- } );
-
- assert.ok( this.isUserInCondition() );
- assert.deepEqual( getBucketSpy.firstCall.args[0],
experimentConfig );
- } );
-
- QUnit.test( 'it should return false if the experiment isn\'t
configured', 1, function ( assert ) {
- this.config.set( 'wgPopupsExperiment', true );
-
- assert.notOk( this.isUserInCondition() );
- } );
-
- QUnit.test( 'it shouldn\'t bucket the user if they have enabled or
disabled Page Previews', 2, function ( assert ) {
- var userSettings = createStubUserSettings( /* hasIsEnabled = */
true ),
- getIsEnabledSpy = this.sandbox.spy( userSettings,
'getIsEnabled' ),
- isUserInCondition = mw.popups.createExperiment(
this.config, this.user, userSettings );
-
- assert.ok( isUserInCondition() );
- assert.strictEqual( 1, getIsEnabledSpy.callCount );
- } );
-
-}( mediaWiki ) );
diff --git a/tests/qunit/ext.popups/isEnabled.test.js
b/tests/qunit/ext.popups/isEnabled.test.js
new file mode 100644
index 0000000..11b8cf7
--- /dev/null
+++ b/tests/qunit/ext.popups/isEnabled.test.js
@@ -0,0 +1,43 @@
+( function ( mw ) {
+
+ function createStubUserSettings( hasIsEnabled ) {
+ return {
+ hasIsEnabled: function () {
+ return hasIsEnabled;
+ },
+ getIsEnabled: function () {
+ return true;
+ }
+ };
+ }
+
+ QUnit.module( 'ext.popups#isEnabled' );
+
+ QUnit.test( 'it should return true when the user has enabled it via UI
interactions', function ( assert ) {
+ var user = mw.popups.tests.stubs.createStubUser( /* isAnon = */
false ),
+ userSettings = createStubUserSettings( /* hasIsEnabled
= */ true ),
+ isEnabled = mw.popups.isEnabled( user, userSettings );
+
+ assert.ok( isEnabled() );
+ } );
+
+ QUnit.test( 'it should return false if the user is an anon', function (
assert ) {
+ var user = mw.popups.tests.stubs.createStubUser( /* isAnon = */
true ),
+ userSettings = createStubUserSettings( /* hasIsEnabled
= */ true ),
+ isEnabled = mw.popups.isEnabled( user, userSettings );
+
+ assert.notOk(
+ isEnabled(),
+ 'It should return false even if the user has enabled it
via UI interactions.'
+ );
+ } );
+
+ QUnit.test( 'it should return true if the user hasn\'t disabled it via
UI interactions', function ( assert ) {
+ var user = mw.popups.tests.stubs.createStubUser( /* isAnon = */
false ),
+ userSettings = createStubUserSettings( /* hasIsEnabled
= */ false ),
+ isEnabled = mw.popups.isEnabled( user, userSettings );
+
+ assert.ok( isEnabled() );
+ } );
+
+}( mediaWiki ) );
--
To view, visit https://gerrit.wikimedia.org/r/327194
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I713a90dfb29866b27738e7d19e8a22518a12d417
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Phuedx <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
