AndyRussG has uploaded a new change for review.
https://gerrit.wikimedia.org/r/188121
Change subject: Remove unused banner variance parameters
......................................................................
Remove unused banner variance parameters
Once a banner has been chosen, we don't actually change banner content based on
device, country, isAnonymous, or bucket. Removing these parameters will reduce
cache fragmentation, and reduce average time to load banners.
This patch should have no user-visible effect.
Change-Id: I1a5ba8773cd8bebcc83edfc9a25d934a3a8530e3
---
M includes/BannerRenderer.php
M includes/MixinController.php
M modules/ext.centralNotice.bannerController/bannerController.js
M special/SpecialBannerLoader.php
M special/SpecialCentralNoticeBanners.php
5 files changed, 35 insertions(+), 58 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice
refs/changes/21/188121/1
diff --git a/includes/BannerRenderer.php b/includes/BannerRenderer.php
index 8b64845..b42c772 100644
--- a/includes/BannerRenderer.php
+++ b/includes/BannerRenderer.php
@@ -20,24 +20,16 @@
protected $mixinController = null;
- function __construct( IContextSource $context, Banner $banner,
$campaignName = null, AllocationContext $allocContext = null ) {
+ protected $debug;
+
+ function __construct( IContextSource $context, Banner $banner,
$campaignName = null, $debug = false ) {
$this->context = $context;
$this->banner = $banner;
$this->campaignName = $campaignName;
+ $this->debug = $debug;
- if ( $allocContext === null ) {
- /**
- * This should only be used when banners are previewed
in management forms.
- * TODO: set realistic context in the admin ui, drawn
from the campaign
- * configuration and current translation settings.
- */
- $this->allocContext = new AllocationContext( 'XX',
'en', 'wikipedia', true, 'desktop', 0 );
- } else {
- $this->allocContext = $allocContext;
- }
-
- $this->mixinController = new MixinController( $this->context,
$this->banner->getMixins(), $allocContext );
+ $this->mixinController = new MixinController( $this->context,
$this->banner->getMixins() );
//FIXME: it should make sense to do this:
// $this->mixinController->registerMagicWord( 'campaign',
array( $this, 'getCampaign' ) );
@@ -128,7 +120,7 @@
if ( $snippets ) {
foreach ( $snippets as $mixin => $code ) {
- if (
!$this->context->getRequest()->getFuzzyBool( 'debug' ) ) {
+ if ( !$this->debug ) {
$code = JavaScriptMinifier::minify(
$code );
}
diff --git a/includes/MixinController.php b/includes/MixinController.php
index 34e33ee..8567423 100644
--- a/includes/MixinController.php
+++ b/includes/MixinController.php
@@ -5,11 +5,9 @@
protected $magicWords = array();
protected $uiContext;
- protected $allocContext;
- function __construct( IContextSource $uiContext, $mixins,
AllocationContext $allocContext = null ) {
+ function __construct( IContextSource $uiContext, $mixins ) {
$this->uiContext = $uiContext;
- $this->allocContext = $allocContext;
$this->mixins = $mixins;
$this->loadPhp();
@@ -17,10 +15,6 @@
function getContext() {
return $this->uiContext;
- }
-
- function getAllocContext() {
- return $this->allocContext;
}
function getMagicWords() {
diff --git a/modules/ext.centralNotice.bannerController/bannerController.js
b/modules/ext.centralNotice.bannerController/bannerController.js
index 3db5fa3..e5247eb 100644
--- a/modules/ext.centralNotice.bannerController/bannerController.js
+++ b/modules/ext.centralNotice.bannerController/bannerController.js
@@ -142,10 +142,6 @@
banner: bannerName,
campaign: campaign,
uselang: mw.config.get( 'wgUserLanguage' ),
- db: mw.config.get( 'wgDBname' ),
- project: mw.config.get( 'wgNoticeProject' ),
- country: mw.centralNotice.data.country,
- device: mw.centralNotice.data.device,
debug: mw.centralNotice.data.getVars.debug
};
@@ -159,14 +155,7 @@
},
loadRandomBanner: function () {
- var fetchBannerQueryParams = {
- uselang: mw.config.get(
'wgUserLanguage' ),
- project: mw.config.get(
'wgNoticeProject' ),
- anonymous: mw.config.get( 'wgUserName'
) === null,
- country: mw.centralNotice.data.country,
- device: mw.centralNotice.data.device,
- debug:
mw.centralNotice.data.getVars.debug
- },
+ var loadBannerQueryParams,
scriptUrl;
// Either choose the banner on the client, or call the
server to get
@@ -196,11 +185,15 @@
// Only fetch a banner if we need to :)
if ( mw.centralNotice.data.banner ) {
- fetchBannerQueryParams.banner =
mw.centralNotice.data.banner;
- fetchBannerQueryParams.campaign =
mw.centralNotice.data.campaign;
+ loadBannerQueryParams = {
+ banner:
mw.centralNotice.data.banner,
+ campaign:
mw.centralNotice.data.campaign,
+ uselang: mw.config.get(
'wgUserLanguage' ),
+ debug:
mw.centralNotice.data.getVars.debug
+ };
scriptUrl = mw.config.get(
'wgCentralSelectedBannerDispatcher' ) +
- '?' + $.param(
fetchBannerQueryParams );
+ '?' + $.param(
loadBannerQueryParams );
// This will call insertBanner() after
the banner is retrieved
$.ajax( {
@@ -219,11 +212,20 @@
} else {
var RAND_MAX = 30;
- fetchBannerQueryParams.slot = Math.floor(
Math.random() * RAND_MAX ) + 1;
- fetchBannerQueryParams.bucket =
mw.centralNotice.data.bucket;
+
+ loadBannerQueryParams = {
+ uselang: mw.config.get(
'wgUserLanguage' ),
+ project: mw.config.get(
'wgNoticeProject' ),
+ anonymous: mw.config.get( 'wgUserName'
) === null,
+ country: mw.centralNotice.data.country,
+ device: mw.centralNotice.data.device,
+ debug:
mw.centralNotice.data.getVars.debug,
+ slot: Math.floor( Math.random() *
RAND_MAX ) + 1,
+ bucket: mw.centralNotice.data.bucket
+ };
scriptUrl = mw.config.get(
'wgCentralBannerDispatcher' ) +
- '?' + $.param( fetchBannerQueryParams );
+ '?' + $.param( loadBannerQueryParams );
$.ajax( {
url: scriptUrl,
diff --git a/special/SpecialBannerLoader.php b/special/SpecialBannerLoader.php
index 692a90b..8ab1229 100644
--- a/special/SpecialBannerLoader.php
+++ b/special/SpecialBannerLoader.php
@@ -9,7 +9,7 @@
/** @var string Name of the campaign that the banner belongs to.*/
public $campaignName;
- public $allocContext = null;
+ protected $debug;
function __construct() {
// Register special page
@@ -34,30 +34,21 @@
function getParams() {
$request = $this->getRequest();
+ // FIXME: Don't allow a default language.
$language = $this->getLanguage()->getCode();
- $project = $this->getSanitized( 'project',
ApiCentralNoticeAllocations::PROJECT_FILTER );
- $country = $this->getSanitized( 'country',
ApiCentralNoticeAllocations::LOCATION_FILTER );
- $anonymous = ( $this->getSanitized( 'anonymous',
ApiCentralNoticeAllocations::ANONYMOUS_FILTER ) === 'true' );
- $bucket = intval( $this->getSanitized( 'bucket',
ApiCentralNoticeAllocations::BUCKET_FILTER ) );
- $device = $this->getSanitized( 'device',
ApiCentralNoticeAllocations::DEVICE_NAME_FILTER );
+ $this->campaignName = $request->getText( 'campaign' );
+ $this->bannerName = $request->getText( 'banner' );
+ $this->debug = $request->getFuzzyBool( 'debug' );
$required_values = array(
- $project, $language, $country, $anonymous, $bucket,
$device
+ $this->campaignName, $this->bannerName, $language
);
foreach ( $required_values as $value ) {
if ( is_null( $value ) ) {
throw new MissingRequiredParamsException();
}
}
-
- $this->allocContext = new AllocationContext(
- $country, $language, $project,
- $anonymous, $device, $bucket
- );
-
- $this->campaignName = $request->getText( 'campaign' );
- $this->bannerName = $request->getText( 'banner' );
}
function getSanitized( $param, $filter ) {
@@ -100,7 +91,7 @@
if ( !$banner->exists() ) {
throw new EmptyBannerException( $bannerName );
}
- $bannerRenderer = new BannerRenderer( $this->getContext(),
$banner, $this->campaignName, $this->allocContext );
+ $bannerRenderer = new BannerRenderer( $this->getContext(),
$banner, $this->campaignName, $this->debug );
$bannerHtml = $bannerRenderer->toHtml();
diff --git a/special/SpecialCentralNoticeBanners.php
b/special/SpecialCentralNoticeBanners.php
index bd66ca5..7408d2c 100644
--- a/special/SpecialCentralNoticeBanners.php
+++ b/special/SpecialCentralNoticeBanners.php
@@ -917,10 +917,8 @@
$langContext = new DerivativeContext( $this->getContext() );
foreach ( $langs as $lang ) {
- // HACK: We need to unify these two contexts...
$langContext->setLanguage( $lang );
- $allocContext = new AllocationContext( 'XX', $lang,
'wikipedia', true, 'desktop', 0 );
- $bannerRenderer = new BannerRenderer( $langContext,
$banner, 'test', $allocContext );
+ $bannerRenderer = new BannerRenderer( $langContext,
$banner, 'test' );
// Link and Preview all available translations
$htmlOut .= Xml::tags(
--
To view, visit https://gerrit.wikimedia.org/r/188121
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a5ba8773cd8bebcc83edfc9a25d934a3a8530e3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: wmf_deploy
Gerrit-Owner: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
