From 6b0c6609d1bae67b2b5c0a53b93888c8159e1d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Thu, 17 Oct 2024 18:43:12 +0200 Subject: [PATCH] Migrate 'ArticleDelete' hook to 'PageDelete' to fix error display Bug: T377384 Change-Id: Ia8df47ffc9a77dabd00c97731b2e335901897bf0 --- extension.json | 6 ++-- .../Hooks/Handlers/FilteredActionsHandler.php | 31 ++++++++++++++----- tests/phpunit/AbuseFilterConsequencesTest.php | 4 ++- .../ActionVariablesIntegrationTest.php | 4 ++- .../FilteredActionsHandlerTest.php | 6 +++- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/extension.json b/extension.json index af854109..cb5ae8fe 100644 --- a/extension.json +++ b/extension.json @@ -410,7 +410,9 @@ "AbuseFilterVariableGeneratorFactory", "AbuseFilterEditRevUpdater", "AbuseFilterBlockedDomainFilter", - "PermissionManager" + "PermissionManager", + "TitleFactory", + "UserFactory" ] }, "CheckUser": { @@ -434,7 +436,7 @@ "EditFilterMergedContent": [ "FilteredActions", "ConfirmEdit" ], "GetAutoPromoteGroups": "AutoPromoteGroups", "TitleMove": "FilteredActions", - "ArticleDelete": "FilteredActions", + "PageDelete": "FilteredActions", "RecentChange_save": "RecentChangeSave", "ListDefinedTags": "ChangeTags", "ChangeTagsListActive": "ChangeTags", diff --git a/includes/Hooks/Handlers/FilteredActionsHandler.php b/includes/Hooks/Handlers/FilteredActionsHandler.php index 59292e08..325e1b78 100644 --- a/includes/Hooks/Handlers/FilteredActionsHandler.php +++ b/includes/Hooks/Handlers/FilteredActionsHandler.php @@ -15,16 +15,20 @@ use MediaWiki\Hook\UploadStashFileHook; use MediaWiki\Hook\UploadVerifyUploadHook; use MediaWiki\Logger\LoggerFactory; -use MediaWiki\Page\Hook\ArticleDeleteHook; +use MediaWiki\Page\Hook\PageDeleteHook; +use MediaWiki\Page\ProperPageIdentity; +use MediaWiki\Permissions\Authority; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Revision\SlotRecord; use MediaWiki\Status\Status; use MediaWiki\Storage\Hook\ParserOutputStashForEditHook; use MediaWiki\Title\Title; +use MediaWiki\Title\TitleFactory; use MediaWiki\User\User; +use MediaWiki\User\UserFactory; +use StatusValue; use UploadBase; use Wikimedia\Stats\IBufferingStatsdDataFactory; -use WikiPage; /** * Handler for actions that can be filtered @@ -32,7 +36,7 @@ class FilteredActionsHandler implements EditFilterMergedContentHook, TitleMoveHook, - ArticleDeleteHook, + PageDeleteHook, UploadVerifyUploadHook, UploadStashFileHook, ParserOutputStashForEditHook @@ -47,6 +51,8 @@ class FilteredActionsHandler implements private $editRevUpdater; private PermissionManager $permissionManager; private BlockedDomainFilter $blockedDomainFilter; + private TitleFactory $titleFactory; + private UserFactory $userFactory; /** * @param IBufferingStatsdDataFactory $statsDataFactory @@ -55,6 +61,8 @@ class FilteredActionsHandler implements * @param EditRevUpdater $editRevUpdater * @param BlockedDomainFilter $blockedDomainFilter * @param PermissionManager $permissionManager + * @param TitleFactory $titleFactory + * @param UserFactory $userFactory */ public function __construct( IBufferingStatsdDataFactory $statsDataFactory, @@ -62,7 +70,9 @@ public function __construct( VariableGeneratorFactory $variableGeneratorFactory, EditRevUpdater $editRevUpdater, BlockedDomainFilter $blockedDomainFilter, - PermissionManager $permissionManager + PermissionManager $permissionManager, + TitleFactory $titleFactory, + UserFactory $userFactory ) { $this->statsDataFactory = $statsDataFactory; $this->filterRunnerFactory = $filterRunnerFactory; @@ -70,6 +80,8 @@ public function __construct( $this->editRevUpdater = $editRevUpdater; $this->blockedDomainFilter = $blockedDomainFilter; $this->permissionManager = $permissionManager; + $this->titleFactory = $titleFactory; + $this->userFactory = $userFactory; } /** @@ -205,18 +217,21 @@ public function onTitleMove( Title $old, Title $nt, User $user, $reason, Status /** * @inheritDoc */ - public function onArticleDelete( WikiPage $wikiPage, User $user, &$reason, &$error, Status &$status, $suppress ) { + public function onPageDelete( + ProperPageIdentity $page, Authority $deleter, string $reason, StatusValue $status, bool $suppress + ) { if ( $suppress ) { // Don't filter suppressions, T71617 return true; } - $builder = $this->variableGeneratorFactory->newRunGenerator( $user, $wikiPage->getTitle() ); + $user = $this->userFactory->newFromAuthority( $deleter ); + $title = $this->titleFactory->newFromPageIdentity( $page ); + $builder = $this->variableGeneratorFactory->newRunGenerator( $user, $title ); $vars = $builder->getDeleteVars( $reason ); - $runner = $this->filterRunnerFactory->newRunner( $user, $wikiPage->getTitle(), $vars, 'default' ); + $runner = $this->filterRunnerFactory->newRunner( $user, $title, $vars, 'default' ); $filterResult = $runner->run(); $status->merge( $filterResult ); - $error = $filterResult->isOK() ? '' : $filterResult->getHTML(); return $filterResult->isOK(); } diff --git a/tests/phpunit/AbuseFilterConsequencesTest.php b/tests/phpunit/AbuseFilterConsequencesTest.php index 596948d9..3662bbe3 100644 --- a/tests/phpunit/AbuseFilterConsequencesTest.php +++ b/tests/phpunit/AbuseFilterConsequencesTest.php @@ -479,7 +479,9 @@ private function doEdit( Title $title, $oldText, $newText, $summary, $fromStash AbuseFilterServices::getVariableGeneratorFactory(), AbuseFilterServices::getEditRevUpdater(), AbuseFilterServices::getBlockedDomainFilter(), - $services->getPermissionManager() + $services->getPermissionManager(), + $services->getTitleFactory(), + $services->getUserFactory() ); $hooksHandler->onEditFilterMergedContent( $context, $content, $status, $summary, $this->user, false ); diff --git a/tests/phpunit/integration/ActionVariablesIntegrationTest.php b/tests/phpunit/integration/ActionVariablesIntegrationTest.php index 4f0750d4..4f1bc567 100644 --- a/tests/phpunit/integration/ActionVariablesIntegrationTest.php +++ b/tests/phpunit/integration/ActionVariablesIntegrationTest.php @@ -320,7 +320,9 @@ public function testEditVariables( AbuseFilterServices::getVariableGeneratorFactory(), AbuseFilterServices::getEditRevUpdater(), AbuseFilterServices::getBlockedDomainFilter(), - MediaWikiServices::getInstance()->getPermissionManager() + MediaWikiServices::getInstance()->getPermissionManager(), + MediaWikiServices::getInstance()->getTitleFactory(), + MediaWikiServices::getInstance()->getUserFactory() ); $this->setTemporaryHook( 'EditFilterMergedContent', diff --git a/tests/phpunit/integration/FilteredActionsHandlerTest.php b/tests/phpunit/integration/FilteredActionsHandlerTest.php index 7664758a..7601eff8 100644 --- a/tests/phpunit/integration/FilteredActionsHandlerTest.php +++ b/tests/phpunit/integration/FilteredActionsHandlerTest.php @@ -19,6 +19,8 @@ use MediaWiki\Permissions\PermissionManager; use MediaWiki\Status\Status; use MediaWiki\Title\Title; +use MediaWiki\Title\TitleFactory; +use MediaWiki\User\UserFactory; use Wikimedia\Stats\NullStatsdDataFactory; /** @@ -120,7 +122,9 @@ private function getFilteredActionsHandler( $urlsAdded ): FilteredActionsHandler $variableGeneratorFactory, $editRevUpdater, $blockedDomainFilter, - $permissionManager + $permissionManager, + $this->createMock( TitleFactory::class ), + $this->createMock( UserFactory::class ) ); } }