r77050 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r77049‎ | r77050 | r77051 >
Date:01:30, 20 November 2010
Author:aaron
Status:deferred (Comments)
Tags:
Comment:
*More on r76962:
**Use $wgContLang as needed rather than $wgLang
**Use content NS text, not hardcoded english "User"
**Skip over rev_deleted user names
**Added user of old rev to summary make things easier to find in history
*Cleaned up raw rev->mDeleted check
*Made review comment box slightly larger
*Tweaked error handling
Modified paths:
  • /trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/language/FlaggedRevs.i18n.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/specialpages/RevisionReview_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/language/FlaggedRevs.i18n.php
——@@ -101,6 +101,7 @@
102102 'review_denied' => 'Permission denied.',
103103 'review_param_missing' => 'A parameter is missing or invalid.',
104104 'review_cannot_undo' => 'Cannot undo these changes because further pending edits changed the same areas.',
 105+ 'review_reject_excessive' => 'Cannot reject this many edits at once.',
105106
106107 'revreview-current' => 'Pending changes',
107108 'revreview-depth' => 'Depth',
——@@ -256,12 +257,11 @@
257258 'revreview-reject-summary' => 'Edit summary:',
258259 'revreview-reject-confirm' => 'Reject these changes',
259260 'revreview-reject-cancel' => 'Cancel',
260 - 'revreview-reject-summary-cur' => 'Rejected the last {{PLURAL:$1|one change|$1 changes}} (by $2) and restored revision $3',
261 - 'revreview-reject-summary-old' => 'Rejected the first {{PLURAL:$1|one change|$1 changes}} (by $2) following revision $3',
262 - 'revreview-reject-summary-cur-short' => 'Rejected the last {{PLURAL:$1|one change|$1 changes}} and restored revision $2',
263 - 'revreview-reject-summary-old-short' => 'Rejected the first {{PLURAL:$1|one change|$1 changes}} following revision $2',
 261+ 'revreview-reject-summary-cur' => 'Rejected the last {{PLURAL:$1|one change|$1 changes}} (by $2) and restored revision $3 by $4',
 262+ 'revreview-reject-summary-old' => 'Rejected the first {{PLURAL:$1|one change|$1 changes}} (by $2) that followed revision $3 by $4',
 263+ 'revreview-reject-summary-cur-short' => 'Rejected the last {{PLURAL:$1|one change|$1 changes}} and restored revision $2 by $3',
 264+ 'revreview-reject-summary-old-short' => 'Rejected the first {{PLURAL:$1|one change|$1 changes}} that followed revision $2 by $3',
264265 'revreview-reject-usercount' => '{{PLURAL:$1|one user|$1 users}}',
265 - 'revreview-reject-toomanyedits' => 'Cannot reject this many edits at once.',
266266
267267 'revreview-reviewlink' => 'pending edits',
268268 'revreview-reviewlink-title' => 'View diff of all pending changes',
Index: trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php
——@@ -313,15 +313,15 @@
314314 $newRev = Revision::newFromTitle( $this->page, $this->oldid );
315315 $oldRev = Revision::newFromTitle( $this->page, $this->refid );
316316 # Do not mess with archived/deleted revisions
317 - if ( is_null( $oldRev ) || $oldRev->mDeleted ) {
 317+ if ( !$oldRev || $newRev->isDeleted( Revision::DELETED_TEXT ) ) {
318318 return 'review_bad_oldid';
319 - } elseif ( is_null( $newRev ) || $newRev->mDeleted ) {
 319+ } elseif ( !$newRev || $newRev->isDeleted( Revision::DELETED_TEXT ) ) {
320320 return 'review_bad_oldid';
321321 }
322322 # Go to confirmation screen first
323323 if ( !$this->rejectConfirm ) {
324 - $this->rejectConfirmationForm( $oldRev, $newRev );
325 - return false; // xxx
 324+ $status = $this->rejectConfirmationForm( $oldRev, $newRev );
 325+ return is_string( $status ) ? $status : false; // xxx
326326 }
327327 $article = new Article( $this->page );
328328 $new_text = $article->getUndoText( $newRev, $oldRev );
——@@ -747,7 +747,7 @@
748748 $form .= "<br />"; // Don't put too much on one line
749749 }
750750 $form .= "<span id='mw-fr-commentbox' style='clear:both'>" .
751 - Xml::inputLabel( wfMsg( 'revreview-log' ), 'wpReason', 'wpReason', 35, '',
 751+ Xml::inputLabel( wfMsg( 'revreview-log' ), 'wpReason', 'wpReason', 40, '',
752752 array( 'class' => 'fr-comment-box' ) ) . "&#160;&#160;&#160;</span>";
753753 }
754754 # Determine if there will be reject button
——@@ -997,16 +997,17 @@
998998 * A bit hacky, but we don't have a way to pass more complicated
999999 * UI things back up, since RevisionReview expects either true
10001000 * or a string message key
 1001+ * @return mixed (string/true)
10011002 */
10021003 private function rejectConfirmationForm( Revision $oldRev, Revision $newRev ) {
1003 - global $wgOut, $wgLang;
 1004+ global $wgOut, $wgLang, $wgContLang;
10041005 $thisPage = SpecialPage::getTitleFor( 'RevisionReview' );
10051006
10061007 $wgOut->addHtml( '<div class="plainlinks">' );
10071008
10081009 $dbr = wfGetDB( DB_SLAVE );
10091010 $res = $dbr->select( 'revision',
1010 - array( 'rev_id', 'rev_user_text' ),
 1011+ Revision::selectFields(),
10111012 array(
10121013 'rev_page' => $oldRev->getPage(),
10131014 'rev_id > ' . $dbr->addQuotes( $oldRev->getId() ),
——@@ -1016,16 +1017,19 @@
10171018 array( 'LIMIT' => 251 ) // sanity check
10181019 );
10191020 if ( !$dbr->numRows( $res ) ) {
1020 - $wgOut->redirect( $this->getPage()->getFullUrl() ); // sanity check
1021 - return;
 1021+ return 'review_bad_oldid';
10221022 } elseif ( $dbr->numRows( $res ) > 250 ) {
1023 - $wgOut->showErrorPage( 'internalerror', 'revreview-reject-toomanyedits' );
1024 - return;
 1023+ return 'review_reject_excessive';
10251024 }
10261025
1027 - $rejectIds = array();
1028 - foreach ( $res as $r ) {
1029 - $rejectIds[$r->rev_id] = "[[User:{$r->rev_user_text}|{$r->rev_user_text}]]";
 1026+ $rejectIds = $rejectAuthors = array();
 1027+ $UserNS = $wgContLang->getNsText( NS_USER );
 1028+ foreach ( $res as $row ) {
 1029+ $rev = new Revision( $row );
 1030+ $rejectIds[] = $rev->getId();
 1031+ $rejectAuthors[] = $rev->isDeleted( Revision::DELETED_USER )
 1032+ ? wfMsg( 'rev-deleted-user' )
 1033+ : "[[{$UserNS}:{$rev->getUserText()}|{$rev->getUserText()}]]";
10301034 }
10311035
10321036 // List of revisions being undone...
——@@ -1034,8 +1038,7 @@
10351039 // FIXME: we need a generic revision list class
10361040 $spRevDelete = SpecialPage::getPage( 'RevisionReview' );
10371041 $spRevDelete->skin = $this->user->getSkin(); // XXX
1038 - $list = new RevDel_RevisionList( $spRevDelete, $oldRev->getTitle(),
1039 - array_keys( $rejectIds ) );
 1042+ $list = new RevDel_RevisionList( $spRevDelete, $oldRev->getTitle(), $rejectIds );
10401043 for ( $list->reset(); $list->current(); $list->next() ) {
10411044 $item = $list->current();
10421045 if ( $item->canView() ) {
——@@ -1052,23 +1055,28 @@
10531056 }
10541057
10551058 // Determine the default edit summary...
 1059+ $oldRevAuthor = $oldRev->isDeleted( Revision::DELETED_USER )
 1060+ ? wfMsg( 'rev-deleted-user' )
 1061+ : $oldRev->getUserText();
10561062 // NOTE: *-cur msg wording not safe for (unlikely) edit auto-merge
1057 - $rejectAuthors = array_values( array_unique( $rejectIds ) );
 1063+ $rejectAuthors = array_values( array_unique( $rejectAuthors ) );
10581064 if ( count( $rejectAuthors ) > 3 ) {
10591065 $msg = $newRev->isCurrent()
10601066 ? 'revreview-reject-summary-cur-short'
10611067 : 'revreview-reject-summary-old-short';
10621068 $defaultSummary = wfMsgExt( $msg, 'parsemag',
1063 - $wgLang->formatNum( count( $rejectIds ) ), $oldRev->getId() );
 1069+ $wgContLang->formatNum( count( $rejectIds ) ),
 1070+ $oldRev->getId(),
 1071+ $oldRevAuthor );
10641072 } else {
10651073 $msg = $newRev->isCurrent()
10661074 ? 'revreview-reject-summary-cur'
10671075 : 'revreview-reject-summary-old';
10681076 $defaultSummary = wfMsgExt( $msg, 'parsemag',
1069 - $wgLang->formatNum( count( $rejectIds ) ),
1070 - $wgLang->listToText( $rejectAuthors ),
1071 - $oldRev->getId()
1072 - );
 1077+ $wgContLang->formatNum( count( $rejectIds ) ),
 1078+ $wgContLang->listToText( $rejectAuthors ),
 1079+ $oldRev->getId(),
 1080+ $oldRevAuthor );
10731081 }
10741082 // Append any review comment...
10751083 if ( $this->comment != '' ) {
——@@ -1099,6 +1107,7 @@
11001108 $form .= Xml::closeElement( 'form' );
11011109
11021110 $wgOut->addHtml( $form );
 1111+ return true;
11031112 }
11041113
11051114 private function getSpecialLinks() {
Index: trunk/extensions/FlaggedRevs/specialpages/RevisionReview_body.php
——@@ -107,7 +107,7 @@
108108 } elseif ( $form->getAction() === 'reject' ) {
109109 $wgOut->redirect( $this->page->getFullUrl() );
110110 }
111 - } elseif( $status === false ) {
 111+ } elseif ( $status === false ) {
112112 // Reject confirmation screen. HACKY :(
113113 return;
114114 } else {

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r76962Message cleanup for r75408:...demon21:25, 18 November 2010

Comments

#Comment by Aaron Schulz (talk | contribs)   01:35, 20 November 2010

Ugh..not "Skip over rev_deleted user names" but rather "use (user hidden)".

Status & tagging log