r71996 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r71995‎ \| r71996 \| r71997 >
Date:	00:27, 31 August 2010
Author:	kaldari
Status:	ok
Tags:
Comment:	redoing how errors are handled for Post/Redirect/Get (fix for r71832), removing extra whitespace
Modified paths:	/trunk/extensions/CentralNotice/SpecialCentralNotice.php (modified) (history) /trunk/extensions/CentralNotice/SpecialNoticeTemplate.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/SpecialCentralNotice.php
—	—	@@ -6,9 +6,8 @@
7	7	}
8	8
9	9	class CentralNotice extends SpecialPage {
10		~~- var $centralNoticeDB;~~
11		~~- /* Functions */~~
12		-
	10	+ var $centralNoticeDB, $editable, $centralNoticeError;
	11	+
13	12	function __construct() {
14	13	// Register special page
15	14	parent::__construct( 'CentralNotice' );
—	—	@@ -36,6 +35,9 @@
37	36
38	37	// Check permissions
39	38	$this->editable = $wgUser->isAllowed( 'centralnotice-admin' );
	39	+
	40	+ // Initialize error variable
	41	+ $this->centralNoticeError = false;
40	42
41	43	// Show summary
42	44	$wgOut->addWikiText( wfMsg( 'centralnotice-summary' ) );
—	—	@@ -70,7 +72,7 @@
71	73	$this->removeNotice( $notice );
72	74	}
73	75
74		~~- // Show list of campaigns~~
	76	+ // Skip subsequent form handling and show list of campaigns
75	77	$this->listNotices();
76	78	$wgOut->addHTML( Xml::closeElement( 'div' ) );
77	79	return;
—	—	@@ -146,14 +148,20 @@
147	149	$project_name = $wgRequest->getVal( 'project_name' );
148	150	$project_languages = $wgRequest->getArray( 'project_languages' );
149	151	if ( $noticeName == '' ) {
150		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-null-string' );~~
	152	+ $this->showError( 'centralnotice-null-string' );
151	153	} else {
152	154	$this->addNotice( $noticeName, '0', $start, $project_name, $project_languages );
153	155	}
154	156	}
155		-
	157	+
	158	+ // If there were no errors, reload the page to prevent duplicate form submission
	159	+ if ( !$this->centralNoticeError ) {
	160	+ $wgOut->redirect( $this->getTitle()->getLocalUrl() );
	161	+ return;
	162	+ }
	163	+
156	164	} else {
157		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );~~
	165	+ $this->showError( 'sessionfailure' );
158	166	}
159	167
160	168	}
—	—	@@ -521,7 +529,7 @@
522	530
523	531	// Make sure notice exists
524	532	if ( !$this->noticeExists( $notice ) ) {
525		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-doesnt-exist' );~~
	533	+ $this->showError( 'centralnotice-notice-doesnt-exist' );
526	534	} else {
527	535
528	536	// Handle form submissions from campaign detail interface
—	—	@@ -533,10 +541,11 @@
534	542	// Handle removing campaign
535	543	if ( $wgRequest->getVal( 'remove' ) ) {
536	544	$this->removeNotice( $notice );
537		-
538		~~- // Leave campaign detail interface~~
539		~~- $wgOut->redirect( $this->getTitle()->getLocalUrl() );~~
540		~~- return;~~
	545	+ if ( !$this->centralNoticeError ) {
	546	+ // Leave campaign detail interface
	547	+ $wgOut->redirect( $this->getTitle()->getLocalUrl() );
	548	+ return;
	549	+ }
541	550	}
542	551
543	552	// Handle locking/unlocking campaign
—	—	@@ -615,8 +624,13 @@
616	625	$this->updateProjectLanguages( $notice, $projectLangs );
617	626	}
618	627
	628	+ // If there were no errors, reload the page to prevent duplicate form submission
	629	+ if ( !$this->centralNoticeError ) {
	630	+ $wgOut->redirect( $this->getTitle()->getLocalUrl( "method=listNoticeDetail&notice=$notice" ) );
	631	+ return;
	632	+ }
619	633	} else {
620		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );~~
	634	+ $this->showError( 'sessionfailure' );
621	635	}
622	636
623	637	}
—	—	@@ -983,10 +997,10 @@
984	998	global $wgOut;
985	999
986	1000	if ( $this->noticeExists( $noticeName ) ) {
987		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-exists' );~~
	1001	+ $this->showError( 'centralnotice-notice-exists' );
988	1002	return;
989	1003	} elseif ( empty( $project_languages ) ) {
990		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-no-language' );~~
	1004	+ $this->showError( 'centralnotice-no-language' );
991	1005	return;
992	1006	} else {
993	1007	$dbw = wfGetDB( DB_MASTER );
—	—	@@ -1036,22 +1050,22 @@
1037	1051	array( 'not_name' => $noticeName )
1038	1052	);
1039	1053	if ( $dbr->numRows( $res ) < 1 ) {
1040		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-remove-notice-doesnt-exist' );~~
1041		~~- return;~~
	1054	+ $this->showError( 'centralnotice-remove-notice-doesnt-exist' );
	1055	+ return;
1042	1056	}
1043	1057	$row = $dbr->fetchObject( $res );
1044	1058	if ( $row->not_locked == '1' ) {
1045		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-is-locked' );~~
1046		~~- return;~~
	1059	+ $this->showError( 'centralnotice-notice-is-locked' );
	1060	+ return;
1047	1061	} else {
1048		~~- $dbw = wfGetDB( DB_MASTER );~~
1049		~~- $dbw->begin();~~
1050		~~- $noticeId = htmlspecialchars( $this->getNoticeId( $noticeName ) );~~
1051		~~- $res = $dbw->delete( 'cn_assignments', array ( 'not_id' => $noticeId ) );~~
1052		~~- $res = $dbw->delete( 'cn_notices', array ( 'not_name' => $noticeName ) );~~
1053		~~- $res = $dbw->delete( 'cn_notice_languages', array ( 'nl_notice_id' => $noticeId ) );~~
1054		~~- $dbw->commit();~~
1055		~~- return;~~
	1062	+ $dbw = wfGetDB( DB_MASTER );
	1063	+ $dbw->begin();
	1064	+ $noticeId = htmlspecialchars( $this->getNoticeId( $noticeName ) );
	1065	+ $res = $dbw->delete( 'cn_assignments', array ( 'not_id' => $noticeId ) );
	1066	+ $res = $dbw->delete( 'cn_notices', array ( 'not_name' => $noticeName ) );
	1067	+ $res = $dbw->delete( 'cn_notice_languages', array ( 'nl_notice_id' => $noticeId ) );
	1068	+ $dbw->commit();
	1069	+ return;
1056	1070	}
1057	1071	}
1058	1072
—	—	@@ -1070,7 +1084,7 @@
1071	1085	)
1072	1086	);
1073	1087	if ( $dbr->numRows( $res ) > 0 ) {
1074		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-template-already-exists' );~~
	1088	+ $this->showError( 'centralnotice-template-already-exists' );
1075	1089	} else {
1076	1090	$dbw = wfGetDB( DB_MASTER );
1077	1091	$dbw->begin();
—	—	@@ -1146,13 +1160,13 @@
1147	1161
1148	1162	// Start/end don't line up
1149	1163	if ( $start > $end \|\| $end < $start ) {
1150		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-invalid-date-range' );~~
	1164	+ $this->showError( 'centralnotice-invalid-date-range' );
1151	1165	return;
1152	1166	}
1153	1167
1154	1168	// Invalid campaign name
1155	1169	if ( !$this->noticeExists( $noticeName ) ) {
1156		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-doesnt-exist' );~~
	1170	+ $this->showError( 'centralnotice-notice-doesnt-exist' );
1157	1171	return;
1158	1172	}
1159	1173
—	—	@@ -1177,7 +1191,7 @@
1178	1192	global $wgOut;
1179	1193
1180	1194	if ( !$this->noticeExists( $noticeName ) ) {
1181		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-doesnt-exist' );~~
	1195	+ $this->showError( 'centralnotice-doesnt-exist' );
1182	1196	} else {
1183	1197	$dbw = wfGetDB( DB_MASTER );
1184	1198	$res = $dbw->update( 'cn_notices',
—	—	@@ -1194,7 +1208,7 @@
1195	1209	global $wgOut;
1196	1210
1197	1211	if ( !$this->noticeExists( $noticeName ) ) {
1198		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-doesnt-exist' );~~
	1212	+ $this->showError( 'centralnotice-doesnt-exist' );
1199	1213	} else {
1200	1214	$dbw = wfGetDB( DB_MASTER );
1201	1215	$res = $dbw->update( 'cn_notices',
—	—	@@ -1211,7 +1225,7 @@
1212	1226	global $wgOut;
1213	1227
1214	1228	if ( !$this->noticeExists( $noticeName ) ) {
1215		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-doesnt-exist' );~~
	1229	+ $this->showError( 'centralnotice-doesnt-exist' );
1216	1230	} else {
1217	1231	$dbw = wfGetDB( DB_MASTER );
1218	1232	$res = $dbw->update( 'cn_notices',
—	—	@@ -1370,6 +1384,12 @@
1371	1385	}
1372	1386	return $text;
1373	1387	}
	1388	+
	1389	+ function showError( $message ) {
	1390	+ global $wgOut;
	1391	+ $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", $message );
	1392	+ $this->centralNoticeError = true;
	1393	+ }
1374	1394	}
1375	1395
1376	1396
Index: trunk/extensions/CentralNotice/SpecialNoticeTemplate.php
—	—	@@ -6,7 +6,7 @@
7	7	}
8	8
9	9	class SpecialNoticeTemplate extends UnlistedSpecialPage {
10		~~- var $editable;~~
	10	+ var $editable, $centralNoticeError;
11	11
12	12	function __construct() {
13	13	parent::__construct( 'NoticeTemplate' );
—	—	@@ -32,6 +32,9 @@
33	33
34	34	// Check permissions
35	35	$this->editable = $wgUser->isAllowed( 'centralnotice-admin' );
	36	+
	37	+ // Initialize error variable
	38	+ $this->centralNoticeError = false;
36	39
37	40	// Show summary
38	41	$wgOut->addWikiMsg( 'centralnotice-summary' );
—	—	@@ -86,7 +89,7 @@
87	90	);
88	91	$sub = 'view';
89	92	} else {
90		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-null-string' );~~
	93	+ $this->showError( 'centralnotice-null-string' );
91	94	}
92	95	}
93	96
—	—	@@ -102,7 +105,7 @@
103	106	}
104	107
105	108	} else {
106		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );~~
	109	+ $this->showError( 'sessionfailure' );
107	110	}
108	111
109	112	}
—	—	@@ -144,7 +147,7 @@
145	148	return;
146	149
147	150	} else {
148		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );~~
	151	+ $this->showError( 'sessionfailure' );
149	152	}
150	153
151	154	}
—	—	@@ -612,7 +615,7 @@
613	616	$res = $dbr->select( 'cn_assignments', 'asn_id', array( 'tmp_id' => $id ), __METHOD__ );
614	617
615	618	if ( $dbr->numRows( $res ) > 0 ) {
616		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-template-still-bound' );~~
	619	+ $this->showError( 'centralnotice-template-still-bound' );
617	620	return;
618	621	} else {
619	622	$dbw = wfGetDB( DB_MASTER );
—	—	@@ -637,7 +640,7 @@
638	641	global $wgOut;
639	642
640	643	if ( $body == '' \|\| $name == '' ) {
641		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-null-string' );~~
	644	+ $this->showError( 'centralnotice-null-string' );
642	645	return;
643	646	}
644	647
—	—	@@ -653,7 +656,7 @@
654	657	);
655	658
656	659	if ( $dbr->numRows( $res ) > 0 ) {
657		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-template-exists' );~~
	660	+ $this->showError( 'centralnotice-template-exists' );
658	661	return false;
659	662	} else {
660	663	$dbw = wfGetDB( DB_MASTER );
—	—	@@ -682,7 +685,7 @@
683	686	global $wgOut;
684	687
685	688	if ( $body == '' \|\| $name == '' ) {
686		~~- $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-null-string' );~~
	689	+ $this->showError( 'centralnotice-null-string' );
687	690	return;
688	691	}
689	692
—	—	@@ -805,4 +808,10 @@
806	809	}
807	810	return $translations;
808	811	}
	812	+
	813	+ function showError( $message ) {
	814	+ global $wgOut;
	815	+ $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", $message );
	816	+ $this->centralNoticeError = true;
	817	+ }
809	818	}

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r71832	moving wasPosted() test inside notice existance test	kaldari	00:38, 28 August 2010

Status & tagging log

00:31, 31 August 2010 Catrope (talk | contribs) changed the status of r71996 [removed: new added: ok]