r71832 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r71831‎ | r71832 | r71833 >
Date:00:38, 28 August 2010
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
moving wasPosted() test inside notice existance test
Modified paths:
  • /trunk/extensions/CentralNotice/SpecialCentralNotice.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/SpecialCentralNotice.php
——@@ -573,65 +573,61 @@
574574
575575 function listNoticeDetail( $notice ) {
576576 global $wgOut, $wgRequest, $wgUser;
577 -
578 - if ( $wgRequest->wasPosted() ) {
 577+
 578+ // Make sure notice exists
 579+ if ( !$this->noticeExists( $notice ) ) {
 580+ $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-doesnt-exist' );
 581+ } else {
 582+
 583+ if ( $wgRequest->wasPosted() ) {
 584+
 585+ // Check authentication token
 586+ if ( $wgUser->matchEditToken( $wgRequest->getVal( 'authtoken' ) ) ) {
 587+
 588+ // Handle adding of banners to the campaign
 589+ $templatesToAdd = $wgRequest->getArray( 'addTemplates' );
 590+ if ( $templatesToAdd ) {
 591+ $weight = $wgRequest->getArray( 'weight' );
 592+ foreach ( $templatesToAdd as $templateName ) {
 593+ $templateId = $this->getTemplateId( $templateName );
 594+ $this->addTemplateTo( $notice, $templateName, $weight[$templateId] );
 595+ }
 596+ }
579597
580 - // Check authentication token
581 - if ( $wgUser->matchEditToken( $wgRequest->getVal( 'authtoken' ) ) ) {
582 -
583 - // Handle adding of banners to the campaign
584 - $templatesToAdd = $wgRequest->getArray( 'addTemplates' );
585 - if ( $templatesToAdd ) {
586 - $weight = $wgRequest->getArray( 'weight' );
587 - foreach ( $templatesToAdd as $templateName ) {
588 - $templateId = $this->getTemplateId( $templateName );
589 - $this->addTemplateTo( $notice, $templateName, $weight[$templateId] );
 598+ // Handle removing of banners from the campaign
 599+ $templateToRemove = $wgRequest->getArray( 'removeTemplates' );
 600+ if ( $templateToRemove ) {
 601+ foreach ( $templateToRemove as $template ) {
 602+ $this->removeTemplateFor( $notice, $template );
 603+ }
590604 }
591 - }
 605+
 606+ // Handle weight changes
 607+ $updatedWeights = $wgRequest->getArray( 'weight' );
 608+ if ( $updatedWeights ) {
 609+ foreach ( $updatedWeights as $templateId => $weight ) {
 610+ $this->updateWeight( $notice, $templateId, $weight );
 611+ }
 612+ }
592613
593 - // Handle removing of banners from the campaign
594 - $templateToRemove = $wgRequest->getArray( 'removeTemplates' );
595 - if ( $templateToRemove ) {
596 - foreach ( $templateToRemove as $template ) {
597 - $this->removeTemplateFor( $notice, $template );
 614+ // Handle new project name
 615+ $projectName = $wgRequest->getVal( 'project_name' );
 616+ if ( $projectName ) {
 617+ $this->updateProjectName ( $notice, $projectName );
598618 }
599 - }
600 -
601 - // Handle weight changes
602 - $updatedWeights = $wgRequest->getArray( 'weight' );
603 - if ( $updatedWeights ) {
604 - foreach ( $updatedWeights as $templateId => $weight ) {
605 - $this->updateWeight( $notice, $templateId, $weight );
 619+
 620+ // Handle new project languages
 621+ $projectLangs = $wgRequest->getArray( 'project_languages' );
 622+ if ( $projectLangs ) {
 623+ $this->updateProjectLanguages( $notice, $projectLangs );
606624 }
 625+
 626+ } else {
 627+ $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );
607628 }
608 -
609 - // Handle new project name
610 - $projectName = $wgRequest->getVal( 'project_name' );
611 - if ( $projectName ) {
612 - $this->updateProjectName ( $notice, $projectName );
613 - }
614 -
615 - // Handle new project languages
616 - $projectLangs = $wgRequest->getArray( 'project_languages' );
617 - if ( $projectLangs ) {
618 - $this->updateProjectLanguages( $notice, $projectLangs );
619 - }
620 -
621 - $wgOut->redirect( $this->getTitle()->getLocalUrl( "method=listNoticeDetail&notice=$notice" ) );
622 - return;
623629
624 - } else {
625 - $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'sessionfailure' );
626630 }
627 -
628 - }
629631
630 - $noticeId = $this->getNoticeId( $notice );
631 -
632 - // Make sure notice exists
633 - if ( !$noticeId ) {
634 - $wgOut->wrapWikiMsg( "<div class='cn-error'>\n$1\n</div>", 'centralnotice-notice-doesnt-exist' );
635 - } else {
636632 $htmlOut = '';
637633
638634 // Begin Campaign detail fieldset

Follow-up revisions

RevisionCommit summaryAuthorDate
r71996redoing how errors are handled for Post/Redirect/Get (fix for r71832), removi...kaldari00:27, 31 August 2010

Comments

#Comment by Kaldari (talk | contribs)   01:31, 28 August 2010 
-   $wgOut->redirect( $this->getTitle()->getLocalUrl( "method=listNoticeDetail&notice=$notice" ) );

I also removed the unneccessary reloading of the page after the form submission is handled. As far as I can tell, everything potentially affected by the routines above is output afterward, so I don't see any reason to reload.

#Comment by Catrope (talk | contribs)   22:39, 30 August 2010

This is actually useful behavior, see wikipedia:Post/Redirect/Get

Status & tagging log