Adamw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/59366
Change subject: WIP: objects to manage allocations data
......................................................................
WIP: objects to manage allocations data
This will make it simple to merge groupings when allocations are the same.
Change-Id: I047074df32f61554d8d21e76727ee1b596710fdc
---
M api/ApiCentralNoticeAllocations.php
M api/ApiCentralNoticeLogs.php
M includes/BannerChooser.php
M includes/CampaignLog.php
M special/SpecialGlobalAllocation.php
5 files changed, 122 insertions(+), 60 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice
refs/changes/66/59366/1
diff --git a/api/ApiCentralNoticeAllocations.php
b/api/ApiCentralNoticeAllocations.php
index e61fc86..b59a48b 100644
--- a/api/ApiCentralNoticeAllocations.php
+++ b/api/ApiCentralNoticeAllocations.php
@@ -51,9 +51,8 @@
$params['language'],
$params['anonymous'],
$params['device'],
- $params['bucket'],
- $params['minimal']
- );
+ $params['bucket']
+ )->getBanners();
$result->setIndexedTagName( $bannerList, 'BannerAllocation' );
$result->addValue( $this->getModuleName(), 'banners',
$bannerList );
@@ -68,7 +67,6 @@
$params['anonymous']=
ApiCentralNoticeAllocations::DEFAULT_ANONYMOUS;
$params['device'] =
ApiCentralNoticeAllocations::DEFAULT_DEVICE_NAME;
$params['bucket'] =
ApiCentralNoticeAllocations::DEFAULT_BUCKET;
- $params['minimal'] = false;
return $params;
}
@@ -84,7 +82,6 @@
$params['anonymous']= "The logged-in status to filter on
(true|false)";
$params['device'] = "Device name to filter on";
$params['bucket'] = "The bucket to filter on, by number (0 ..
$wgNoticeNumberOfBuckets, optional)";
- $params['minimal'] = "Alters return - only what is required
for the banner loader will be returned";
return $params;
}
@@ -110,17 +107,14 @@
* MediaWiki interface to this API call -- obtains banner allocation
information; ie how many
* buckets there are in a campaign, and what banners should be
displayed for a given filter.
*
- * Returns results as an array of banners
- * - banners
+ * @return BannerChooser object, which can be queried with getBanners()
to
+ * obtain a list of banner structs having:
*
- * The following information is provided in minimal mode:
* - name The name of the banner
* - allocation What the allocation proportion (0 to 1)
should be
* - campaign The name of the associated campaign
* - fundraising 1 if this is a fundraising banner
* - bucket The bucket this is assigned to in the
campaign
- *
- * In normal mode the following information is
additionally supplied:
* - weight The assigned weight in the campaign
* - display_anon 1 if should be displayed to
anonymous users
* - display_account 1 if should be displayed to logged
in users
@@ -134,26 +128,21 @@
* @param string $anonymous - Is user anonymous, eg 'true'
* @param string $device - What device to filter on, eg 'desktop' or
'mobile.device.ie'
* @param string $bucket - Which A/B bucket the user is in
- * @param bool $minimize - True if the results should be minimized
for banner usage
*
* @return array
*/
- public static function getBannerAllocation( $project, $country,
$language, $anonymous, $device, $bucket = null, $minimize = false ) {
- self::sanitizeParams( $project, $country, $language,
$anonymous, $device, $bucket, $minimize );
+ public static function getBannerAllocation( $project, $country,
$language, $anonymous, $device, $bucket = null ) {
+ self::sanitizeParams( $project, $country, $language,
$anonymous, $device, $bucket );
$chooser = new BannerChooser();
$chooser->filter( $project, $language, $country, $anonymous,
$device, $bucket );
$banners = $chooser->getBanners();
- if ( $minimize ) {
- $banners = static::minimizeBanners( $banners );
- }
-
return $banners;
}
// FIXME: this function is steeoopid
- protected static function sanitizeParams( &$project, &$country,
&$language, &$anonymous, &$device, &$bucket, &$minimize ) {
+ protected static function sanitizeParams( &$project, &$country,
&$language, &$anonymous, &$device, &$bucket ) {
$project = ApiCentralNoticeAllocations::sanitizeText(
$project,
self::PROJECT_FILTER,
@@ -190,8 +179,6 @@
self::BUCKET_FILTER,
self::DEFAULT_BUCKET
);
-
- $minimize = (boolean) $minimize;
}
/**
@@ -213,25 +200,5 @@
} else {
return $default;
}
- }
-
- /**
- * Reduces a set of banners to only what the banner controller needs
- */
- private static function minimizeBanners( $banners ) {
- $requiredKeys = array(
- 'allocation',
- 'campaign',
- 'fundraising',
- 'name',
- 'bucket',
- );
-
- $filtVal = array();
- foreach ( $banners as $banner ) {
- $filtVal[] = array_intersect_key( $banner, array_flip(
$requiredKeys ) );
- }
-
- return $filtVal;
}
}
diff --git a/api/ApiCentralNoticeLogs.php b/api/ApiCentralNoticeLogs.php
index cd913e4..a74fd63 100644
--- a/api/ApiCentralNoticeLogs.php
+++ b/api/ApiCentralNoticeLogs.php
@@ -4,7 +4,7 @@
class ApiCentralNoticeLogs extends ApiQueryBase {
- #XXX
+ #FIXME: from a shared include
const USER_FILTER = '/[a-zA-Z0-9_.]+/';
const CAMPAIGNS_FILTER = '/[a-zA-Z0-9_|\-]+/';
diff --git a/includes/BannerChooser.php b/includes/BannerChooser.php
index 22a0f7c..32c0281 100644
--- a/includes/BannerChooser.php
+++ b/includes/BannerChooser.php
@@ -183,4 +183,17 @@
function getBanners() {
return $this->banners;
}
+
+ function getBanners() {
+ return $this->banners;
+ }
+
+ function getAllocationHash() {
+ $allocSignatures = array();
+ foreach ( $this->banners as $banner ) {
+ //FIXME: no LCF search
+ $allocSignatures[] =
"{$banner['name']}:{$banner['allocation']}";
+ }
+ return sha1( implode( ";", $allocSignatures ) );
+ }
}
diff --git a/includes/CampaignLog.php b/includes/CampaignLog.php
index 8600513..de9b1b3 100644
--- a/includes/CampaignLog.php
+++ b/includes/CampaignLog.php
@@ -46,7 +46,7 @@
$removed = array();
$added = array();
- # XXX cannot use "this" in closures until php 5.4
+ # TODO: cannot use "this" in closures until php 5.4
$begin =& $this->begin;
$end =& $this->end;
diff --git a/special/SpecialGlobalAllocation.php
b/special/SpecialGlobalAllocation.php
index 9e701f5..115debc 100644
--- a/special/SpecialGlobalAllocation.php
+++ b/special/SpecialGlobalAllocation.php
@@ -353,21 +353,7 @@
}
if ( $result ) {
- sort( $contributing );
- if ( $excluding ) {
- sort( $excluding );
- $label = $this->msg(
'centralnotice-excluding-list',
-
$this->getLanguage()->commaList( $contributing ),
-
$this->getLanguage()->listToText( $excluding )
- );
- } else {
- $label =
$this->getLanguage()->commaList( $contributing );
- }
-
- $groupings[] = array(
- 'label' => $label,
- 'rows' => $result,
- );
+ $groupings[] = new AllocationGrouping(
$contributing, $excluding, $result );
}
}
}
@@ -377,7 +363,7 @@
/**
* Return every (unordered) combination of $num elements from $list
- * TODO: don't use recursion.
+ * TODO: don't use recursion, it limits num_campaigns << 100
*/
protected static function makeCombinations( array $list, $num ) {
if ( $num <= 0 or $num > count( $list ) ) {
@@ -417,6 +403,8 @@
* @return string HTML for the table
*/
protected function getBannerAllocationsTable( $project, $location,
$language, $numBuckets ) {
+ //XXX pass BannerChooser obj
+
// This is annoying. Within the campaign, banners usually vary
by user
// logged-in status, and bucket. Determine the allocations and
// collapse any dimensions which do not vary.
@@ -490,7 +478,7 @@
$this->msg( 'centralnotice-notice' )->text() );
$htmlOut .= Html::closeElement( 'tr' );
- foreach ( $variations as $isAnon => $bucketVariations ) {
+ foreach ( $grouping->getAllocations() as $isAnon =>
$bucketVariations ) {
foreach ( $bucketVariations as $bucket => $banners ) {
foreach ( $banners as $banner ) {
$htmlOut .=
$this->getBannerAllocationsVariantRow( $banner, $variesAnon, $variesBucket,
$isAnon, $bucket );
@@ -509,7 +497,7 @@
/**
* Print one line of banner allocations.
*/
- function getBannerAllocationsVariantRow( $banner, $variesAnon,
$variesBucket, $isAnon, $bucket ) {
+ function getBannerAllocationsVariantRow( array $banner, $variesAnon,
$variesBucket, $isAnon, $bucket ) {
$htmlOut = '';
$viewBanner = $this->getTitleFor( 'NoticeTemplate', 'view' );
@@ -582,6 +570,100 @@
return $htmlOut;
}
+
+ protected function getGroupingLabel( AllocationGrouping $grouping ) {
+ sort( $grouping->getContributing() );
+ if ( $grouping->getExcluding() ) {
+ sort( $grouping->getExcluding() );
+ return $this->msg( 'centralnotice-excluding-list',
+ $this->getLanguage()->commaList(
$grouping->getContributing() ),
+ $this->getLanguage()->listToText(
$grouping->getExcluding() )
+ );
+ } else {
+ return $this->getLanguage()->commaList(
$grouping->getContributing() );
+ }
+ }
+}
+
+/**
+ * A group of targets (project, country, language) for which the allocations
are the same.
+ */
+class AllocationGrouping {
+ protected $contributing;
+ protected $excluding;
+ protected $rows;
+
+ protected $numBuckets = 0;
+
+ /**
+ * @var multidimensional array with indexes (isAnon, bucket)
+ * -> BannerChooser as returned by
ApiCentralNoticeAllocations::getAllocationInformation()
+ */
+ protected $variations;
+
+ function __construct( $contributing, $excluding, $rows ) {
+ sort( $contributing );
+ sort( $excluding );
+ $this->contributing = $contributing;
+ $this->excluding = $excluding;
+ $this->rows = $rows;
+
+ foreach ( $this->contributing as $campaign ) {
+ $this->numBuckets = max( $this->numBuckets,
$campaign['buckets'] );
+ }
+
+ $this->variations = $this->calculateAllocations();
+ }
+
+ static function mergeGroupings() {
+ }
+
+ function isVaryBucket() {
+ // Even if allocations don't vary, this is enough reason to
show the full matrix:
+ //FIXME: merging should preserve
+ return ( $this->numBuckets > 1 );
+ }
+
+ function isVaryAuthenticated() {
+ foreach ( range( 0, $numBuckets - 1 ) as $bucket ) {
+ if ( $this->variations[0][$bucket]->getAllocationHash()
!= $this->variations[1][$bucket]->getAllocationHash() ) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ function getHash() {
+ $summary = array();
+ array_walk_recursive( $this->variations,
+ function ( $alloc ) use ( &$summary ) {
+ $summary[] = $alloc->getAllocationHash();
+ }
+ );
+ return sha1( implode( ";", $summary ) );
+ }
+
+ function getVariations() {
+ return $this->variations;
+ }
+
+ protected function calculateAllocations() {
+ // This is annoying. Within the campaign, banners usually vary
by user
+ // logged-in status, and bucket. Determine the allocations and
+ // collapse any dimensions which do not vary.
+ //
+ // TODO: the allocation hash should also be used to collapse
groupings which
+ // are identical because of e.g. z-index
+ foreach ( array( true, false ) as $isAnon ) {
+ for ( $bucket = 0; $bucket < $numBuckets; $bucket++ ) {
+ $variations[$isAnon][$bucket] =
ApiCentralNoticeAllocations::getAllocationInformation(
+ $project, $country, $language,
+ $isAnon ? 'true' : 'false',
+ $bucket
+ );
+ }
+ }
+ }
}
class CampaignCriteria {
--
To view, visit https://gerrit.wikimedia.org/r/59366
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I047074df32f61554d8d21e76727ee1b596710fdc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Adamw <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
