diff --git a/RELEASE-NOTES-1.38 b/RELEASE-NOTES-1.38 index 539a80e1fb5..846b10121eb 100644 --- a/RELEASE-NOTES-1.38 +++ b/RELEASE-NOTES-1.38 @@ -85,6 +85,7 @@ because of Phabricator reports. * TemplateParser used to support disabling the cache with a boolean parameter in its constructor. This was deprecated in 1.35 and has now been removed. * The ArticleUndeleteLogEntry hook, deprecated in 1.37, was removed. +* The BeforeResetNotificationTimestamp hook, deprecated in 1.37, was removed. * The global function mimeTypeMatch() has been removed without a deprecation process. * The signature of PageUpdater::markAsRevert method was changed. It has never diff --git a/autoload.php b/autoload.php index 712907f2fad..37e87ea7d44 100644 --- a/autoload.php +++ b/autoload.php @@ -872,7 +872,6 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Hook\\BeforeParserFetchTemplateAndtitleHook' => __DIR__ . '/includes/parser/Hook/BeforeParserFetchTemplateAndtitleHook.php', 'MediaWiki\\Hook\\BeforeParserFetchTemplateRevisionRecordHook' => __DIR__ . '/includes/parser/Hook/BeforeParserFetchTemplateRevisionRecordHook.php', 'MediaWiki\\Hook\\BeforeParserrenderImageGalleryHook' => __DIR__ . '/includes/parser/Hook/BeforeParserrenderImageGalleryHook.php', - 'MediaWiki\\Hook\\BeforeResetNotificationTimestampHook' => __DIR__ . '/includes/watcheditem/Hook/BeforeResetNotificationTimestampHook.php', 'MediaWiki\\Hook\\BeforeWelcomeCreationHook' => __DIR__ . '/includes/specials/Hook/BeforeWelcomeCreationHook.php', 'MediaWiki\\Hook\\BitmapHandlerCheckImageAreaHook' => __DIR__ . '/includes/media/Hook/BitmapHandlerCheckImageAreaHook.php', 'MediaWiki\\Hook\\BitmapHandlerTransformHook' => __DIR__ . '/includes/media/Hook/BitmapHandlerTransformHook.php', diff --git a/includes/HookContainer/DeprecatedHooks.php b/includes/HookContainer/DeprecatedHooks.php index f8a4bfa0df5..c4aebbec69e 100644 --- a/includes/HookContainer/DeprecatedHooks.php +++ b/includes/HookContainer/DeprecatedHooks.php @@ -38,7 +38,6 @@ class DeprecatedHooks { 'BaseTemplateAfterPortlet' => [ 'deprecatedVersion' => '1.35' ], 'BeforeParserFetchTemplateAndtitle' => [ 'deprecatedVersion' => '1.36' ], 'BeforeParserrenderImageGallery' => [ 'deprecatedVersion' => '1.35' ], - 'BeforeResetNotificationTimestamp' => [ 'deprecatedVersion' => '1.37' ], 'EditPageBeforeEditToolbar' => [ 'deprecatedVersion' => '1.36' ], 'InternalParseBeforeSanitize' => [ 'deprecatedVersion' => '1.35' ], 'LocalFile::getHistory' => [ 'deprecatedVersion' => '1.37' ], diff --git a/includes/HookContainer/HookRunner.php b/includes/HookContainer/HookRunner.php index 4a2d1037d5a..38123b75a5d 100644 --- a/includes/HookContainer/HookRunner.php +++ b/includes/HookContainer/HookRunner.php @@ -115,7 +115,6 @@ class HookRunner implements \MediaWiki\Hook\BeforeParserFetchTemplateAndtitleHook, \MediaWiki\Hook\BeforeParserFetchTemplateRevisionRecordHook, \MediaWiki\Hook\BeforeParserrenderImageGalleryHook, - \MediaWiki\Hook\BeforeResetNotificationTimestampHook, \MediaWiki\Hook\BeforeWelcomeCreationHook, \MediaWiki\Hook\BitmapHandlerCheckImageAreaHook, \MediaWiki\Hook\BitmapHandlerTransformHook, @@ -981,15 +980,6 @@ class HookRunner implements ); } - public function onBeforeResetNotificationTimestamp( &$userObj, &$titleObj, - $force, &$oldid - ) { - return $this->container->run( - 'BeforeResetNotificationTimestamp', - [ &$userObj, &$titleObj, $force, &$oldid ] - ); - } - public function onBeforeRevertedTagUpdate( $wikiPage, $user, $summary, $flags, $revisionRecord, $editResult, &$approved ): void { diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index fdde5840761..c32aaa3df3a 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -1795,9 +1795,7 @@ return [ $services->getReadOnlyMode(), $services->getNamespaceInfo(), $services->getRevisionLookup(), - $services->getHookContainer(), $services->getLinkBatchFactory(), - $services->getUserFactory(), $services->getTitleFactory() ); $store->setStatsdDataFactory( $services->getStatsdDataFactory() ); diff --git a/includes/watcheditem/Hook/BeforeResetNotificationTimestampHook.php b/includes/watcheditem/Hook/BeforeResetNotificationTimestampHook.php deleted file mode 100644 index ce033f4d752..00000000000 --- a/includes/watcheditem/Hook/BeforeResetNotificationTimestampHook.php +++ /dev/null @@ -1,32 +0,0 @@ -assertRequiredOptions( self::CONSTRUCTOR_OPTIONS ); @@ -181,9 +166,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac [ DeferredUpdates::class, 'addCallableUpdate' ]; $this->nsInfo = $nsInfo; $this->revisionLookup = $revisionLookup; - $this->hookRunner = new HookRunner( $hookContainer ); $this->linkBatchFactory = $linkBatchFactory; - $this->userFactory = $userFactory; $this->titleFactory = $titleFactory; $this->latestUpdateCache = new HashBagOStuff( [ 'maxKeys' => 3 ] ); @@ -1451,26 +1434,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return false; } - // Hook expects User and Title, not UserIdentity and LinkTarget|PageIdentity - $userObj = $this->userFactory->newFromUserIdentity( $user ); - if ( $title instanceof LinkTarget ) { - $titleObj = $this->titleFactory->castFromLinkTarget( $title ); - } else { - // instanceof PageIdentity - $titleObj = $this->titleFactory->castFromPageIdentity( $title ); - } - if ( !$this->hookRunner->onBeforeResetNotificationTimestamp( - $userObj, $titleObj, $force, $oldid ) - ) { - return false; - } - if ( !$userObj->equals( $user ) ) { - $user = $userObj; - } - if ( !$titleObj->equals( $title ) ) { - $title = $titleObj; - } - $item = null; if ( $force != 'force' ) { $item = $this->loadWatchedItem( $user, $title ); @@ -1519,6 +1482,17 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac ); // If the page is watched by the user (or may be watched), update the timestamp + // ActivityUpdateJob requires a LinkTarget, and creates a Title object from that, + // to pass to Job, which only needs a PageReference. TODO clean that up, T291531. + // If we already have a LinkTarget, we still convert to a Title object so that + // the Title::newFromLinkTarget() call in ActivityUpdateJob doesn't break things + // in unit tests + if ( $title instanceof LinkTarget ) { + $titleObj = $this->titleFactory->castFromLinkTarget( $title ); + } else { + // instanceof PageIdentity + $titleObj = $this->titleFactory->castFromPageIdentity( $title ); + } $job = new ActivityUpdateJob( $titleObj, [ diff --git a/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php b/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php index 6a3e54b779d..3351284b990 100644 --- a/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php @@ -7,8 +7,6 @@ use MediaWiki\Page\PageIdentityValue; use MediaWiki\Revision\RevisionLookup; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Tests\Unit\DummyServicesTrait; -use MediaWiki\User\UserFactory; -use MediaWiki\User\UserIdentity; use MediaWiki\User\UserIdentityValue; use PHPUnit\Framework\MockObject\MockObject; use Wikimedia\Rdbms\DBConnRef; @@ -149,39 +147,6 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase { ); } - /** - * @param User[] $users - * @return MockObject|UserFactory - */ - private function getUserFactory( array $users = [] ) { - // UserFactory is only needed for newFromUserIdentity. Create a mock User object - // based on the UserIdentity. Used for WatchedItemStore::resetNotificationTimestamp - // which needs full User objects for a hook. Mock users returned have the same - // name and id, and pass User::equals() comparison with the UserIdentity they were - // created from. - $userFactory = $this->createNoOpMock( UserFactory::class, [ 'newFromUserIdentity' ] ); - $userFactory->method( 'newFromUserIdentity' )->willReturnCallback( - function ( $userIdentity ) { - // Like real UserFactory, return $userIdentity if its a User - if ( $userIdentity instanceof User ) { - return $userIdentity; - } - - $user = $this->createMock( User::class ); - $user->method( 'getId' )->willReturn( $userIdentity->getId() ); - $user->method( 'getName' )->willReturn( $userIdentity->getName() ); - $user->method( 'equals' )->willReturnCallback( - static function ( UserIdentity $otherUser ) use ( $userIdentity ) { - // $user's name is the same as $userIdentity's - return $otherUser->getName() === $userIdentity->getName(); - } - ); - return $user; - } - ); - return $userFactory; - } - /** * @param LinkTarget|PageIdentity|null $target * @param Title|null $title @@ -240,7 +205,6 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase { * * readOnlyMode * * nsInfo * * revisionLookup - * * userFactory * * titleFactory * * expiryEnabled * * maxExpiryDuration @@ -271,9 +235,7 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase { $mocks['readOnlyMode'] ?? $this->getDummyReadOnlyMode( false ), $nsInfo, $mocks['revisionLookup'] ?? $this->getMockRevisionLookup(), - $this->createHookContainer(), $this->getMockLinkBatchFactory( $db ), - $this->getUserFactory(), $mocks['titleFactory'] ?? $this->getTitleFactory() ); }