Mattflaschen has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/334744 )
Change subject: SECURITY: Attribute Special:EnableFlow to initiating user ...................................................................... SECURITY: Attribute Special:EnableFlow to initiating user It already does when the page doesn't exist. When it does, it uses the Converter Flow, which didn't fully support this. So refactor the Converter to allow this, and pass in the right user. Bug: T146425 Change-Id: I3636e637fe725f96fd80e57c55f6b0cbcea90744 --- M includes/Import/Converter.php M includes/Import/Importer.php M includes/Import/Wikitext/ConversionStrategy.php M includes/Import/Wikitext/ImportSource.php M includes/Specials/SpecialEnableFlow.php M maintenance/convertLqtPageFromRemoteApiForTesting.php M maintenance/convertNamespaceFromWikitext.php M tests/phpunit/Import/TalkpageImportOperationTest.php M tests/phpunit/Import/Wikitext/ConversionStrategyTest.php M tests/phpunit/Import/Wikitext/ImportSourceTest.php 10 files changed, 70 insertions(+), 19 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow refs/changes/44/334744/1 diff --git a/includes/Import/Converter.php b/includes/Import/Converter.php index 44c8e84..e2d7710 100644 --- a/includes/Import/Converter.php +++ b/includes/Import/Converter.php @@ -65,8 +65,7 @@ * @param DatabaseBase $dbw Master wiki database to read from * @param Importer $importer * @param LoggerInterface $logger - * @param User $user Administrative user for moves and edits related - * to the conversion process. + * @param User $user User for moves and edits related to the conversion process * @param IConversionStrategy $strategy * @throws ImportException When $user does not have an Id */ @@ -185,7 +184,7 @@ } $source = $this->strategy->createImportSource( $archiveTitle ); - if ( $this->importer->import( $source, $title, $this->strategy->getSourceStore() ) ) { + if ( $this->importer->import( $source, $title, $this->user, $this->strategy->getSourceStore() ) ) { $this->createArchiveCleanupRevision( $title, $archiveTitle ); $this->logger->info( "Completed import to $title from $archiveTitle" ); } else { diff --git a/includes/Import/Importer.php b/includes/Import/Importer.php index d93a4d8..24834a7 100644 --- a/includes/Import/Importer.php +++ b/includes/Import/Importer.php @@ -107,11 +107,14 @@ * * @param IImportSource $source * @param Title $targetPage + * @param User User doing the conversion actions (e.g. initial description, + * wikitext archive edit). However, actions will be attributed to the original + * user when possible (e.g. the user who did the original LQT reply) * @param ImportSourceStore $sourceStore * @return bool True When the import completes with no failures */ - public function import( IImportSource $source, Title $targetPage, ImportSourceStore $sourceStore ) { - $operation = new TalkpageImportOperation( $source, $this->occupationController ); + public function import( IImportSource $source, Title $targetPage, User $user, ImportSourceStore $sourceStore ) { + $operation = new TalkpageImportOperation( $source, $user, $this->occupationController ); $pageImportState = new PageImportState( $this->workflowLoaderFactory ->createWorkflowLoader( $targetPage ) @@ -521,14 +524,24 @@ */ protected $importSource; + /** @var User User doing the conversion actions (e.g. initial description, wikitext + * archive edit). However, actions will be attributed to the original user when + * possible (e.g. the user who did the original LQT reply) + */ + protected $user; + /** @var OccupationController */ protected $occupationController; /** * @param IImportSource $source + * @param User $user The import user; this will only be used when there is no + * 'original' user + * @param OccupationController $occupationController */ - public function __construct( IImportSource $source, OccupationController $occupationController ) { + public function __construct( IImportSource $source, User $user, OccupationController $occupationController ) { $this->importSource = $source; + $this->user = $user; $this->occupationController = $occupationController; } @@ -547,7 +560,7 @@ // Explicitly allow creation of board $allowCreationStatus = $this->occupationController->allowCreation( $destinationTitle, - $this->occupationController->getTalkpageManager(), + $this->user, /* $mustNotExist = */ true ); if ( !$allowCreationStatus->isGood() ) { diff --git a/includes/Import/Wikitext/ConversionStrategy.php b/includes/Import/Wikitext/ConversionStrategy.php index 6fd8c30..7037005 100644 --- a/includes/Import/Wikitext/ConversionStrategy.php +++ b/includes/Import/Wikitext/ConversionStrategy.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use StubObject; use Title; +use User; use WikitextContent; /** @@ -55,10 +56,19 @@ */ protected $headerSuffix; + /** @var User User doing the conversion actions (e.g. initial description, wikitext + * archive edit). However, actions will be attributed to the original user when + * possible (e.g. the user who did the original LQT reply) + * + */ + protected $user; + /** * @param Parser|StubObject $parser * @param ImportSourceStore $sourceStore * @param LoggerInterface $logger + * @param User $user User to take conversion actions are (applicable for actions + * where if there is no 'original' user) * @param Title[] $noConvertTemplates List of templates that flag pages that * shouldn't be converted (optional) * @param string $headerSuffix Wikitext to add to the end of the header (optional) @@ -67,12 +77,14 @@ $parser, ImportSourceStore $sourceStore, LoggerInterface $logger, + User $user, array $noConvertTemplates = array(), $headerSuffix = null ) { $this->parser = $parser; $this->sourceStore = $sourceStore; $this->logger = $logger; + $this->user = $user; $this->noConvertTemplates = $noConvertTemplates; $this->headerSuffix = $headerSuffix; @@ -124,7 +136,7 @@ * {@inheritDoc} */ public function createImportSource( Title $title ) { - return new ImportSource( $title, $this->parser, $this->headerSuffix ); + return new ImportSource( $title, $this->parser, $this->user, $this->headerSuffix ); } /** diff --git a/includes/Import/Wikitext/ImportSource.php b/includes/Import/Wikitext/ImportSource.php index 93f0d41..4257cbe 100644 --- a/includes/Import/Wikitext/ImportSource.php +++ b/includes/Import/Wikitext/ImportSource.php @@ -15,6 +15,7 @@ use Revision; use StubObject; use Title; +use User; /** * Imports the header of a wikitext talk page. Does not attempt to @@ -22,19 +23,26 @@ * ConversionStrategy for more details. */ class ImportSource implements IImportSource { + /** @var User User doing the conversion actions (e.g. initial description, wikitext + * archive edit). However, actions will be attributed to the original user if + * applicable. + */ + protected $user; /** * @param Title $title * @param Parser|StubObject $parser * @param string $headerSuffix + * @param User $user User to take actions as * @throws ImportException When $title is an external title */ - public function __construct( Title $title, $parser, $headerSuffix = null ) { + public function __construct( Title $title, $parser, User $user, $headerSuffix = null ) { if ( $title->isExternal() ) { throw new ImportException( "Invalid non-local title: $title" ); } $this->title = $title; $this->parser = $parser; + $this->user = $user; $this->headerSuffix = $headerSuffix; } @@ -80,7 +88,7 @@ array( new ObjectRevision( $content, wfTimestampNow(), - FlowHooks::getOccupationController()->getTalkpageManager()->getName(), + $this->user->getName(), "wikitext-import:header-revision:{$revision->getId()}" ) ), "wikitext-import:header:{$this->title->getPrefixedText()}" @@ -115,4 +123,3 @@ return new ArrayIterator( array() ); } } - diff --git a/includes/Specials/SpecialEnableFlow.php b/includes/Specials/SpecialEnableFlow.php index 364d907..f495aac 100644 --- a/includes/Specials/SpecialEnableFlow.php +++ b/includes/Specials/SpecialEnableFlow.php @@ -91,11 +91,12 @@ wfGetDB( DB_MASTER ), Container::get( 'importer' ), $logger, - $this->occupationController->getTalkpageManager(), + $this->getUser(), new EnableFlowWikitextConversionStrategy( Container::get( 'parser' ), new NullImportSourceStore(), $logger, + $this->getUser(), array(), $data['header'] ) diff --git a/maintenance/convertLqtPageFromRemoteApiForTesting.php b/maintenance/convertLqtPageFromRemoteApiForTesting.php index 25bdee9..5d482a8 100644 --- a/maintenance/convertLqtPageFromRemoteApiForTesting.php +++ b/maintenance/convertLqtPageFromRemoteApiForTesting.php @@ -79,7 +79,7 @@ $logger->info( "Starting LQT conversion of page $srcPageName" ); - $importer->import( $source, $dstTitle, $sourceStore ); + $importer->import( $source, $dstTitle, $talkPageManagerUser, $sourceStore ); $logger->info( "Finished LQT conversion of page $srcPageName" ); } diff --git a/maintenance/convertNamespaceFromWikitext.php b/maintenance/convertNamespaceFromWikitext.php index a8d6d72..af54d69 100644 --- a/maintenance/convertNamespaceFromWikitext.php +++ b/maintenance/convertNamespaceFromWikitext.php @@ -69,15 +69,18 @@ $logger = new MaintenanceDebugLogger( $this ); $dbw = wfGetDB( DB_MASTER ); + $talkpageManager = FlowHooks::getOccupationController()->getTalkpageManager(); $converter = new \Flow\Import\Converter( $dbw, Flow\Container::get( 'importer' ), $logger, - FlowHooks::getOccupationController()->getTalkpageManager(), + $talkpageManager, + new Flow\Import\Wikitext\ConversionStrategy( $wgParser, new Flow\Import\NullImportSourceStore(), $logger, + $talkpageManager, $noConvertTemplates, $this->getOption( 'archive-pattern', null ) ) diff --git a/tests/phpunit/Import/TalkpageImportOperationTest.php b/tests/phpunit/Import/TalkpageImportOperationTest.php index c56e4f1..a4a151c 100644 --- a/tests/phpunit/Import/TalkpageImportOperationTest.php +++ b/tests/phpunit/Import/TalkpageImportOperationTest.php @@ -113,7 +113,13 @@ ) ); - $op = new TalkpageImportOperation( $source, Container::get( 'occupation_controller' ) ); + $occupationController = Container::get( 'occupation_controller' ); + $op = new TalkpageImportOperation( + $source, + $occupationController->getTalkpageManager(), + $occupationController + ); + $store = new NullImportSourceStore; $op->import( new PageImportState( $workflow, diff --git a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php index 293bc7f..fa5985e 100644 --- a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php +++ b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php @@ -187,7 +187,8 @@ return new ConversionStrategy( $parser ?: $wgParser, $sourceStore ?: new NullImportSourceStore, - Container::get( 'default_logger' ) + Container::get( 'default_logger' ), + Container::get( 'occupation_controller' )->getTalkpageManager() ); } } diff --git a/tests/phpunit/Import/Wikitext/ImportSourceTest.php b/tests/phpunit/Import/Wikitext/ImportSourceTest.php index 8c1db61..6c215ee 100644 --- a/tests/phpunit/Import/Wikitext/ImportSourceTest.php +++ b/tests/phpunit/Import/Wikitext/ImportSourceTest.php @@ -4,6 +4,7 @@ use DateTime; use DateTimeZone; +use Flow\Container; use Flow\Exception\WikitextException; use Flow\Import\Wikitext\ImportSource; use Flow\Parsoid\Utils; @@ -34,7 +35,9 @@ /** * @dataProvider getHeaderProvider */ - public function testGetHeader( $content, $expect ) { + public function testGetHeader( $content, $expectText ) { + $user = Container::get( 'occupation_controller' )->getTalkpageManager(); + // create a page with some content $status = WikiPage::factory( Title::newMainPage() ) ->doEditContent( @@ -45,7 +48,12 @@ $this->fail( $status->getMessage()->plain() ); } - $source = new ImportSource( Title::newMainPage(), new Parser ); + $source = new ImportSource( + Title::newMainPage(), + new Parser, + $user + ); + $header = $source->getHeader(); $this->assertNotNull( $header ); $this->assertGreaterThan( 1, strlen( $header->getObjectKey() ) ); @@ -55,7 +63,8 @@ $revision = reset( $revisions ); $this->assertInstanceOf( 'Flow\Import\IObjectRevision', $revision ); - $this->assertEquals( $expect, $revision->getText() ); + $this->assertEquals( $expectText, $revision->getText() ); + $this->assertEquals( $user->getName(), $revision->getAuthor() ); } public function getHeaderProvider() { -- To view, visit https://gerrit.wikimedia.org/r/334744 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3636e637fe725f96fd80e57c55f6b0cbcea90744 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: REL1_26 Gerrit-Owner: Mattflaschen <mflasc...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits