Migrate 'ArticleDelete' hook to 'PageDelete' to fix error display

Bug: T377384 Change-Id: Ia8df47ffc9a77dabd00c97731b2e335901897bf0
wikimedia · Oct 21, 2024 · 6b0c660 · 6b0c660
1 parent 8b57be7
commit 6b0c660
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 13 deletions.
diff --git 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
@@ -15,24 +15,28 @@
 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
  */
 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,21 +61,27 @@ class FilteredActionsHandler implements
 	 * @param EditRevUpdater $editRevUpdater
 	 * @param BlockedDomainFilter $blockedDomainFilter
 	 * @param PermissionManager $permissionManager
+	 * @param TitleFactory $titleFactory
+	 * @param UserFactory $userFactory
 	 */
 	public function __construct(
 		IBufferingStatsdDataFactory $statsDataFactory,
 		FilterRunnerFactory $filterRunnerFactory,
 		VariableGeneratorFactory $variableGeneratorFactory,
 		EditRevUpdater $editRevUpdater,
 		BlockedDomainFilter $blockedDomainFilter,
-		PermissionManager $permissionManager
+		PermissionManager $permissionManager,
+		TitleFactory $titleFactory,
+		UserFactory $userFactory
 	) {
 		$this->statsDataFactory = $statsDataFactory;
 		$this->filterRunnerFactory = $filterRunnerFactory;
 		$this->variableGeneratorFactory = $variableGeneratorFactory;
 		$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
@@ -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
@@ -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
@@ -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 )
 		);
 	}
 }