jenkins-bot has submitted this change and it was merged.
Change subject: Merge mw.popups.experiment into mw.popups.core
......................................................................
Merge mw.popups.experiment into mw.popups.core
ext.popups.experiment depends on .core as it initialized the mw.popups
namespace and .core depends on .experiments for
mw.popups#getEnabledState.
By merging the experiment module into core, we can eliminate any
circular dependencies.
Changes:
* Move ext.popups.experiment.js code into ext.popups.core.js
* Remove mw.popups.experiment module and any references to it
Note: ext.popups.experiment.test.js was left in its own file for cleaner
QUnit module setups and easier removal later. I'm not entirely happy
with doing it this way, but I'm not sure changing the mw.config within
the mw.popups.core QUnit module is worth merging the files.
Bug: T146035
Change-Id: I1f024567010acaa61c1d613c6e59c998198a5976
---
M Popups.hooks.php
M extension.json
M resources/ext.popups.core.js
D resources/ext.popups.experiment.js
M tests/qunit/ext.popups.core.test.js
M tests/qunit/ext.popups.experiment.test.js
6 files changed, 96 insertions(+), 118 deletions(-)
Approvals:
Jdlrobson: Looks good to me, approved
Bmansurov: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/Popups.hooks.php b/Popups.hooks.php
index 196c742..a0466f0 100644
--- a/Popups.hooks.php
+++ b/Popups.hooks.php
@@ -238,7 +238,6 @@
),
'dependencies' => array(
'ext.popups.desktop',
- 'ext.popups.experiment',
'ext.popups.schemaPopups.utils'
),
'localBasePath' => __DIR__,
diff --git a/extension.json b/extension.json
index ca47486..f355eb6 100644
--- a/extension.json
+++ b/extension.json
@@ -68,7 +68,9 @@
"mediawiki.Title",
"mediawiki.Uri",
"mediawiki.RegExp",
- "mediawiki.storage"
+ "mediawiki.storage",
+ "mediawiki.user",
+ "mediawiki.experiments"
],
"targets": [
"desktop",
@@ -86,8 +88,7 @@
"mediawiki.storage",
"jquery.client",
"ext.popups.core",
- "ext.popups.renderer.desktopRenderer",
- "ext.popups.experiment"
+ "ext.popups.renderer.desktopRenderer"
],
"targets": [ "desktop" ]
},
@@ -105,20 +106,6 @@
],
"dependencies": [
"ext.popups.core"
- ]
- },
- "ext.popups.experiment": {
- "scripts": [
- "resources/ext.popups.experiment.js"
- ],
- "dependencies": [
- "ext.popups.core",
- "mediawiki.user",
- "mediawiki.storage",
- "mediawiki.experiments"
- ],
- "targets": [
- "desktop"
]
},
"ext.popups.schemaPopups.utils": {
diff --git a/resources/ext.popups.core.js b/resources/ext.popups.core.js
index 28a941b..20a2054 100644
--- a/resources/ext.popups.core.js
+++ b/resources/ext.popups.core.js
@@ -237,7 +237,90 @@
if ( !mw.config.get( 'wgPopupsExperiment', false ) ) {
return mw.storage.get( popupsEnabledStorageKey ) !==
'0';
} else {
- return mw.popups.experiment.isUserInCondition();
+ return this.isUserInCondition();
+ }
+ };
+
+ /**
+ * @ignore
+ */
+ function getToken() {
+ var key = 'PopupsExperimentID',
+ id = mw.storage.get( key );
+
+ if ( !id ) {
+ id = mw.user.generateRandomSessionId();
+
+ mw.storage.set( key, id );
+ }
+
+ return id;
+ }
+
+ /**
+ * Has the user previously enabled Popups by clicking "Enable previews"
in the
+ * footer menu?
+ *
+ * @return {boolean}
+ * @ignore
+ */
+ function hasUserEnabledFeature() {
+ var value = mw.storage.get( 'mwe-popups-enabled' );
+
+ return Boolean( value ) && value !== '0';
+ }
+
+ /**
+ * Has the user previously disabled Popups by clicking "Disable
previews" in the settings
+ * overlay?
+ *
+ * @return {boolean}
+ * @ignore
+ */
+ function hasUserDisabledFeature() {
+ return mw.storage.get( 'mwe-popups-enabled' ) === '0';
+ }
+
+ /**
+ * Gets whether or not the user has Popups enabled, i.e. whether they
are in the experiment
+ * condition.
+ *
+ * The user is in the experiment condition if:
+ * * they've enabled Popups by click "Enable previews" in the footer
menu, or
+ * * they've enabled Popups as a beta feature, or
+ * * they aren't in the control bucket of the experiment
+ *
+ * N.B. that the user isn't entered into the experiment, i.e. they
aren't assigned or a bucket,
+ * if the experiment isn't configured.
+ *
+ * @return {boolean}
+ * @ignore
+ */
+ mw.popups.isUserInCondition = function isUserInCondition() {
+ var config = mw.config.get( 'wgPopupsExperimentConfig' );
+
+ // The first two tests deal with whether the user has
/explicitly/ enable or disabled via its
+ // settings.
+ if ( hasUserEnabledFeature() ) {
+ return true;
+ }
+
+ if ( hasUserDisabledFeature() ) {
+ return false;
+ }
+
+ if ( mw.user.isAnon() ) {
+ if ( !config ) {
+ return false;
+ }
+
+ // FIXME: mw.experiments should expose the
CONTROL_BUCKET constant, e.g.
+ // `mw.experiments.CONTROL_BUCKET`.
+ return mw.experiments.getBucket( config, getToken() )
!== 'control';
+ } else {
+ // Logged in users are in condition depending on the
beta feature flag
+ // instead of bucketing
+ return mw.config.get(
'wgPopupsExperimentIsBetaFeatureEnabled', false );
}
};
diff --git a/resources/ext.popups.experiment.js
b/resources/ext.popups.experiment.js
deleted file mode 100644
index 990fa17..0000000
--- a/resources/ext.popups.experiment.js
+++ /dev/null
@@ -1,91 +0,0 @@
-( function ( mw ) {
-
- /**
- * @ignore
- */
- function getToken() {
- var key = 'PopupsExperimentID',
- id = mw.storage.get( key );
-
- if ( !id ) {
- id = mw.user.generateRandomSessionId();
-
- mw.storage.set( key, id );
- }
-
- return id;
- }
-
- /**
- * Has the user previously enabled Popups by clicking "Enable previews"
in the
- * footer menu?
- *
- * @return {boolean}
- * @ignore
- */
- function hasUserEnabledFeature() {
- var value = mw.storage.get( 'mwe-popups-enabled' );
-
- return Boolean( value ) && value !== '0';
- }
-
- /**
- * Has the user previously disabled Popups by clicking "Disable
previews" in the settings
- * overlay?
- *
- * @return {boolean}
- * @ignore
- */
- function hasUserDisabledFeature() {
- return mw.storage.get( 'mwe-popups-enabled' ) === '0';
- }
-
- /**
- * @class mw.popups.experiment
- * @singleton
- */
- mw.popups.experiment = {};
-
- /**
- * Gets whether or not the user has Popups enabled, i.e. whether they
are in the experiment
- * condition.
- *
- * The user is in the experiment condition if:
- * * they've enabled Popups by click "Enable previews" in the footer
menu, or
- * * they've enabled Popups as a beta feature, or
- * * they aren't in the control bucket of the experiment
- *
- * N.B. that the user isn't entered into the experiment, i.e. they
aren't assigned or a bucket,
- * if the experiment isn't configured.
- *
- * @return {boolean}
- */
- mw.popups.experiment.isUserInCondition = function isUserInCondition() {
- var config = mw.config.get( 'wgPopupsExperimentConfig' );
-
- // The first two tests deal with whether the user has
/explicitly/ enable or disabled via its
- // settings.
- if ( hasUserEnabledFeature() ) {
- return true;
- }
-
- if ( hasUserDisabledFeature() ) {
- return false;
- }
-
- if ( mw.user.isAnon() ) {
- if ( !config ) {
- return false;
- }
-
- // FIXME: mw.experiments should expose the
CONTROL_BUCKET constant, e.g.
- // `mw.experiments.CONTROL_BUCKET`.
- return mw.experiments.getBucket( config, getToken() )
!== 'control';
- } else {
- // Logged in users are in condition depending on the
beta feature flag
- // instead of bucketing
- return mw.config.get(
'wgPopupsExperimentIsBetaFeatureEnabled', false );
- }
- };
-
-}( mediaWiki ) );
diff --git a/tests/qunit/ext.popups.core.test.js
b/tests/qunit/ext.popups.core.test.js
index 159b74c..360c2ee 100644
--- a/tests/qunit/ext.popups.core.test.js
+++ b/tests/qunit/ext.popups.core.test.js
@@ -312,7 +312,7 @@
.withArgs( 'wgPopupsExperiment' ),
deviceStorageStub = this.sandbox.stub( mw.storage,
'get' )
.withArgs( storageKey ),
- experimentStub = this.sandbox.stub(
mw.popups.experiment,
+ experimentStub = this.sandbox.stub( mw.popups,
'isUserInCondition' );
QUnit.expect( 7 );
diff --git a/tests/qunit/ext.popups.experiment.test.js
b/tests/qunit/ext.popups.experiment.test.js
index 2e52b81..c298b9b 100644
--- a/tests/qunit/ext.popups.experiment.test.js
+++ b/tests/qunit/ext.popups.experiment.test.js
@@ -1,6 +1,6 @@
( function ( mw ) {
- QUnit.module( 'ext.popups.experiment', QUnit.newMwEnvironment( {
+ QUnit.module( 'ext.popups.core:experiment', QUnit.newMwEnvironment( {
config: {
wgPopupsExperimentConfig: {
name: 'Popups A/B Test - May, 2016',
@@ -31,7 +31,7 @@
mw.config.set( 'wgPopupsExperimentIsBetaFeatureEnabled', true );
assert.strictEqual(
- mw.popups.experiment.isUserInCondition(),
+ mw.popups.isUserInCondition(),
true,
'If the user has the beta feature enabled, then they
aren\'t in the condition.'
);
@@ -43,7 +43,7 @@
result,
firstCallArgs;
- result = mw.popups.experiment.isUserInCondition();
+ result = mw.popups.isUserInCondition();
firstCallArgs = getBucketSpy.firstCall.args;
@@ -66,7 +66,7 @@
this.sandbox.stub( mw.user, 'generateRandomSessionId'
).returns( token );
- mw.popups.experiment.isUserInCondition();
+ mw.popups.isUserInCondition();
assert.deepEqual(
setSpy.firstCall.args[ 1 ],
@@ -79,7 +79,7 @@
mw.config.set( 'wgPopupsExperimentConfig', null );
assert.strictEqual(
- mw.popups.experiment.isUserInCondition(),
+ mw.popups.isUserInCondition(),
false,
'If the experiment isn\'t configured, then the user
isn\'t in the condition.'
);
@@ -89,7 +89,7 @@
mw.storage.set( 'mwe-popups-enabled', '1' );
assert.strictEqual(
- mw.popups.experiment.isUserInCondition(),
+ mw.popups.isUserInCondition(),
true,
'If the experiment has enabled the feature, then the
user is in the condition.'
);
@@ -102,7 +102,7 @@
mw.storage.set( 'mwe-popups-enabled', '0' );
assert.strictEqual(
- mw.popups.experiment.isUserInCondition(),
+ mw.popups.isUserInCondition(),
false,
'If the experiment has enabled the feature, then the
user is in the condition.'
);
--
To view, visit https://gerrit.wikimedia.org/r/311977
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1f024567010acaa61c1d613c6e59c998198a5976
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Phuedx <g...@samsmith.io>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Jhobs <jhob...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
