Awight has uploaded a new change for review.
https://gerrit.wikimedia.org/r/176251
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, 16 insertions(+), 47 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice
refs/changes/51/176251/1
diff --git a/includes/BannerRenderer.php b/includes/BannerRenderer.php
index 36fcdca..d2d5125 100644
--- a/includes/BannerRenderer.php
+++ b/includes/BannerRenderer.php
@@ -20,24 +20,13 @@
protected $mixinController = null;
- function __construct( IContextSource $context, Banner $banner,
$campaignName = null, AllocationContext $allocContext = null ) {
+ function __construct( IContextSource $context, Banner $banner,
$campaignName = null ) {
$this->context = $context;
$this->banner = $banner;
$this->campaignName = $campaignName;
- 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' ) );
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 1ed85af..9c09ee7 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
};
@@ -196,11 +192,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;
+ var 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( {
diff --git a/special/SpecialBannerLoader.php b/special/SpecialBannerLoader.php
index 692a90b..bf46746 100644
--- a/special/SpecialBannerLoader.php
+++ b/special/SpecialBannerLoader.php
@@ -9,8 +9,6 @@
/** @var string Name of the campaign that the banner belongs to.*/
public $campaignName;
- public $allocContext = null;
-
function __construct() {
// Register special page
parent::__construct( "BannerLoader" );
@@ -34,30 +32,20 @@
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' );
$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 +88,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 );
$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/176251
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: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
