Pmiazga has uploaded a new change for review.
https://gerrit.wikimedia.org/r/322230
Change subject: Hygiene: use constants
......................................................................
Hygiene: use constants
For better maintenance use contants instead of strings
Changes:
- introduced constants for all cookies
- introduced constants for stable/mode
- unit tests now will also use constants
Change-Id: I67743bb5b9512578d0a4efbdd9af9d1c0f586339
---
M includes/MobileContext.php
M includes/MobileFrontend.hooks.php
M tests/phpunit/MobileContextTest.php
M tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php
4 files changed, 36 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/30/322230/1
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index c9233e3..58a4f5e 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -11,7 +11,11 @@
* Provide various request-dependant methods to use in mobile context
*/
class MobileContext extends ContextSource {
-
+ const MODE_BETA = 'beta';
+ const MODE_STABLE = 'stable';
+ const DISABLE_IMAGES_COOKIE_NAME = 'disableImages';
+ const OPTIN_COOKIE_NAME = 'optin';
+ const STOP_MOBILE_REDIRECT_COKIE_NAME = 'stopMobileRedirect';
const USEFORMAT_COOKIE_NAME = 'mf_useformat';
const USER_MODE_PREFERENCE_NAME = 'mfMode';
const LAZY_LOAD_IMAGES_COOKIE_NAME = 'mfLazyLoadImages';
@@ -261,8 +265,9 @@
public function imagesDisabled() {
if ( is_null( $this->disableImages ) ) {
$this->disableImages = (
- ( isset( $_COOKIE['disableImages'] ) &&
$_COOKIE['disableImages'] === '1' ) ||
- (bool) $this->getRequest()->getCookie(
'disableImages' )
+ ( isset( $_COOKIE[
self::DISABLE_IMAGES_COOKIE_NAME ] )
+ && $_COOKIE[ self::DISABLE_IMAGES_COOKIE_NAME
] === '1' ) ||
+ (bool) $this->getRequest()->getCookie(
self::DISABLE_IMAGES_COOKIE_NAME )
);
}
@@ -343,7 +348,7 @@
* If the cookie is not set the value will be an empty string.
*/
private function loadMobileModeCookie() {
- $this->mobileMode = $this->getRequest()->getCookie( 'optin', ''
);
+ $this->mobileMode = $this->getRequest()->getCookie(
self::OPTIN_COOKIE_NAME, '' );
}
/**
@@ -358,7 +363,7 @@
}
if ( is_null( $this->mobileMode ) ) {
$mobileAction = $this->getMobileAction();
- if ( $mobileAction === 'beta' || $mobileAction ===
'stable' ) {
+ if ( $mobileAction === self::MODE_BETA || $mobileAction
=== self::MODE_STABLE ) {
$this->mobileMode = $mobileAction;
} else {
$user = $this->getUser();
@@ -384,11 +389,11 @@
* @param string $mode Mode to set
*/
public function setMobileMode( $mode ) {
- if ( $mode !== 'beta' ) {
+ if ( $mode !== self::MODE_BETA ) {
$mode = '';
}
// Update statistics
- if ( $mode === 'beta' ) {
+ if ( $mode === self::MODE_BETA ) {
wfIncrStats( 'mobile.opt_in_cookie_set' );
}
if ( !$mode ) {
@@ -399,7 +404,7 @@
$user->setOption( self::USER_MODE_PREFERENCE_NAME, $mode );
$user->saveSettings();
- $this->getRequest()->response()->setCookie( 'optin', $mode, 0, [
+ $this->getRequest()->response()->setCookie(
self::OPTIN_COOKIE_NAME, $mode, 0, [
'prefix' => '',
'domain' => $this->getBaseDomain()
] );
@@ -410,7 +415,7 @@
* @return boolean
*/
public function isBetaGroupMember() {
- return $this->getMobileMode() === 'beta';
+ return $this->getMobileMode() === self::MODE_BETA;
}
/**
@@ -605,7 +610,8 @@
$expiry = $this->getUseFormatCookieExpiry();
}
- $this->getRequest()->response()->setcookie(
'stopMobileRedirect', 'true', $expiry,
+ $this->getRequest()->response()->setcookie(
+ self::STOP_MOBILE_REDIRECT_COKIE_NAME, 'true', $expiry,
[
'domain' =>
$this->getStopMobileRedirectCookieDomain(),
'prefix' => '',
@@ -631,7 +637,8 @@
* @return string
*/
public function getStopMobileRedirectCookie() {
- $stopMobileRedirectCookie = $this->getRequest()->getCookie(
'stopMobileRedirect', '' );
+ $stopMobileRedirectCookie = $this->getRequest()
+ ->getCookie( self::STOP_MOBILE_REDIRECT_COKIE_NAME, ''
);
return $stopMobileRedirectCookie;
}
@@ -657,9 +664,9 @@
public function setDisableImagesCookie( $shouldDisableImages ) {
$resp = $this->getRequest()->response();
if ( $shouldDisableImages ) {
- $resp->setCookie( 'disableImages', 1, 0, [ 'prefix' =>
'' ] );
+ $resp->setCookie( self::DISABLE_IMAGES_COOKIE_NAME, 1,
0, [ 'prefix' => '' ] );
} else {
- $resp->clearCookie( 'disableImages', [ 'prefix' => '' ]
);
+ $resp->clearCookie( self::DISABLE_IMAGES_COOKIE_NAME, [
'prefix' => '' ] );
}
}
diff --git a/includes/MobileFrontend.hooks.php
b/includes/MobileFrontend.hooks.php
index 1d752a4..7a9fc53 100644
--- a/includes/MobileFrontend.hooks.php
+++ b/includes/MobileFrontend.hooks.php
@@ -360,10 +360,10 @@
// Enables mobile cookies on wikis w/o mobile domain
$cookies[] = MobileContext::USEFORMAT_COOKIE_NAME;
// Don't redirect to mobile if user had explicitly opted out of
it
- $cookies[] = 'stopMobileRedirect';
+ $cookies[] = MobileContext::STOP_MOBILE_REDIRECT_COKIE_NAME;
if ( $context->shouldDisplayMobileView() || !$mobileUrlTemplate
) {
- $cookies[] = 'optin'; // beta cookie
+ $cookies[] = MobileContext::OPTIN_COOKIE_NAME; // beta
cookie
}
// Redirect people who want so from HTTP to HTTPS. Ideally,
should be
// only for HTTP but we don't vary on protocol.
@@ -746,11 +746,12 @@
$request = $context->getRequest();
// Migrate prefixed disableImages cookie to unprefixed cookie.
- if ( isset( $_COOKIE[$config->get( 'CookiePrefix' ) .
'disableImages'] ) ) {
- if ( (bool)$request->getCookie( 'disableImages' ) ) {
+ $rawCookie = $config->get( 'CookiePrefix' ) .
MobileContext::DISABLE_IMAGES_COOKIE_NAME;
+ if ( isset( $_COOKIE[ $rawCookie ] ) ) {
+ if ( (bool)$request->getCookie(
MobileContext::DISABLE_IMAGES_COOKIE_NAME ) ) {
$context->setDisableImagesCookie( true );
}
- $request->response()->clearCookie( 'disableImages' );
+ $request->response()->clearCookie(
MobileContext::DISABLE_IMAGES_COOKIE_NAME );
}
# Add deep link to a mobile app specified by $wgMFAppScheme
diff --git a/tests/phpunit/MobileContextTest.php
b/tests/phpunit/MobileContextTest.php
index 5208208..dfc9224 100644
--- a/tests/phpunit/MobileContextTest.php
+++ b/tests/phpunit/MobileContextTest.php
@@ -470,11 +470,11 @@
public function optInProvider() {
return [
[ [], false, true ],
- [ [ 'optin' => 'beta' ], true, true ],
- [ [ 'optin' => 'foobar' ], false, true ],
+ [ [ MobileContext::OPTIN_COOKIE_NAME =>
MobileContext::MODE_BETA ], true, true ],
+ [ [ MobileContext::OPTIN_COOKIE_NAME => 'foobar' ],
false, true ],
[ [], false, false ],
- [ [ 'optin' => 'beta' ], false, false ],
- [ [ 'optin' => 'foobar' ], false, false ],
+ [ [ MobileContext::OPTIN_COOKIE_NAME =>
MobileContext::MODE_BETA ], false, false ],
+ [ [ MobileContext::OPTIN_COOKIE_NAME => 'foobar' ],
false, false ],
];
}
@@ -571,7 +571,7 @@
public function testGetConfigVariable(
$expected,
$wgMinervaUseFooterV2,
- $mobileMode = 'stable'
+ $mobileMode = MobileContext::MODE_STABLE
) {
$this->setMwGlobals( [
'wgMFEnableBeta' => true,
@@ -594,8 +594,8 @@
];
return [
- [ 'foo', $wgMinervaUseFooterV2, 'stable' ],
- [ 'bar', $wgMinervaUseFooterV2, 'beta' ],
+ [ 'foo', $wgMinervaUseFooterV2,
MobileContext::MODE_STABLE ],
+ [ 'bar', $wgMinervaUseFooterV2,
MobileContext::MODE_BETA ],
[ null, [ 'alpha' => 'baz' ] ],
diff --git
a/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php
b/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php
index 526b916..0e9f690 100644
---
a/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php
+++
b/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php
@@ -69,11 +69,12 @@
// N.B. that the format and the "stop mobile redirect"
cookies
// ("mf_useformat" and "stopMobileRedirect"
respectively) aren't prefix
// with MediaWiki's cookie prefix ($wgCookiePrefix).
- $request->setCookie( 'mf_useformat', $formatCookie, ''
);
+ $request->setCookie(
MobileContext::USEFORMAT_COOKIE_NAME, $formatCookie, '' );
}
if ( $stopMobileRedirectCookie !== null ) {
- $request->setCookie( 'stopMobileRedirect',
$stopMobileRedirectCookie, '' );
+ $request->setCookie(
+ MobileContext::STOP_MOBILE_REDIRECT_COKIE_NAME,
$stopMobileRedirectCookie, '' );
}
if ( $isMobileUA ) {
--
To view, visit https://gerrit.wikimedia.org/r/322230
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I67743bb5b9512578d0a4efbdd9af9d1c0f586339
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Pmiazga <pmia...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
