r85691 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r85690‎ | r85691 | r85692 >
Date:21:08, 8 April 2011
Author:aaron
Status:ok
Tags:
Comment:
Refactored out some form code into a GenericSubmitForm class
Modified paths:
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/business/FRGenericSubmitForm.php (added) (history)
  • /trunk/extensions/FlaggedRevs/business/PageStabilityForm.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/business/RevisionReviewForm.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php
@@ -258,6 +258,7 @@
259259 $wgAutoloadClasses['FlaggedRevision'] = $dir . 'FlaggedRevision.php';
260260
261261 # Business object classes
 262+$wgAutoloadClasses['FRGenericSubmitForm'] = $dir . 'business/FRGenericSubmitForm.php';
262263 $wgAutoloadClasses['RevisionReviewForm'] = $dir . 'business/RevisionReviewForm.php';
263264 $wgAutoloadClasses['PageStabilityForm'] = $dir . 'business/PageStabilityForm.php';
264265 $wgAutoloadClasses['PageStabilityGeneralForm'] = $dir . 'business/PageStabilityForm.php';
Index: trunk/extensions/FlaggedRevs/business/FRGenericSubmitForm.php
@@ -0,0 +1,130 @@
 2+<?php
 3+/**
 4+ * Class containing generic form business logic
 5+ * Note: edit tokens are the responsibility of the caller
 6+ * Usage: (a) set ALL form params before doing anything else
 7+ * (b) call ready() when all params are set
 8+ * (c) call submit() as needed
 9+ */
 10+abstract class FRGenericSubmitForm {
 11+ const ON_SUBMISSION = 1; # Notify functions when we are submitting
 12+
 13+ protected $inputLock = 0; # Disallow bad submissions
 14+ protected $user = null;
 15+
 16+ public function __construct( User $user ) {
 17+ $this->user = $user;
 18+ $this->initialize();
 19+ }
 20+
 21+ /**
 22+ * Initialize any parameters on construction
 23+ * @return void
 24+ */
 25+ protected function initialize() {}
 26+
 27+ /**
 28+ * Get the submitting user
 29+ * @return User
 30+ */
 31+ public function getUser() {
 32+ return $this->user;
 33+ }
 34+
 35+ /**
 36+ * Signal that inputs will be given (via accessors)
 37+ * @return void
 38+ */
 39+ public function start() {
 40+ $this->inputLock = 0;
 41+ }
 42+
 43+ /**
 44+ * Signal that inputs are all given (via accessors)
 45+ * @return mixed (true on success, error string on target failure)
 46+ */
 47+ public function ready() {
 48+ $this->inputLock = 1;
 49+ $status = $this->doCheckTarget();
 50+ if ( $status !== true ) {
 51+ return $status; // bad target
 52+ }
 53+ return $this->doLoadOnReady();
 54+ }
 55+
 56+ /**
 57+ * Set a member field to a value if the fields are unlocked
 58+ * @param mixed &$field Field of this form
 59+ * @param mixed $value Value to set the field to
 60+ * @return void
 61+ */
 62+ protected function trySet( &$field, $value ) {
 63+ if ( $this->inputLock ) {
 64+ throw new MWException( __CLASS__ . " fields cannot be set anymore.\n");
 65+ } else {
 66+ $field = $value; // still allowing input
 67+ }
 68+ }
 69+
 70+ /*
 71+ * Check that the target is valid (e.g. from GET/POST request)
 72+ * @param int $flags ON_SUBMISSION (set on submit)
 73+ * @return mixed (true on success, error string on failure)
 74+ */
 75+ protected function doCheckTarget( $flags = 0 ) {
 76+ return true;
 77+ }
 78+
 79+ /**
 80+ * Load any parameters after ready() called
 81+ * @return mixed (true on success, error string on failure)
 82+ */
 83+ protected function doLoadOnReady() {
 84+ return true;
 85+ }
 86+
 87+ /*
 88+ * Verify and clean up parameters (e.g. from POST request)
 89+ * @return mixed (true on success, error string on failure)
 90+ */
 91+ protected function checkParameters() {
 92+ $status = $this->doCheckTarget( self::ON_SUBMISSION );
 93+ if ( $status !== true ) {
 94+ return $status; // bad target
 95+ }
 96+ return $this->doCheckParameters();
 97+ }
 98+
 99+ /*
 100+ * Verify and clean up parameters (e.g. from POST request)
 101+ * @return mixed (true on success, error string on failure)
 102+ */
 103+ protected function doCheckParameters() {
 104+ return true;
 105+ }
 106+
 107+ /**
 108+ * Submit the form parameters for the page config to the DB.
 109+ *
 110+ * @return mixed (true on success, error string on failure)
 111+ */
 112+ public function submit() {
 113+ if ( !$this->inputLock ) {
 114+ throw new MWException( __CLASS__ . " input fields not set yet.\n");
 115+ }
 116+ $status = $this->checkParameters();
 117+ if ( $status !== true ) {
 118+ return $status; // cannot submit - broken params
 119+ }
 120+ return $this->doSubmit();
 121+ }
 122+
 123+ /**
 124+ * Submit the form parameters for the page config to the DB.
 125+ *
 126+ * @return mixed (true on success, error string on failure)
 127+ */
 128+ protected function doSubmit() {
 129+ return true;
 130+ }
 131+}
Property changes on: trunk/extensions/FlaggedRevs/business/FRGenericSubmitForm.php
___________________________________________________________________
Added: svn:eol-style
1132 + native
Index: trunk/extensions/FlaggedRevs/business/RevisionReviewForm.php
@@ -1,12 +1,8 @@
22 <?php
33 /**
44 * Class containing revision review form business logic
5 - * Note: edit tokens are the responsibility of caller
6 - * Usage: (a) set ALL form params before doing anything else
7 - * (b) call ready() when all params are set
8 - * (c) call submit() as needed
95 */
10 -class RevisionReviewForm {
 6+class RevisionReviewForm extends FRGenericSubmitForm {
117 /* Form parameters which can be user given */
128 protected $page = null;
139 protected $approve = false;
@@ -24,21 +20,13 @@
2521 protected $newLastChangeTime = null; # Conflict handling
2622
2723 protected $oflags = array();
28 - protected $inputLock = 0; # Disallow bad submissions
2924
30 - protected $user = null;
31 -
32 - public function __construct( User $user ) {
33 - $this->user = $user;
 25+ public function initialize() {
3426 foreach ( FlaggedRevs::getTags() as $tag ) {
3527 $this->dims[$tag] = 0; // default to "inadequate"
3628 }
3729 }
3830
39 - public function getUser() {
40 - return $this->user;
41 - }
42 -
4331 public function getPage() {
4432 return $this->page;
4533 }
@@ -135,52 +123,31 @@
136124 }
137125
138126 /**
139 - * Set a member field to a value if the fields are unlocked
140 - */
141 - protected function trySet( &$field, $value ) {
142 - if ( $this->inputLock ) {
143 - throw new MWException( __CLASS__ . " fields cannot be set anymore.\n");
144 - } else {
145 - $field = $value; // still allowing input
146 - }
147 - }
148 -
149 - /**
150 - * Signal that inputs are starting
151 - */
152 - public function start() {
153 - $this->inputLock = 0;
154 - }
155 -
156 - /**
157 - * Signal that inputs are done and load old config
 127+ * Load old config and last review status change time
158128 * @return mixed (true on success, error string on target failure)
159129 */
160 - public function ready() {
161 - $this->inputLock = 1;
162 - $status = $this->checkTarget();
163 - if ( $status !== true ) {
164 - return $status; // bad target
165 - }
 130+ public function doLoadOnReady() {
166131 # Get the revision's current flags, if any
167132 $this->oflags = FlaggedRevs::getRevisionTags( $this->page, $this->oldid, FR_MASTER );
168133 # Set initial value for newLastChangeTime (if unchanged on submit)
169134 $this->newLastChangeTime = $this->lastChangeTime;
170 - return $status;
 135+ return true;
171136 }
172137
173138 /*
174 - * Check that the target page is valid
 139+ * Check that the target is valid (e.g. from GET/POST request)
 140+ * @param int $flags ON_SUBMISSION (set on submit)
175141 * @return mixed (true on success, error string on failure)
176142 */
177 - protected function checkTarget() {
 143+ protected function doCheckTarget( $flags = 0 ) {
178144 if ( is_null( $this->page ) ) {
179145 return 'review_page_invalid';
180146 } elseif ( !$this->page->exists() ) {
181147 return 'review_page_notexists';
182148 }
183149 $fa = FlaggedArticle::getTitleInstance( $this->page );
184 - if ( !$fa->isReviewable() ) {
 150+ $flags = ( $flags & self::ON_SUBMISSION ) ? FR_MASTER : 0;
 151+ if ( !$fa->isReviewable( $flags ) ) {
185152 return 'review_page_unreviewable';
186153 }
187154 return true;
@@ -190,11 +157,8 @@
191158 * Verify and clean up parameters (e.g. from POST request).
192159 * @return mixed (true on success, error string on failure)
193160 */
194 - protected function checkSettings() {
195 - $status = $this->checkTarget();
196 - if ( $status !== true ) {
197 - return $status; // bad page target
198 - } elseif ( !$this->oldid ) {
 161+ protected function doCheckParameters() {
 162+ if ( !$this->oldid ) {
199163 return 'review_no_oldid'; // bad revision target
200164 }
201165 # Check that an action is specified (approve, reject, de-approve)
@@ -275,14 +239,7 @@
276240 *
277241 * @return mixed (true on success, error string on failure)
278242 */
279 - public function submit() {
280 - if ( !$this->inputLock ) {
281 - throw new MWException( __CLASS__ . " input fields not set yet.\n");
282 - }
283 - $status = $this->checkSettings();
284 - if ( $status !== true ) {
285 - return $status; // cannot submit - broken params
286 - }
 243+ public function doSubmit() {
287244 # Double-check permissions
288245 if ( !$this->isAllowed() ) {
289246 return 'review_denied';
Index: trunk/extensions/FlaggedRevs/business/PageStabilityForm.php
@@ -1,12 +1,8 @@
22 <?php
33 /**
44 * Class containing stability settings form business logic
5 - * Note: edit tokens are the responsibility of caller
6 - * Usage: (a) set ALL form params before doing anything else
7 - * (b) call ready() when all params are set
8 - * (c) call preloadSettings() or submit() as needed
95 */
10 -abstract class PageStabilityForm {
 6+abstract class PageStabilityForm extends FRGenericSubmitForm {
117 /* Form parameters which can be user given */
128 protected $page = false; # Target page obj
139 protected $watchThis = null; # Watch checkbox
@@ -19,18 +15,7 @@
2016 protected $autoreview = ''; # Autoreview restrictions...
2117
2218 protected $oldConfig = array(); # Old page config
23 - protected $inputLock = 0; # Disallow bad submissions
2419
25 - protected $user = null;
26 -
27 - public function __construct( User $user ) {
28 - $this->user = $user;
29 - }
30 -
31 - public function getUser() {
32 - return $this->user;
33 - }
34 -
3520 public function getPage() {
3621 return $this->page;
3722 }
@@ -133,38 +118,6 @@
134119 return $comment;
135120 }
136121
137 - /**
138 - * Set a member field to a value if the fields are unlocked
139 - */
140 - protected function trySet( &$field, $value ) {
141 - if ( $this->inputLock ) {
142 - throw new MWException( __CLASS__ . " fields cannot be set anymore.\n");
143 - } else {
144 - $field = $value; // still allowing input
145 - }
146 - }
147 -
148 - /**
149 - * Signal that inputs are starting
150 - */
151 - public function start() {
152 - $this->inputLock = 0;
153 - }
154 -
155 - /**
156 - * Signal that inputs are done and load old config
157 - * @return mixed (true on success, error string on target failure)
158 - */
159 - public function ready() {
160 - $this->inputLock = 1;
161 - $status = $this->checkTarget();
162 - if ( $status !== true ) {
163 - return $status; // bad target
164 - }
165 - $this->loadOldConfig(); // current settings from DB
166 - return $status;
167 - }
168 -
169122 /*
170123 * Preload existing page settings (e.g. from GET request).
171124 * @return mixed (true on success, error string on failure)
@@ -173,7 +126,7 @@
174127 if ( !$this->inputLock ) {
175128 throw new MWException( __CLASS__ . " input fields not set yet.\n");
176129 }
177 - $status = $this->checkTarget();
 130+ $status = $this->doCheckTarget();
178131 if ( $status !== true ) {
179132 return $status; // bad target
180133 }
@@ -196,31 +149,28 @@
197150 * Verify and clean up parameters (e.g. from POST request).
198151 * @return mixed (true on success, error string on failure)
199152 */
200 - protected function checkSettings() {
201 - $status = $this->checkTarget();
202 - if ( $status !== true ) {
203 - return $status; // bad target
204 - }
 153+ protected function doCheckParameters() {
205154 if ( $this->expiryCustom != '' ) {
206155 // Custom expiry takes precedence
207156 $this->expirySelection = 'othertime';
208157 }
209 - $status = $this->reallyCheckSettings(); // check other params...
 158+ $status = $this->reallyDoCheckParameters(); // check other params...
210159 return $status;
211160 }
212161
213162 /*
214163 * @return mixed (true on success, error string on failure)
215164 */
216 - protected function reallyCheckSettings() {
 165+ protected function reallyDoCheckParameters() {
217166 return true;
218167 }
219168
220169 /*
221170 * Check that the target page is valid
 171+ * @param int $flags ON_SUBMISSION (set on submit)
222172 * @return mixed (true on success, error string on failure)
223173 */
224 - protected function checkTarget() {
 174+ protected function doCheckTarget( $flags = 0 ) {
225175 if ( is_null( $this->page ) ) {
226176 return 'stabilize_page_invalid';
227177 } elseif ( !$this->page->exists() ) {
@@ -231,9 +181,10 @@
232182 return true;
233183 }
234184
235 - protected function loadOldConfig() {
 185+ protected function doLoadOnReady() {
236186 # Get the current page config
237187 $this->oldConfig = FlaggedPageConfig::getPageStabilitySettings( $this->page, FR_MASTER );
 188+ return true;
238189 }
239190
240191 /*
@@ -256,14 +207,7 @@
257208 *
258209 * @return mixed (true on success, error string on failure)
259210 */
260 - public function submit() {
261 - if ( !$this->inputLock ) {
262 - throw new MWException( __CLASS__ . " input fields not set yet.\n");
263 - }
264 - $status = $this->checkSettings();
265 - if ( $status !== true ) {
266 - return $status; // cannot submit - broken params
267 - }
 211+ public function doSubmit() {
268212 # Double-check permissions
269213 if ( !$this->isAllowed() ) {
270214 return 'stablize_denied';
@@ -435,7 +379,7 @@
436380 return true;
437381 }
438382
439 - protected function reallyCheckSettings() {
 383+ protected function reallyDoCheckParameters() {
440384 $this->override = $this->override ? 1 : 0; // default version settings is 0 or 1
441385 // Check autoreview restriction setting
442386 if ( $this->autoreview != '' // restriction other than 'none'
@@ -533,12 +477,12 @@
534478 return true;
535479 }
536480
537 - protected function reallyCheckSettings() {
 481+ protected function reallyDoCheckParameters() {
538482 # WMF temp hack...protection limit quota
539483 global $wgFlaggedRevsProtectQuota;
540484 if ( isset( $wgFlaggedRevsProtectQuota ) // quota exists
541485 && $this->autoreview != '' // and we are protecting
542 - && FlaggedPageConfig::getProtectionLevel( $this->oldConfig ) == 'none' ) // page unprotected
 486+ && FlaggedPageConfig::getProtectionLevel( $this->oldConfig ) == 'none' ) // unprotected
543487 {
544488 $dbw = wfGetDB( DB_MASTER );
545489 $count = $dbw->selectField( 'flaggedpage_config', 'COUNT(*)', '', __METHOD__ );

Status & tagging log