Phuedx has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/312226

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
(cherry picked from commit 0b2961c3187931b6a217b4bffac5a10df21ade11)
---
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(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups 
refs/changes/26/312226/1

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/312226
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f024567010acaa61c1d613c6e59c998198a5976
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: wmf/1.28.0-wmf.20
Gerrit-Owner: Phuedx <g...@samsmith.io>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to