r72932 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r72931‎ \| r72932 \| r72933 >
Date:	21:05, 13 September 2010
Author:	kaldari
Status:	resolved (Comments)
Tags:
Comment:	adding schema changes for geotargetting, adding BannerController class
Modified paths:	/trunk/extensions/CentralNotice/CentralNotice.php (modified) (history) /trunk/extensions/CentralNotice/SpecialBannerController.php (added) (history) /trunk/extensions/CentralNotice/SpecialBannerListLoader.php (modified) (history) /trunk/extensions/CentralNotice/SpecialBannerLoader.php (modified) (history) /trunk/extensions/CentralNotice/patches/patch-notice_geo.sql (added) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/SpecialBannerListLoader.php
—	—	@@ -7,8 +7,8 @@
8	8	public $project; // Project name
9	9	public $language; // Project language
10	10	public $centralNoticeDB;
11		~~- protected $sharedMaxAge = 22; // Cache for ? minutes on the server side~~
12		~~- protected $maxAge = 10; // Cache for ? minutes on the client side~~
	11	+ protected $sharedMaxAge = 150; // Cache for ? minutes on the server side
	12	+ protected $maxAge = 150; // Cache for ? minutes on the client side
13	13	protected $contentType = 'text/javascript';
14	14
15	15	function __construct() {
Index: trunk/extensions/CentralNotice/CentralNotice.php
—	—	@@ -83,25 +83,28 @@
84	84	$wgHooks['MakeGlobalVariablesScript'][] = 'efCentralNoticeDefaults';
85	85	$wgHooks['SiteNoticeAfter'][] = 'efCentralNoticeDisplay';
86	86	}
	87	+
	88	+ $wgSpecialPages['BannerLoader'] = 'SpecialBannerLoader';
	89	+ $wgAutoloadClasses['SpecialBannerLoader'] = $dir . 'SpecialBannerLoader.php';
	90	+
	91	+ $wgSpecialPages['BannerListLoader'] = 'SpecialBannerListLoader';
	92	+ $wgAutoloadClasses['SpecialBannerListLoader'] = $dir . 'SpecialBannerListLoader.php';
	93	+
	94	+ $wgSpecialPages['BannerController'] = 'SpecialBannerController';
	95	+ $wgAutoloadClasses['SpecialBannerController'] = $dir . 'SpecialBannerController.php';
87	96
88		~~- $wgAutoloadClasses['NoticePage'] = $dir . 'NoticePage.php';~~
89		-
90	97	if ( $wgNoticeInfrastructure ) {
91	98	$wgSpecialPages['CentralNotice'] = 'CentralNotice';
92	99	$wgSpecialPageGroups['CentralNotice'] = 'wiki'; // Wiki data and tools"
93	100	$wgAutoloadClasses['CentralNotice'] = $dir . 'SpecialCentralNotice.php';
94	101
95		~~- $wgSpecialPages['BannerLoader'] = 'SpecialBannerLoader';~~
96		~~- $wgAutoloadClasses['SpecialBannerLoader'] = $dir . 'SpecialBannerLoader.php';~~
	102	+ $wgSpecialPages['NoticeTemplate'] = 'SpecialNoticeTemplate';
	103	+ $wgAutoloadClasses['SpecialNoticeTemplate'] = $dir . 'SpecialNoticeTemplate.php';
97	104
98		~~- $wgSpecialPages['BannerListLoader'] = 'SpecialBannerListLoader';~~
99		~~- $wgAutoloadClasses['SpecialBannerListLoader'] = $dir . 'SpecialBannerListLoader.php';~~
100		-
	105	+ // remove these as soon as banner loader is complete
101	106	$wgSpecialPages['NoticeText'] = 'SpecialNoticeText';
102	107	$wgAutoloadClasses['SpecialNoticeText'] = $dir . 'SpecialNoticeText.php';
103		-
104		~~- $wgSpecialPages['NoticeTemplate'] = 'SpecialNoticeTemplate';~~
105		~~- $wgAutoloadClasses['SpecialNoticeTemplate'] = $dir . 'SpecialNoticeTemplate.php';~~
	108	+ $wgAutoloadClasses['NoticePage'] = $dir . 'NoticePage.php';
106	109
107	110	$wgAutoloadClasses['CentralNoticeDB'] = $dir . 'CentralNotice.db.php';
108	111	$wgAutoloadClasses['TemplatePager'] = $dir . 'TemplatePager.php';
—	—	@@ -117,6 +120,7 @@
118	121	$wgExtNewFields[] = array( 'cn_notices', 'not_preferred', $base . '/patches/patch-notice_preferred.sql' );
119	122	$wgExtNewTables[] = array( 'cn_notice_languages', $base . '/patches/patch-notice_languages.sql' );
120	123	$wgExtNewFields[] = array( 'cn_templates', 'tmp_display_anon', $base . '/patches/patch-template_settings.sql' );
	124	+ $wgExtNewTables[] = array( 'cn_notice_geo', $base . '/patches/patch-notice_geo.sql' );
121	125	}
122	126	return true;
123	127	}
—	—	@@ -124,7 +128,7 @@
125	129	function efCentralNoticeLoader( $out, $skin ) {
126	130	global $wgUser, $wgOut;
127	131
128		~~- $centralLoader = SpecialPage::getTitleFor( 'NoticeText' )->getLocalUrl();~~
	132	+ $centralLoader = SpecialPage::getTitleFor( 'BannerController' )->getLocalUrl();
129	133
130	134	// Insert the banner controller Javascript into the <head>
131	135	$wgOut->addScriptFile( $centralLoader );
Index: trunk/extensions/CentralNotice/SpecialBannerLoader.php
—	—	@@ -6,7 +6,7 @@
7	7	class SpecialBannerLoader extends UnlistedSpecialPage {
8	8	public $siteName = 'Wikipedia'; // Site name
9	9	public $language = 'en'; // User language
10		~~- protected $sharedMaxAge = 22; // Cache for 2 hours on the server side~~
	10	+ protected $sharedMaxAge = 150; // Cache for 2 hours on the server side
11	11	protected $maxAge = 0; // No client-side banner caching so we get all impressions
12	12	protected $contentType = 'text/html';
13	13
Index: trunk/extensions/CentralNotice/patches/patch-notice_geo.sql
—	—	@@ -0,0 +1,8 @@
	2	+-- Update to allow for any number of languages per notice.
	3	+
	4	+CREATE TABLE IF NOT EXISTS /$wgDBprefix/cn_notice_geo (
	5	+ ng_notice_id int unsigned NOT NULL,
	6	+ ng_country varchar(2) NOT NULL
	7	+) /$wgDBTableOptions/;
	8	+CREATE UNIQUE INDEX /i/ng_notice_id_geo ON /$wgDBprefix/cn_notice_geo (ng_notice_id, ng_country);
	9	+ALTER TABLE /$wgDBprefix/cn_notices ADD not_geo BOOLEAN NOT NULL DEFAULT '0' AFTER not_locked;
Index: trunk/extensions/CentralNotice/SpecialBannerController.php
—	—	@@ -0,0 +1,199 @@
	2	+<?php
	3	+
	4	+/**
	5	+ * Generates Javascript file which controls banner selection on the client side
	6	+ */
	7	+class SpecialBannerController extends UnlistedSpecialPage {
	8	+ public $centralNoticeDB;
	9	+ protected $sharedMaxAge = 150; // Cache for ? minutes on the server side
	10	+ protected $maxAge = 150; // Cache for ? minutes on the client side
	11	+ protected $contentType = 'text/javascript';
	12	+
	13	+ function __construct() {
	14	+ // Register special page
	15	+ parent::__construct( "BannerController" );
	16	+ $this->centralNoticeDB = new CentralNoticeDB();
	17	+ }
	18	+
	19	+ function execute( $par ) {
	20	+ global $wgOut, $wgRequest;
	21	+ global $wgNoticeLang, $wgNoticeProject;
	22	+
	23	+ $wgOut->disable();
	24	+ $this->sendHeaders();
	25	+
	26	+ $content = $this->getOutput();
	27	+ if ( strlen( $content ) == 0 ) {
	28	+ // Hack for IE/Mac 0-length keepalive problem, see RawPage.php
	29	+ echo "/* Empty */";
	30	+ } else {
	31	+ echo $content;
	32	+ }
	33	+
	34	+ }
	35	+
	36	+ /**
	37	+ * Generate the HTTP response headers for the banner controller
	38	+ */
	39	+ function sendHeaders() {
	40	+ header( "Content-type: $this->contentType; charset=utf-8" );
	41	+ header( "Cache-Control: public, s-maxage=$this->sharedMaxAge, max-age=$this->maxAge" );
	42	+ }
	43	+
	44	+ /**
	45	+ * Generate the body for a static Javascript file
	46	+ */
	47	+ function getOutput() {
	48	+ $js = $this->getScriptFunctions() . $this->getToggleScripts();
	49	+ $js .= <<<EOT
	50	+( function( $ ) {
	51	+ $.centralNotice = {
	52	+ 'data': {
	53	+ 'getVars': {}
	54	+ },
	55	+ 'fn': {
	56	+ 'loadBanner': function( bannerName ) {
	57	+ // get the requested banner from /centralnotice/banners/<bannername>/<wgUserLanguage>.js
	58	+ var bannerPage = 'Special:BannerLoader?banner='+bannerName+'&userlang='+wgContentLanguage+'&sitename='+wgNoticeProject;
	59	+ //centralized version:
	60	+ //var bannerURL = 'http://meta.wikimedia.org/wiki/'+bannerPage;
	61	+ //var bannerURL = wgArticlePath.replace( '$1', bannerPage );
	62	+ var bannerURL = 'http://localhost/~rkaldari/banner.html';
	63	+ var request = $.ajax( {
	64	+ url: bannerURL,
	65	+ dataType: 'html',
	66	+ success: function( data ) {
	67	+ $.centralNotice.fn.displayBanner( data );
	68	+ }
	69	+ });
	70	+ },
	71	+ 'loadBannerList': function( timestamp ) {
	72	+ var listURL;
	73	+ if ( timestamp ) {
	74	+ listURL = "TBD"
	75	+ } else {
	76	+ // http://geoiplookup.wikimedia.org/
	77	+ var geoLocation = 'US'; // Hard-coding for now
	78	+ var bannerListPage = 'Special:BannerListLoader?language='+wgContentLanguage+'&project='+wgNoticeProject+'&location='+geoLocation;
	79	+ //centralized version:
	80	+ //var bannerListURL = 'http://meta.wikimedia.org/wiki/'+bannerListPage;
	81	+ var bannerListURL = wgArticlePath.replace( '$1', bannerListPage );
	82	+ }
	83	+ var request = $.ajax( {
	84	+ url: bannerListURL,
	85	+ dataType: 'json',
	86	+ success: $.centralNotice.fn.chooseBanner
	87	+ } );
	88	+ },
	89	+ 'chooseBanner': function( bannerList ) {
	90	+ // convert the json object to a true array
	91	+ bannerList = Array.prototype.slice.call( bannerList );
	92	+
	93	+ // Make sure there are some banners to choose from
	94	+ if ( bannerList.length == 0 ) return false;
	95	+
	96	+ var groomedBannerList = [];
	97	+
	98	+ for( var i = 0; i < bannerList.length; i++ ) {
	99	+ // only include this banner if it's inteded for the current user
	100	+ if( ( wgUserName ? bannerList[i].display_account == 1 : bannerList.display_anon == 1 ) ) {
	101	+ // add the banner to our list once per weight
	102	+ for( var j=0; j < bannerList[i].weight; j++ ) {
	103	+ groomedBannerList.push( bannerList[i] );
	104	+ }
	105	+ }
	106	+ }
	107	+
	108	+ // return if there's nothing left after the grooming
	109	+ if( groomedBannerList.length == 0 ) return false;
	110	+ // load a random banner from our groomed list
	111	+
	112	+ $.centralNotice.fn.loadBanner(
	113	+ groomedBannerList[ Math.floor( Math.random() * groomedBannerList.length ) ].name
	114	+ );
	115	+ },
	116	+ 'displayBanner': function( bannerHTML ) {
	117	+ // inject the banner html into the page
	118	+ $( '#siteNotice' )
	119	+ .prepend( '<div id="centralnotice" class="' + ( wgNoticeToggleState ? 'expanded' : 'collapsed' ) + '">' + bannerHTML + '</div>' );
	120	+ },
	121	+ 'getQueryStringVariables': function() {
	122	+ document.location.search.replace( /\??(?:([^=]+)=([^&]*)&?)/g, function () {
	123	+ function decode( s ) {
	124	+ return decodeURIComponent( s.split( "+" ).join( " " ) );
	125	+ }
	126	+ $.centralNotice.data.getVars[decode( arguments[1] )] = decode( arguments[2] );
	127	+ } );
	128	+ }
	129	+ }
	130	+ }
	131	+ $( document ).ready( function () {
	132	+ // initialize the query string vars
	133	+ $.centralNotice.fn.getQueryStringVariables();
	134	+ if( $.centralNotice.data.getVars['forceBanner'] ) {
	135	+ // if we're forcing one banner
	136	+ $.centralNotice.fn.loadBanner( $.centralNotice.data.getVars['forceBanner'] );
	137	+ } else if ( $.centralNotice.data.getVars['forceTimestamp'] ) {
	138	+ // if we're forcing a future campaign time
	139	+ $.centralNotice.fn.loadBannerList( $.centralNotice.data.getVars['forceTimestamp'] );
	140	+ } else {
	141	+ // look for banners ready to go NOW
	142	+ $.centralNotice.fn.loadBannerList( );
	143	+ }
	144	+ } ); //document ready
	145	+} )( jQuery );
	146	+EOT;
	147	+ return $js;
	148	+
	149	+ }
	150	+
	151	+ function getToggleScripts() {
	152	+ $showStyle = <<<END
	153	+<style type="text/css">
	154	+#centralNotice .siteNoticeSmall {display:none;}
	155	+#centralNotice .siteNoticeSmallAnon {display:none;}
	156	+#centralNotice .siteNoticeSmallUser {display:none;}
	157	+#centralNotice.collapsed .siteNoticeBig {display:none;}
	158	+#centralNotice.collapsed .siteNoticeSmall {display:block;}
	159	+#centralNotice.collapsed .siteNoticeSmallUser {display:block;}
	160	+#centralNotice.collapsed .siteNoticeSmallAnon {display:block;}
	161	+#centralNotice.anonnotice .siteNoticeSmallUser {display:none !important;}
	162	+#centralNotice.usernotice .siteNoticeSmallAnon {display:none !important;}
	163	+</style>
	164	+END;
	165	+ $encShowStyle = Xml::encodeJsVar( $showStyle );
	166	+
	167	+ $script = "
	168	+var wgNoticeToggleState = (document.cookie.indexOf('hidesnmessage=1')==-1);
	169	+document.writeln($encShowStyle);\n\n";
	170	+ return $script;
	171	+ }
	172	+
	173	+ function getScriptFunctions() {
	174	+ $script = "
	175	+function toggleNotice() {
	176	+ var notice = document.getElementById('centralNotice');
	177	+ if (!wgNoticeToggleState) {
	178	+ notice.className = notice.className.replace('collapsed', 'expanded');
	179	+ toggleNoticeCookie('0');
	180	+ } else {
	181	+ notice.className = notice.className.replace('expanded', 'collapsed');
	182	+ toggleNoticeCookie('1');
	183	+ }
	184	+ wgNoticeToggleState = !wgNoticeToggleState;
	185	+}
	186	+function toggleNoticeStyle(elems, display) {
	187	+ if(elems)
	188	+ for(var i=0;i<elems.length;i++)
	189	+ elems[i].style.display = display;
	190	+}
	191	+function toggleNoticeCookie(state) {
	192	+ var e = new Date();
	193	+ e.setTime( e.getTime() + (72460601000) ); // one week
	194	+ var work='hidesnmessage='+state+'; expires=' + e.toGMTString() + '; path=/';
	195	+ document.cookie = work;
	196	+}\n\n";
	197	+ return $script;
	198	+ }
	199	+
	200	+}

Follow-up revisions

Revision	Commit summary	Author	Date
r72950	schema changes from r72932 (futureproofing), adding basic support for saving ...	kaldari	01:19, 14 September 2010

Comments

#Comment by Catrope (talk | contribs) 18:36, 14 September 2010

+ALTER TABLE /*$wgDBprefix*/cn_notices ADD not_geo BOOLEAN NOT NULL DEFAULT '0' AFTER not_locked;

What is this field for?

+		$js .= <<<EOT
+( function( $ ) {
+	$.centralNotice = {
blah blah blah
+EOT

Is this all static JS? If so, it should be in a .js file.

+document.writeln($encShowStyle);\n\n";

Don't you need to put quotes around $encShowStyle here?

#Comment by Kaldari (talk | contribs) 19:54, 14 September 2010

The not_geo field records whether the campaign has geotargeting turned on or off.

#Comment by Catrope (talk | contribs) 17:28, 27 September 2010

OK. Did the other issues get fixed? If so, in which revs?

#Comment by Kaldari (talk | contribs) 18:00, 27 September 2010

Regarding your second comment, it's not all static JS, although it was at the time, so that should be a moot issue. Regarding the third comment, the quotes are added by: $encShowStyle = Xml::encodeJsVar( $showStyle );

Status & tagging log

18:01, 27 September 2010 Catrope (talk | contribs) changed the status of r72932 [removed: fixme added: resolved]
18:36, 14 September 2010 Catrope (talk | contribs) changed the status of r72932 [removed: new added: fixme]