jenkins-bot has submitted this change and it was merged.
Change subject: Hygiene: Split the giant show function in Flow\View
......................................................................
Hygiene: Split the giant show function in Flow\View
Change-Id: Ie8e59ba2eeaeae328744bf907e4bd57523dbbb86
---
M includes/Actions/ViewAction.php
M includes/View.php
M includes/WorkflowLoader.php
M includes/api/ApiFlowBasePost.php
4 files changed, 147 insertions(+), 129 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Actions/ViewAction.php b/includes/Actions/ViewAction.php
index c7130e7..6637d98 100644
--- a/includes/Actions/ViewAction.php
+++ b/includes/Actions/ViewAction.php
@@ -4,9 +4,45 @@
use IContextSource;
use Page;
+use Title;
class ViewAction extends FlowAction {
function __construct( Page $page, IContextSource $context ) {
parent::__construct( $page, $context, 'view' );
}
-}
\ No newline at end of file
+
+ public function showForAction( $action, OutputPage $output = null ) {
+ parent::showForAction( $action, $output );
+
+ $title = $this->context->getTitle();
+ $this->context->getUser()->clearNotification( $title );
+
+ if ( $output === null ) {
+ $output = $this->context->getOutput();
+ }
+ $output->addCategoryLinks( $this->getCategories( $title ) );
+
+ }
+
+ protected function getCategories( Title $title ) {
+ $id = $title->getArticleId();
+ if ( !$id ) {
+ return array();
+ }
+
+ $dbr = wfGetDB( DB_SLAVE );
+ $res = $dbr->select(
+ /* from */ 'categorylinks',
+ /* select */ array( 'cl_to', 'cl_sortkey' ),
+ /* conditions */ array( 'cl_from' => $id ),
+ __METHOD__
+ );
+
+ $categories = array();
+ foreach ( $res as $row ) {
+ $categories[$row->cl_to] = $row->cl_sortkey;
+ }
+
+ return $categories;
+ }
+}
diff --git a/includes/View.php b/includes/View.php
index 1a318ef..7594782 100644
--- a/includes/View.php
+++ b/includes/View.php
@@ -2,15 +2,16 @@
namespace Flow;
+use ContextSource;
use Flow\Block\AbstractBlock;
use Flow\Exception\InvalidActionException;
use Flow\Model\Anchor;
use Flow\Model\UUID;
use Flow\Model\Workflow;
-use ContextSource;
use Html;
use IContextSource;
use Message;
+use OutputPage;
use Title;
use WebRequest;
@@ -39,8 +40,48 @@
public function show( WorkflowLoader $loader, $action ) {
wfProfileIn( __CLASS__ . '-init' );
- $out = $this->getOutput();
- $styles = array(
+ $this->addModules( $this->getOutput() );
+
+ $blocks = $loader->getBlocks();
+ wfProfileOut( __CLASS__ . '-init' );
+
+ $parameters = $this->extractBlockParameters( $action, $blocks );
+ foreach ( $loader->getBlocks() as $block ) {
+ $block->init( $this, $action );
+ }
+
+ if ( $this->getRequest()->wasPosted() ) {
+ $retval = $this->handleSubmit( $loader, $action,
$parameters );
+ // successfull submission
+ if ( $retval === true ) {
+ $this->redirect( $loader->getWorkflow(), 'view'
);
+ return;
+ // only render the returned subset of blocks
+ } elseif ( is_array( $retval ) ) {
+ $blocks = $retval;
+ }
+ }
+
+ $apiResponse = $this->buildApiResponse( $loader, $blocks,
$action, $parameters );
+
+ /**
+ header( 'Content-Type: application/json; content=utf-8' );
+ $data = json_encode( $apiResponse );
+ //return;
+ die( $data );
+ **/
+
+ // Please note that all blocks can set page title, which may
cause them
+ // to override one another's titles
+ foreach ( $blocks as $block ) {
+ $block->setPageTitle( $this->getOutput() );
+ }
+
+ $this->renderApiResponse( $apiResponse );
+ }
+
+ protected function addModules( OutputPage $out ) {
+ $out->addModuleStyles( array(
'mediawiki.ui',
'mediawiki.ui.anchor',
'mediawiki.ui.button',
@@ -54,52 +95,38 @@
'ext.flow.icons.styles',
'ext.flow.board.styles',
'ext.flow.board.topic.styles'
- );
- $out->addModuleStyles( $styles );
+ ) );
$out->addModules( array( 'ext.flow' ) );
// Allow other extensions to add modules
wfRunHooks( 'FlowAddModules', array( $out ) );
+ }
- $workflow = $loader->getWorkflow();
+ protected function handleSubmit( WorkflowLoader $loader, $action, array
$parameters ) {
+ $this->getOutput()->enableClientCache( false );
- $title = $workflow->getArticleTitle();
-
- $request = $this->getRequest();
- $user = $this->getUser();
-
- $blocks = $loader->getBlocks();
- wfProfileOut( __CLASS__ . '-init' );
-
- $parameters = $this->extractBlockParameters( $action, $request,
$blocks );
-
- $wasPosted = $request->wasPosted();
- if ( $wasPosted ) {
- wfProfileIn( __CLASS__ . '-submit' );
- $out->enableClientCache( false );
- $blocksToCommit = $loader->handleSubmit( $this,
$action, $parameters );
- if ( $blocksToCommit ) {
- if ( !$user->matchEditToken( $request->getVal(
'wpEditToken' ) ) ) {
- // only render the failed blocks
- $blocks = $blocksToCommit;
- foreach ( $blocks as $block ) {
- $block->addError( 'edit-token',
$this->msg( 'sessionfailure' ) );
- }
- } else {
- $loader->commit( $workflow,
$blocksToCommit );
- $this->redirect( $workflow, 'view' );
- wfProfileOut( __CLASS__ . '-submit' );
- return;
- }
- }
- wfProfileOut( __CLASS__ . '-submit' );
- } else {
- foreach ( $blocks as $block ) {
- $block->init( $this, $action );
- }
+ $blocksToCommit = $loader->handleSubmit( $this, $action,
$parameters );
+ if ( !$blocksToCommit ) {
+ return false;
}
- wfProfileIn( __CLASS__ . '-serialize' );
+ if ( !$this->getUser()->matchEditToken(
$this->getRequest()->getVal( 'wpEditToken' ) ) ) {
+ // this uses the above $blocksToCommit reference to
only render the failed blocks
+ foreach ( $blocksToCommit as $block ) {
+ $block->addError( 'edit-token', $this->msg(
'sessionfailure' ) );
+ }
+ return $blocksToCommit;
+ }
+
+ $loader->commit( $blocksToCommit );
+ return true;
+ }
+
+ protected function buildApiResponse( WorkflowLoader $loader, array
$blocks, $action, array $parameters ) {
+ $workflow = $loader->getWorkflow();
+ $title = $workflow->getArticleTitle();
+ $user = $this->getUser();
+
// @todo This and API should use same code
$apiResponse = array(
'title' => $title->getPrefixedText(),
@@ -118,6 +145,7 @@
);
$editToken = $user->getEditToken();
+ $wasPosted = $this->getRequest()->wasPosted();
foreach ( $blocks as $block ) {
if ( $wasPosted ? $block->canSubmit( $action ) :
$block->canRender( $action ) ) {
$apiResponse['blocks'][] = $block->renderApi(
$parameters[$block->getName()] )
@@ -127,12 +155,6 @@
'editToken' => $editToken,
);
}
- }
-
- // Please note that all blocks can set page title, which may
cause them
- // to override one another's titles
- foreach ( $blocks as $block ) {
- $block->setPageTitle( $this->getOutput() );
}
if ( count( $apiResponse['blocks'] ) === 0 ) {
@@ -156,65 +178,47 @@
$value = $value->getAlphadecimal();
}
} );
- wfProfileOut( __CLASS__ . '-serialize' );
- if ( $action === 'view' ) {
- // Update newtalk and watchlist notification status on
view action of any workflow
- // since the normal page view that resets notification
status is not accessiable
- // anymore due to Flow occupation
- $user->clearNotification( $title );
+ return $apiResponse;
+ }
- // attach categories that would be shown on a normal
page
- $out->addCategoryLinks( $this->getCategories( $title )
);
- }
-
- /**
- header( 'Content-Type: application/json; content=utf-8' );
- $data = json_encode( $apiResponse );
- //return;
- die( $data );
- **/
-
- // Render with lightncandy. The exact template to render
- // will likely need to vary, but not yet.
- wfProfileIn( __CLASS__ . '-render' );
-
+ protected function renderApiResponse( array $apiResponse ) {
// Render the flow-component wrapper
- if ( !empty( $apiResponse['blocks'] ) ) {
- $renderedBlocks = array();
-
- foreach ( $apiResponse['blocks'] as $block ) {
- // @todo find a better way to do this;
potentially make all blocks their own components
- switch ( $block['type'] ) {
- case 'board-history':
- $flowComponent = 'boardHistory';
- break;
- default:
- $flowComponent = 'board';
- }
-
- // Don't re-render a block type twice in one
page
- if ( isset( $renderedBlocks[$flowComponent] ) )
{
- continue;
- }
- $renderedBlocks[$flowComponent] = true;
-
- // Get the block loop template
- $template = $this->lightncandy->getTemplate(
'flow_block_loop' );
- // Output the component, with the rendered
blocks inside it
- $out->addHTML( Html::rawElement(
- 'div',
- array(
- 'class' =>
'flow-component',
- 'data-flow-component' =>
$flowComponent,
- 'data-flow-id' =>
$apiResponse['workflow'],
- ),
- $template( $apiResponse )
- ) );
- }
+ if ( empty( $apiResponse['blocks'] ) ) {
+ return array();
}
- wfProfileOut( __CLASS__ . '-render' );
+ $out = $this->getOutput();
+ $renderedBlocks = array();
+ foreach ( $apiResponse['blocks'] as $block ) {
+ // @todo find a better way to do this; potentially make
all blocks their own components
+ switch ( $block['type'] ) {
+ case 'board-history':
+ $flowComponent = 'boardHistory';
+ break;
+ default:
+ $flowComponent = 'board';
+ }
+
+ // Don't re-render a block type twice in one page
+ if ( isset( $renderedBlocks[$flowComponent] ) ) {
+ continue;
+ }
+ $renderedBlocks[$flowComponent] = true;
+
+ // Get the block loop template
+ $template = $this->lightncandy->getTemplate(
'flow_block_loop' );
+ // Output the component, with the rendered blocks
inside it
+ $out->addHTML( Html::rawElement(
+ 'div',
+ array(
+ 'class' =>
'flow-component',
+ 'data-flow-component' => $flowComponent,
+ 'data-flow-id' =>
$apiResponse['workflow'],
+ ),
+ $template( $apiResponse )
+ ) );
+ }
}
protected function redirect( Workflow $workflow ) {
@@ -233,7 +237,8 @@
* @param AbstractBlock[] $blocks
* @return array
*/
- public function extractBlockParameters( $action, WebRequest $request,
array $blocks ) {
+ public function extractBlockParameters( $action, array $blocks ) {
+ $request = $this->getRequest();
$result = array();
// BC for old parameters enclosed in square brackets
foreach ( $blocks as $block ) {
@@ -264,27 +269,5 @@
}
return $result;
- }
-
- protected function getCategories( Title $title ) {
- $id = $title->getArticleId();
- if ( !$id ) {
- return array();
- }
-
- $dbr = wfGetDB( DB_SLAVE );
- $res = $dbr->select(
- /* from */ 'categorylinks',
- /* select */ array( 'cl_to', 'cl_sortkey' ),
- /* conditions */ array( 'cl_from' => $id ),
- __METHOD__
- );
-
- $categories = array();
- foreach ( $res as $row ) {
- $categories[$row->cl_to] = $row->cl_sortkey;
- }
-
- return $categories;
}
}
diff --git a/includes/WorkflowLoader.php b/includes/WorkflowLoader.php
index 0746d3f..9952cf4 100644
--- a/includes/WorkflowLoader.php
+++ b/includes/WorkflowLoader.php
@@ -53,7 +53,6 @@
/**
* @param IContextSource $context
- * @param AbstractBlock[] $blocks
* @param string $action
* @param array $parameters
* @return Block[]
@@ -63,7 +62,7 @@
->handleSubmit( $this->workflow, $context,
$this->blocks, $action, $parameters );
}
- public function commit( Workflow $workflow, array $blocks ) {
- return $this->submissionHandler->commit( $workflow, $blocks );
+ public function commit( array $blocks ) {
+ return $this->submissionHandler->commit( $this->workflow,
$blocks );
}
}
diff --git a/includes/api/ApiFlowBasePost.php b/includes/api/ApiFlowBasePost.php
index 813443a..cfd7248 100644
--- a/includes/api/ApiFlowBasePost.php
+++ b/includes/api/ApiFlowBasePost.php
@@ -34,7 +34,7 @@
);
}
- $commitMetadata = $loader->commit( $workflow, $blocksToCommit );
+ $commitMetadata = $loader->commit( $blocksToCommit );
$savedBlocks = array();
$result->setIndexedTagName( $savedBlocks, 'block' );
--
To view, visit https://gerrit.wikimedia.org/r/194146
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie8e59ba2eeaeae328744bf907e4bd57523dbbb86
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: SG <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits