jenkins-bot has submitted this change and it was merged.
Change subject: Simplify opt-in cookies
......................................................................
Simplify opt-in cookies
* Don't use two cookies where you can use one
* Simplifies accessors greatly
Change-Id: I4db3329d36ee3e5d3af276dae34fc38b4b1f70be
---
M includes/MobileContext.php
M includes/specials/SpecialMobileOptions.php
M tests/MobileContextTest.php
3 files changed, 101 insertions(+), 113 deletions(-)
Approvals:
Kaldari: Looks good to me, approved
JGonera: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 934a957..a9c5b64 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -1,8 +1,7 @@
<?php
class MobileContext extends ContextSource {
- protected $betaGroupMember;
- protected $alphaGroupMember;
+ protected $mobileMode;
protected $contentFormat = '';
protected $useFormatCookieName;
protected $disableImages;
@@ -183,32 +182,70 @@
return true;
}
- public function isAlphaGroupMember() {
- if ( is_null( $this->alphaGroupMember ) ) {
- $this->checkUserStatus();
- if ( $this->getMobileAction() == 'alpha' ) {
- $this->setAlphaGroupMember( true );
+ /**
+ * Returns the testing mode user has opted in: 'alpha', 'beta' or any
other value for stable
+ * @return string
+ */
+ protected function getMobileMode() {
+ if ( is_null( $this->mobileMode ) ) {
+ $mobileAction = $this->getMobileAction();
+ if ( $mobileAction === 'alpha' || $mobileAction ===
'beta' ) {
+ $this->mobileMode = $mobileAction;
+ } else {
+ // First check old cookie
+ // @todo: Remove in September when old cookies
expire
+ $req = $this->getRequest();
+ $alpha = $req->getCookie( 'mf_alpha', '' );
+ if ( $alpha == 1 ) {
+ $req->response()->setcookie(
'mf_alpha', '', 0, '' );
+ $this->setMobileMode( 'alpha' );
+ } else {
+ $this->mobileMode =
$this->getRequest()->getCookie( 'optin', '' );
+ // Old cookie format - handle it but no
point in overwriting the cookie
+ if ( $this->mobileMode == '1' ) {
+ $this->mobileMode = 'beta';
+ }
+ }
}
}
- return $this->alphaGroupMember;
+ return $this->mobileMode;
}
- public function setAlphaGroupMember( $value ) {
- $this->alphaGroupMember = $value;
+ /**
+ * Sets testing group membership, both cookie and this class variables
+ * @param string $mode: Mode to set
+ */
+ public function setMobileMode( $mode ) {
+ wfProfileIn( __METHOD__ );
+ if ( $mode !== 'alpha' && $mode !== 'beta' ) {
+ $mode = '';
+ }
+ // Update statistics
+ if ( $mode === 'alpha' && !is_null( $this->mobileMode ) ) {
+ wfIncrStats( 'mobile.alpha.opt_in_cookie_set' );
+ }
+ if ( $mode === 'beta' ) {
+ if ( $this->mobileMode === 'alpha' ) {
+ wfIncrStats( 'mobile.alpha.opt_in_cookie_unset'
);
+ } else {
+ wfIncrStats( 'mobile.opt_in_cookie_set' );
+ }
+ }
+ if ( !$mode ) {
+ wfIncrStats( 'mobile.opt_in_cookie_unset' );
+ }
+ $this->mobileMode = $mode;
+ $this->getRequest()->response()->setcookie( 'optin', $mode, 0,
'', $this->getBaseDomain() );
+ wfProfileOut( __METHOD__ );
+ }
+
+ public function isAlphaGroupMember() {
+ return $this->getMobileMode() === 'alpha';
}
public function isBetaGroupMember() {
- if ( is_null( $this->betaGroupMember ) ) {
- $this->checkUserStatus();
- if ( $this->getMobileAction() == 'beta' ) {
- $this->setBetaGroupMember( true );
- }
- }
- return $this->betaGroupMember;
- }
-
- public function setBetaGroupMember( $value ) {
- $this->betaGroupMember = $value;
+ $mode = $this->getMobileMode();
+ return $mode === 'beta' || $mode === 'alpha';
}
public function getMobileToken() {
@@ -433,88 +470,6 @@
$useFormatFromCookie = $this->getRequest()->getCookie(
$this->getUseFormatCookieName(), '' );
return $useFormatFromCookie;
- }
-
- public function checkUserStatus() {
- wfProfileIn( __METHOD__ );
-
- $optInCookie = $this->getOptInOutCookie();
- $alpha = $this->getAlphaOptInOutCookie();
- if ( !empty( $optInCookie ) &&
- $optInCookie == 1 ) {
- $this->betaGroupMember = true;
- }
- if ( !empty( $alpha ) &&
- $alpha == 1 ) {
- $this->setAlphaGroupMember( true );
- } else {
- $this->setAlphaGroupMember( false );
- }
- wfProfileOut( __METHOD__ );
- return true;
- }
-
- /**
- * @param $value bool
- * @return bool
- */
- public function setOptInOutCookie( $value ) {
- global $wgCookieDomain, $wgCookiePrefix;
- wfProfileIn( __METHOD__ );
- if ( $value ) {
- wfIncrStats( 'mobile.opt_in_cookie_set' );
- }
- $tempWgCookieDomain = $wgCookieDomain;
- $wgCookieDomain = $this->getBaseDomain();
- $tempWgCookiePrefix = $wgCookiePrefix;
- $wgCookiePrefix = '';
- $this->getRequest()->response()->setcookie( 'optin', $value ?
'1' : '', 0, '' );
- $wgCookieDomain = $tempWgCookieDomain;
- $wgCookiePrefix = $tempWgCookiePrefix;
- wfProfileOut( __METHOD__ );
- return true;
- }
-
- /**
- * @param bool $value
- * @param bool $disabled
- *
- * @return bool
- */
- public function setAlphaOptInOutCookie( $value, $disabled = false ) {
- wfProfileIn( __METHOD__ );
- // track opt ins
- if ( $value ) {
- wfIncrStats( 'mobile.alpha.opt_in_cookie_set' );
- }
- // track opt outs
- if ( $disabled ) {
- wfIncrStats( 'mobie.alpha.opt_in_cookie_unset' );
- }
- $cookieDomain = $this->getBaseDomain();
- $this->getRequest()->response()->setcookie( 'mf_alpha', $value
? '1' : '', 0, '', $cookieDomain );
- wfProfileOut( __METHOD__ );
- return true;
- }
-
- /**
- * @return Mixed
- */
- private function getAlphaOptInOutCookie() {
- wfProfileIn( __METHOD__ );
- $optInCookie = $this->getRequest()->getCookie( 'mf_alpha', '' );
- wfProfileOut( __METHOD__ );
- return $optInCookie;
- }
-
- /**
- * @return Mixed
- */
- private function getOptInOutCookie() {
- wfProfileIn( __METHOD__ );
- $optInCookie = $this->getRequest()->getCookie( 'optin', '' );
- wfProfileOut( __METHOD__ );
- return $optInCookie;
}
/**
diff --git a/includes/specials/SpecialMobileOptions.php
b/includes/specials/SpecialMobileOptions.php
index 06a06d2..13d885d 100644
--- a/includes/specials/SpecialMobileOptions.php
+++ b/includes/specials/SpecialMobileOptions.php
@@ -194,20 +194,16 @@
wfDebug( __METHOD__ . "(): token mismatch\n" );
//return; // Display something here?
}
- $inBeta = $request->getBool( 'enableBeta' );
- $inAlpha = $request->getBool( 'enableAlpha' );
- // determine whether or not alpha was disabled
- if ( $context->isAlphaGroupMember() && !$inAlpha ) {
- $alphaDisabled = true;
+ if ( $request->getBool( 'enableAlpha' ) ) {
+ $group = 'alpha';
+ } elseif ( $request->getBool( 'enableBeta' ) ) {
+ $group = 'beta';
} else {
- $alphaDisabled = false;
+ $group = '';
}
+ $context->setMobileMode( $group );
$imagesDisabled = !$request->getBool( 'enableImages' );
$context->setDisableImagesCookie( $imagesDisabled );
- $context->setOptInOutCookie( $inBeta ? '1' : '' );
- $context->setBetaGroupMember( $inBeta );
- $context->setAlphaOptInOutCookie( $inAlpha ? '1' : '',
$alphaDisabled );
- $context->setAlphaGroupMember( $inAlpha );
$returnToTitle = Title::newFromText( $request->getText(
'returnto' ) );
if ( $returnToTitle ) {
diff --git a/tests/MobileContextTest.php b/tests/MobileContextTest.php
index 47604cd..9ebcec0 100644
--- a/tests/MobileContextTest.php
+++ b/tests/MobileContextTest.php
@@ -408,4 +408,41 @@
);
}
+ /**
+ * @dataProvider optInProvider
+ */
+ public function testOptIn( array $cookies, $isAlpha, $isBeta ) {
+ $ctx = new RequestContext();
+ $ctx->setRequest( new MFFauxRequest( $cookies ) );
+ MobileContext::setInstance( null );
+ $mobileContext = MobileContext::singleton();
+ $mobileContext->setContext( $ctx );
+ $this->assertEquals( $isAlpha,
$mobileContext->isAlphaGroupMember() );
+ $this->assertEquals( $isBeta,
$mobileContext->isBetaGroupMember() );
+ }
+
+ public function optInProvider() {
+ return array(
+ array( array(), false, false ),
+ array( array( 'optin' => '1' ), false, true ),
+ array( array( 'optin' => 'beta' ), false, true ),
+ array( array( 'optin' => 'alpha' ), true, true ),
+ array( array( 'optin' => 'foobar' ), false, false ),
+ array( array( 'optin' => '1', 'mf_alpha' => '1' ),
true, true ),
+ array( array( 'mf_alpha' => '1' ), true, true ),
+ array( array( 'mf_alpha' => 'foobar' ), false, false ),
+ );
+ }
+}
+
+class MFFauxRequest extends FauxRequest {
+ private $cookies;
+
+ public function __construct( array $cookies ) {
+ $this->cookies = $cookies;
+ }
+
+ public function getCookie( $key, $prefix = null, $default = null ) {
+ return isset( $this->cookies[$key] ) ? $this->cookies[$key] :
$default;
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/74667
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4db3329d36ee3e5d3af276dae34fc38b4b1f70be
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
Gerrit-Reviewer: JGonera <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: awjrichards <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
