r99181 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r99180‎ | r99181 | r99182 >
Date:01:28, 7 October 2011
Author:kaldari
Status:ok
Tags:fundraising 
Comment:
more generic messaging, automatically constructed fundraising links dont neccessaily use LandingCheck
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.db.php (modified) (history)
  • /trunk/extensions/CentralNotice/CentralNotice.i18n.php (modified) (history)
  • /trunk/extensions/CentralNotice/CentralNotice.php (modified) (history)
  • /trunk/extensions/CentralNotice/CentralNoticeBannerLogPager.php (modified) (history)
  • /trunk/extensions/CentralNotice/centralnotice.js (modified) (history)
  • /trunk/extensions/CentralNotice/patches/patch-template_autolink.sql (added) (history)
  • /trunk/extensions/CentralNotice/patches/patch-template_landingcheck.sql (deleted) (history)
  • /trunk/extensions/CentralNotice/special/SpecialBannerController.php (modified) (history)
  • /trunk/extensions/CentralNotice/special/SpecialBannerLoader.php (modified) (history)
  • /trunk/extensions/CentralNotice/special/SpecialNoticeTemplate.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/special/SpecialBannerController.php
@@ -161,7 +161,7 @@
162162 $script = <<<JAVASCRIPT
163163 function insertBanner(bannerJson) {
164164 jQuery( 'div#centralNotice' ).prepend( bannerJson.bannerHtml );
165 - if ( bannerJson.landingCheck ) {
 165+ if ( bannerJson.autolink ) {
166166 JAVASCRIPT;
167167 $script .= "\n\t\tvar url = '" .
168168 Xml::escapeJsString( $wgNoticeFundraisingUrl ) . "';\n";
Index: trunk/extensions/CentralNotice/special/SpecialBannerLoader.php
@@ -75,7 +75,7 @@
7676 'bannerHtml' => $bannerHtml,
7777 'campaign' => $this->campaign,
7878 'fundraising' => $this->getFundraising( $bannerName ),
79 - 'landingCheck' => $this->getLandingCheck( $bannerName ),
 79+ 'autolink' => $this->getAutolink( $bannerName ),
8080 'landingPages' => $this->getLandingPages( $bannerName )
8181 );
8282 $bannerJs = 'insertBanner('.FormatJson::encode( $bannerArray ).');';
@@ -204,12 +204,12 @@
205205 return $row->tmp_fundraising;
206206 }
207207
208 - function getLandingCheck( $bannerName ) {
 208+ function getAutolink( $bannerName ) {
209209 global $wgCentralDBname;
210210 $dbr = wfGetDB( DB_SLAVE, array(), $wgCentralDBname );
211211 $eBannerName = htmlspecialchars( $bannerName );
212 - $row = $dbr->selectRow( 'cn_templates', 'tmp_landingcheck', array( 'tmp_name' => $eBannerName ) );
213 - return $row->tmp_landingcheck;
 212+ $row = $dbr->selectRow( 'cn_templates', 'tmp_autolink', array( 'tmp_name' => $eBannerName ) );
 213+ return $row->tmp_autolink;
214214 }
215215
216216 function getLandingPages( $bannerName ) {
Index: trunk/extensions/CentralNotice/special/SpecialNoticeTemplate.php
@@ -90,7 +90,7 @@
9191 $wgRequest->getBool( 'displayAnon' ),
9292 $wgRequest->getBool( 'displayAccount' ),
9393 $wgRequest->getBool( 'fundraising' ),
94 - $wgRequest->getBool( 'landingCheck' ),
 94+ $wgRequest->getBool( 'autolink' ),
9595 $wgRequest->getVal( 'landingPages' )
9696 );
9797 $sub = 'view';
@@ -107,7 +107,7 @@
108108 $wgRequest->getBool( 'displayAnon' ),
109109 $wgRequest->getBool( 'displayAccount' ),
110110 $wgRequest->getBool( 'fundraising' ),
111 - $wgRequest->getBool( 'landingCheck' ),
 111+ $wgRequest->getBool( 'autolink' ),
112112 $wgRequest->getVal( 'landingPages' )
113113 );
114114 $sub = 'view';
@@ -240,7 +240,7 @@
241241 $displayAnon = $wgRequest->getCheck( 'displayAnon' );
242242 $displayAccount = $wgRequest->getCheck( 'displayAccount' );
243243 $fundraising = $wgRequest->getCheck( 'fundraising' );
244 - $landingCheck = $wgRequest->getCheck( 'landingCheck' );
 244+ $autolink = $wgRequest->getCheck( 'autolink' );
245245 $landingPages = $wgRequest->getVal( 'landingPages' );
246246 $body = $wgRequest->getVal( 'templateBody' );
247247 } else { // Use default values
@@ -248,7 +248,7 @@
249249 $displayAnon = true;
250250 $displayAccount = true;
251251 $fundraising = false;
252 - $landingCheck = false;
 252+ $autolink = false;
253253 $landingPages = '';
254254 $body = '';
255255 }
@@ -279,15 +279,15 @@
280280 $htmlOut .= Xml::label( wfMsg( 'centralnotice-banner-fundraising' ), 'fundraising' );
281281 $htmlOut .= Html::closeElement( 'p' );
282282
283 - // Checkbox for whether or not to use the LandingCheck extension
 283+ // Checkbox for whether or not to automatically create landing page link
284284 $htmlOut .= Html::openElement( 'p', null );
285 - $htmlOut .= Xml::check( 'landingCheck', $landingCheck, array( 'id' => 'landingCheck' ) );
286 - $htmlOut .= Xml::label( wfMsg( 'centralnotice-banner-autolink' ), 'landingCheck' );
 285+ $htmlOut .= Xml::check( 'autolink', $autolink, array( 'id' => 'autolink' ) );
 286+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-banner-autolink' ), 'autolink' );
287287 $htmlOut .= Html::closeElement( 'p' );
288288
289289 // Interface for setting the landing pages
290290 $htmlOut .= Html::openElement( 'div',
291 - array( 'id' => 'landingCheckInterface', 'style' => 'display: none;' ) );
 291+ array( 'id' => 'autolinkInterface', 'style' => 'display: none;' ) );
292292 $htmlOut .= Xml::tags( 'p', array(),
293293 wfMsg( 'centralnotice-banner-autolink-help', 'id="cn-landingpage-link"', 'JimmyAppeal01' ) );
294294 $htmlOut .= Xml::tags( 'p', array(),
@@ -563,14 +563,14 @@
564564 $displayAnon = $wgRequest->getCheck( 'displayAnon' );
565565 $displayAccount = $wgRequest->getCheck( 'displayAccount' );
566566 $fundraising = $wgRequest->getCheck( 'fundraising' );
567 - $landingCheck = $wgRequest->getCheck( 'landingCheck' );
 567+ $autolink = $wgRequest->getCheck( 'autolink' );
568568 $landingPages = $wgRequest->getVal( 'landingPages' );
569569 $body = $wgRequest->getVal( 'templateBody', $body );
570570 } else { // Use previously stored values
571571 $displayAnon = ( $bannerSettings['anon'] == 1 );
572572 $displayAccount = ( $bannerSettings['account'] == 1 );
573573 $fundraising = ( $bannerSettings['fundraising'] == 1 );
574 - $landingCheck = ( $bannerSettings['landingcheck'] == 1 );
 574+ $autolink = ( $bannerSettings['autolink'] == 1 );
575575 $landingPages = $bannerSettings['landingpages'];
576576 // $body default is defined prior to message interface code
577577 }
@@ -598,20 +598,20 @@
599599 'fundraising' );
600600 $htmlOut .= Html::closeElement( 'p' );
601601
602 - // Checkbox for whether or not to use the LandingCheck extension
 602+ // Checkbox for whether or not to automatically create landing page link
603603 $htmlOut .= Html::openElement( 'p', null );
604 - $htmlOut .= Xml::check( 'landingCheck', $landingCheck,
605 - wfArrayMerge( $disabled, array( 'id' => 'landingCheck' ) ) );
 604+ $htmlOut .= Xml::check( 'autolink', $autolink,
 605+ wfArrayMerge( $disabled, array( 'id' => 'autolink' ) ) );
606606 $htmlOut .= Xml::label( wfMsg( 'centralnotice-banner-autolink' ),
607 - 'landingCheck' );
 607+ 'autolink' );
608608 $htmlOut .= Html::closeElement( 'p' );
609609
610610 // Interface for setting the landing pages
611 - if ( $landingCheck ) {
612 - $htmlOut .= Html::openElement( 'div', array( 'id'=>'landingCheckInterface' ) );
 611+ if ( $autolink ) {
 612+ $htmlOut .= Html::openElement( 'div', array( 'id'=>'autolinkInterface' ) );
613613 } else {
614614 $htmlOut .= Html::openElement( 'div',
615 - array( 'id'=>'landingCheckInterface', 'style'=>'display:none;' ) );
 615+ array( 'id'=>'autolinkInterface', 'style'=>'display:none;' ) );
616616 }
617617 $htmlOut .= Xml::tags( 'p', array(),
618618 wfMsg( 'centralnotice-banner-autolink-help', 'id="cn-landingpage-link"', 'JimmyAppeal01' ) );
@@ -810,12 +810,12 @@
811811 * @param $displayAnon integer flag for display to anonymous users
812812 * @param $displayAccount integer flag for display to logged in users
813813 * @param $fundraising integer flag for fundraising banner (optional)
814 - * @param $landingCheck integer flag for using LandingCheck (optional)
 814+ * @param $autolink integer flag for automatically creating landing page links (optional)
815815 * @param $landingPages string list of landing pages (optional)
816816 * @return true or false depending on whether banner was successfully added
817817 */
818818 public function addTemplate( $name, $body, $displayAnon, $displayAccount, $fundraising = 0,
819 - $landingCheck = 0, $landingPages = '' ) {
 819+ $autolink = 0, $landingPages = '' ) {
820820
821821 if ( $body == '' || $name == '' ) {
822822 $this->showError( 'centralnotice-null-string' );
@@ -844,7 +844,7 @@
845845 'tmp_display_anon' => $displayAnon,
846846 'tmp_display_account' => $displayAccount,
847847 'tmp_fundraising' => $fundraising,
848 - 'tmp_landingcheck' => $landingCheck,
 848+ 'tmp_autolink' => $autolink,
849849 'tmp_landing_pages' => $landingPages
850850 ),
851851 __METHOD__
@@ -863,7 +863,7 @@
864864 'anon' => $displayAnon,
865865 'account' => $displayAccount,
866866 'fundraising' => $fundraising,
867 - 'landingcheck' => $landingCheck,
 867+ 'autolink' => $autolink,
868868 'landingpages' => $landingPages
869869 );
870870 $this->logBannerChange( 'created', $bannerId, $beginSettings, $endSettings );
@@ -876,7 +876,7 @@
877877 * Update a banner
878878 */
879879 private function editTemplate( $name, $body, $displayAnon, $displayAccount, $fundraising,
880 - $landingCheck, $landingPages ) {
 880+ $autolink, $landingPages ) {
881881
882882 if ( $body == '' || $name == '' ) {
883883 $this->showError( 'centralnotice-null-string' );
@@ -898,7 +898,7 @@
899899 'tmp_display_anon' => $displayAnon,
900900 'tmp_display_account' => $displayAccount,
901901 'tmp_fundraising' => $fundraising,
902 - 'tmp_landingcheck' => $landingCheck,
 902+ 'tmp_autolink' => $autolink,
903903 'tmp_landing_pages' => $landingPages
904904 ),
905905 array( 'tmp_name' => $name )
@@ -945,7 +945,7 @@
946946 'tmp_display_anon',
947947 'tmp_display_account',
948948 'tmp_fundraising',
949 - 'tmp_landingcheck',
 949+ 'tmp_autolink',
950950 'tmp_landing_pages'
951951 ),
952952 array( 'tmp_name' => $source ),
@@ -954,7 +954,7 @@
955955 $displayAnon = $row->tmp_display_anon;
956956 $displayAccount = $row->tmp_display_account;
957957 $fundraising = $row->tmp_fundraising;
958 - $landingCheck = $row->tmp_landingcheck;
 958+ $autolink = $row->tmp_autolink;
959959 $landingPages = $row->tmp_landing_pages;
960960
961961 // Pull banner text and respect any inc: markup
@@ -963,7 +963,7 @@
964964
965965 // Create new banner
966966 if ( $this->addTemplate( $dest, $template_body, $displayAnon, $displayAccount, $fundraising,
967 - $landingCheck, $landingPages ) ) {
 967+ $autolink, $landingPages ) ) {
968968
969969 // Populate the fields
970970 foreach ( $langs as $lang => $fields ) {
Index: trunk/extensions/CentralNotice/CentralNotice.php
@@ -161,8 +161,8 @@
162162 $base . '/patches/patch-notice_log.sql' );
163163 $wgExtNewTables[] = array( 'cn_template_log',
164164 $base . '/patches/patch-template_log.sql' );
165 - $wgExtNewFields[] = array( 'cn_templates', 'tmp_landingcheck',
166 - $base . '/patches/patch-template_landingcheck.sql' );
 165+ $wgExtNewFields[] = array( 'cn_templates', 'tmp_autolink',
 166+ $base . '/patches/patch-template_autolink.sql' );
167167 }
168168 } else {
169169 if ( $updater->getDB()->getType() == 'mysql' ) {
@@ -184,8 +184,8 @@
185185 $base . '/patches/patch-notice_log.sql', true ) );
186186 $updater->addExtensionUpdate( array( 'addTable', 'cn_template_log',
187187 $base . '/patches/patch-template_log.sql', true ) );
188 - $updater->addExtensionUpdate( array( 'addField', 'cn_templates', 'tmp_landingcheck',
189 - $base . '/patches/patch-template_landingcheck.sql', true ) );
 188+ $updater->addExtensionUpdate( array( 'addField', 'cn_templates', 'tmp_autolink',
 189+ $base . '/patches/patch-template_autolink.sql', true ) );
190190 }
191191 }
192192 return true;
Index: trunk/extensions/CentralNotice/patches/patch-template_landingcheck.sql
@@ -1,8 +0,0 @@
2 -
3 -ALTER TABLE /*$wgDBprefix*/cn_templates ADD `tmp_landingcheck` bool NOT NULL DEFAULT 0 AFTER `tmp_fundraising`;
4 -
5 -ALTER TABLE /*$wgDBprefix*/cn_template_log ADD `tmplog_begin_landingcheck` bool NULL DEFAULT NULL AFTER `tmplog_end_fundraising`;
6 -ALTER TABLE /*$wgDBprefix*/cn_template_log ADD `tmplog_end_landingcheck` bool NULL DEFAULT NULL AFTER `tmplog_begin_landingcheck`;
\ No newline at end of file
Index: trunk/extensions/CentralNotice/patches/patch-template_autolink.sql
@@ -0,0 +1,8 @@
 2+-- Update to add a separate flag for automatic link creation
 3+
 4+-- Store a flag indicating whether or not this banner uses automatic link creation
 5+ALTER TABLE /*$wgDBprefix*/cn_templates ADD `tmp_autolink` bool NOT NULL DEFAULT 0 AFTER `tmp_fundraising`;
 6+
 7+-- Store before and after flag values for logging
 8+ALTER TABLE /*$wgDBprefix*/cn_template_log ADD `tmplog_begin_autolink` bool NULL DEFAULT NULL AFTER `tmplog_end_fundraising`;
 9+ALTER TABLE /*$wgDBprefix*/cn_template_log ADD `tmplog_end_autolink` bool NULL DEFAULT NULL AFTER `tmplog_begin_autolink`;
\ No newline at end of file
Index: trunk/extensions/CentralNotice/CentralNoticeBannerLogPager.php
@@ -151,8 +151,8 @@
152152 )."<br/>";
153153 $details .= wfMsg (
154154 'centralnotice-log-label',
155 - wfMsg ( 'centralnotice-landingcheck' ),
156 - ($row->tmplog_end_landingcheck ? 'on' : 'off')
 155+ wfMsg ( 'centralnotice-autolink' ),
 156+ ($row->tmplog_end_autolink ? 'on' : 'off')
157157 )."<br/>";
158158 if ( $row->tmplog_end_landingpages ) {
159159 $details .= wfMsg (
@@ -170,7 +170,7 @@
171171 $details .= $this->testBooleanChange( 'anon', $row );
172172 $details .= $this->testBooleanChange( 'account', $row );
173173 $details .= $this->testBooleanChange( 'fundraising', $row );
174 - $details .= $this->testBooleanChange( 'landingcheck', $row );
 174+ $details .= $this->testBooleanChange( 'autolink', $row );
175175 $details .= $this->testTextChange( 'landingpages', $row );
176176 if ( $row->tmplog_content_change ) {
177177 // Show changes to banner content
Index: trunk/extensions/CentralNotice/CentralNotice.i18n.php
@@ -168,7 +168,7 @@
169169 'centralnotice-anon' => 'Display to anonymous users',
170170 'centralnotice-account' => 'Display to logged in users',
171171 'centralnotice-fundraising' => 'Fundraising',
172 - 'centralnotice-landingcheck' => 'LandingCheck',
 172+ 'centralnotice-autolink' => 'Automatic link creation',
173173 'centralnotice-landingpages' => 'Landing pages',
174174 'centralnotice-banner-content' => 'Banner content',
175175 'centralnotice-banner-content-changed' => 'Changed',
@@ -292,7 +292,7 @@
293293 'centralnotice-banner-settings' => "Label for a radio button
294294
295295 {{msg-mw|Centralnotice-banner-settings}} and {{msg-mw|Centralnotice-banner-content}} are visible at [{{fullurl:meta:Special:CentralNoticeLogs|log=bannersettings}} Special:CentralNoticeLogs] – ''settings'' chooses between logs for campaigns and log for banners, ''content'' is one of the things which could be changed on a banner (see the details of a log entry). So, I would go with “banners in general” for ''settings'' and “a single banner” in ''content''.",
296 - 'centralnotice-landingcheck' => 'This is a the name of a MediaWiki extension.',
 296+ 'centralnotice-autolink' => 'Label for a setting',
297297 'centralnotice-banner-content' => "{{msg-mw|Centralnotice-banner-settings}} and {{msg-mw|Centralnotice-banner-content}} appear to be visible at [{{fullurl:meta:Special:CentralNoticeLogs|log=bannersettings}} Special:CentralNoticeLogs] – ''settings'' chooses between logs for campaigns and log for banners, ''content'' is one of the things which could be changed on a banner (see the details of a log entry). So, I would go with “banners in general” for ''settings'' and “a single banner” in ''content''.",
298298 'centralnotice-filters' => 'Label for a set of options that control filtering of logs',
299299 'centralnotice-date' => 'Label for a date selection interface',
Index: trunk/extensions/CentralNotice/CentralNotice.db.php
@@ -214,7 +214,7 @@
215215 'tmp_display_anon',
216216 'tmp_display_account',
217217 'tmp_fundraising',
218 - 'tmp_landingcheck',
 218+ 'tmp_autolink',
219219 'tmp_landing_pages',
220220 'not_name'
221221 ),
@@ -233,7 +233,7 @@
234234 'display_anon' => intval( $row->tmp_display_anon ), // display to anonymous users?
235235 'display_account' => intval( $row->tmp_display_account ), // display to logged in users?
236236 'fundraising' => intval( $row->tmp_fundraising ), // fundraising banner?
237 - 'landingcheck' => intval( $row->tmp_landingcheck ), // use LandingCheck?
 237+ 'autolink' => intval( $row->tmp_autolink ), // automatically create links?
238238 'landing_pages' => $row->tmp_landing_pages, // landing pages to link to
239239 'campaign' => $row->not_name // campaign the banner is assigned to
240240 );
@@ -265,7 +265,7 @@
266266 'tmp_display_anon',
267267 'tmp_display_account',
268268 'tmp_fundraising',
269 - 'tmp_landingcheck',
 269+ 'tmp_autolink',
270270 'tmp_landing_pages'
271271 ),
272272 array( 'tmp_name' => $bannerName ),
@@ -277,7 +277,7 @@
278278 'anon' => $row->tmp_display_anon,
279279 'account' => $row->tmp_display_account,
280280 'fundraising' => $row->tmp_fundraising,
281 - 'landingcheck' => $row->tmp_landingcheck,
 281+ 'autolink' => $row->tmp_autolink,
282282 'landingpages' => $row->tmp_landing_pages
283283 );
284284 }
Index: trunk/extensions/CentralNotice/centralnotice.js
@@ -106,12 +106,12 @@
107107 $("#geoMultiSelector").fadeOut('fast');
108108 }
109109 });
110 - // Reveal the LandingCheck interface when the LandingCheck checkbox is checked
111 - $("#landingCheck").click(function () {
112 - if ($('#landingCheck:checked').val() !== undefined) {
113 - $("#landingCheckInterface").fadeIn('fast');
 110+ // Reveal the landing page interface when the autolink checkbox is checked
 111+ $("#autolink").click(function () {
 112+ if ($('#autolink:checked').val() !== undefined) {
 113+ $("#autolinkInterface").fadeIn('fast');
114114 } else {
115 - $("#landingCheckInterface").fadeOut('fast');
 115+ $("#autolinkInterface").fadeOut('fast');
116116 }
117117 });
118118 });

Follow-up revisions

RevisionCommit summaryAuthorDate
r100100MFT r92510, r92676, r96496, r97304, r99160, r99165, r99169, r99176, r99178, r...awjrichards23:56, 17 October 2011
r100102picking up i18n changes; MFT r99033, r99145, r99160, r99166, r99176, r99181, ...awjrichards00:03, 18 October 2011

Status & tagging log