jenkins-bot has submitted this change and it was merged.
Change subject: FlowUpdateWorkflowPageId.php: Check for fatal Status objs
returned by OccupationController
......................................................................
FlowUpdateWorkflowPageId.php: Check for fatal Status objs returned by
OccupationController
Right now, we'll only report something as failed when an exception
occurred (and won't even show the reason). It's also very possible
for the page creation to fail with a Status::newFatal, which was
left unchecked (in which case it would then later in the code fail
to associate the workflow with an id and still show up as failed,
but we'd have no more way of knowing why...)
Now I'll treat "failed to create page" differently from "failed to
associate workflow with page": the first is a warning, the second
is a failure. Both of them will be reported (and when the first one
happens, it's very likely that the second will also happen) and
we'll have much more detailed info when something goes wrong.
Bug: T122329
Change-Id: If6dbf62ace71fffa79e8ae99031c18647350efa4
---
M maintenance/FlowUpdateWorkflowPageId.php
1 file changed, 44 insertions(+), 24 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/maintenance/FlowUpdateWorkflowPageId.php
b/maintenance/FlowUpdateWorkflowPageId.php
index c67debc..aecff80 100644
--- a/maintenance/FlowUpdateWorkflowPageId.php
+++ b/maintenance/FlowUpdateWorkflowPageId.php
@@ -2,6 +2,7 @@
use Flow\Container;
use Flow\Model\UUID;
+use Flow\Model\Workflow;
use Flow\OccupationController;
$IP = getenv( 'MW_INSTALL_PATH' );
@@ -70,7 +71,8 @@
*/
protected $lang;
protected $fixedCount = 0;
- protected $failed = array();
+ protected $failures = array();
+ protected $warnings = array();
/**
* @param Language|StubUserLang $lang
@@ -93,26 +95,14 @@
// at some point, we failed to create page entries for new
workflows: only
// create that page if the workflow was stored with a 0 page id
(otherwise,
// we could mistake the $title for a deleted page)
- if ( $row->workflow_page_id === 0 && $title->getArticleID() ===
0 ) {
- // build workflow object (yes, loading them piecemeal
is suboptimal, but
- // this is just a one-time script; considering the
alternative is
- // creating a derivative BatchRowIterator that returns
workflows,
- // it doesn't really matter)
- $storage = Container::get( 'storage' );
- $workflow = $storage->get( 'Workflow', UUID::create(
$row->workflow_id ) );
-
- try {
- /** @var OccupationController
$occupationController */
- $occupationController = Container::get(
'occupation_controller' );
- $occupationController->allowCreation( $title,
$occupationController->getTalkpageManager() );
- $occupationController->ensureFlowRevision( new
Article( $title ), $workflow );
-
- // force article id to be refetched from db
- $title->getArticleID( Title::GAID_FOR_UPDATE );
- } catch ( \Exception $e ) {
- // catch all exception to keep going with the
rest we want to
- // iterate over, we'll report on the failed
entries at the end
- $this->failed[] = $row;
+ if ( (int) $row->workflow_page_id === 0 &&
$title->getArticleID() === 0 ) {
+ $workflow = Workflow::fromStorageRow( (array) $row );
+ $status = $this->createPage( $title, $workflow );
+ if ( !$status->isGood() ) {
+ // just warn when we failed to create the page,
but keep this code
+ // going and see if we manage to associate the
workflow anyways
+ // (or if that fails, we'll also get an error
there)
+ $this->warnings[] =
$status->getMessage()->text();
}
}
@@ -124,15 +114,45 @@
'workflow_page_id' => $title->getArticleID(),
);
} elseif ( !$row->workflow_page_id ) {
- // No id exists for this workflow?
- $this->failed[] = $row;
+ // No id exists for this workflow? (reason should
likely show up in $this->warnings)
+ $this->failures[] = $row;
}
return array();
}
+ /**
+ * @param Title $title
+ * @param Workflow $workflow
+ * @return Status
+ */
+ protected function createPage( Title $title, $workflow ) {
+ /** @var OccupationController $occupationController */
+ $occupationController = Container::get( 'occupation_controller'
);
+
+ try {
+ $status = $occupationController->allowCreation( $title,
$occupationController->getTalkpageManager() );
+ $status2 = $occupationController->ensureFlowRevision(
new Article( $title ), $workflow );
+
+ $status->merge( $status2 );
+ } catch ( \Exception $e ) {
+ // "convert" exception into Status
+ $message = new RawMessage( $e->getMessage() );
+ $status = Status::newFatal( $message );
+ }
+
+ if ( $status->isGood() ) {
+ // force article id to be refetched from db
+ $title->getArticleID( Title::GAID_FOR_UPDATE );
+ }
+
+ return $status;
+ }
+
public function report() {
- return "Updated {$this->fixedCount} workflows\nFailed: " .
count( $this->failed ) . "\n\n" . print_r( $this->failed, true );
+ return "Updated {$this->fixedCount} workflows\n\n" .
+ "Warnings: " . count( $this->warnings ) . "\n" .
print_r( $this->warnings, true ) . "\n\n" .
+ "Failed: " . count( $this->failures ) . "\n" . print_r(
$this->failures, true );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/265707
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If6dbf62ace71fffa79e8ae99031c18647350efa4
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
