diff --git a/includes/Hooks.php b/includes/Hooks.php index 785a9719b..900e073ea 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -596,7 +596,7 @@ class Hooks implements // test for them reaching a congratulatory threshold $thresholds = [ 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000 ]; if ( $userIdentity->isRegistered() ) { - $thresholdCount = $this->getEditCount( $userIdentity ); + $thresholdCount = $this->getPredictedEditCount( $userIdentity ); if ( in_array( $thresholdCount, $thresholds ) ) { DeferredUpdates::addCallableUpdate( static function () use ( $revisionRecord, $userIdentity, $title, $thresholdCount @@ -664,17 +664,16 @@ class Hooks implements } /** + * Get the predicted edit count after a page save hook + * * @param UserIdentity $user * @return int */ - private function getEditCount( UserIdentity $user ) { + private function getPredictedEditCount( UserIdentity $user ) { $editCount = $this->userEditTracker->getUserEditCount( $user ) ?: 0; - // When this code runs from a maintenance script or unit tests - // the deferred update incrementing edit count runs right away - // so the edit count is right. Otherwise it lags by one. - if ( wfIsCLI() ) { - return $editCount; - } + // When this code runs, the deferred update that increments the edit count + // will still be pending. + return $editCount + 1; } diff --git a/tests/phpunit/ThankYouEditTest.php b/tests/phpunit/ThankYouEditTest.php index 027f709e1..079cdeddc 100644 --- a/tests/phpunit/ThankYouEditTest.php +++ b/tests/phpunit/ThankYouEditTest.php @@ -5,6 +5,7 @@ namespace MediaWiki\Extension\Notifications\Test; use MediaWiki\Deferred\DeferredUpdates; use MediaWiki\Extension\Notifications\DbFactory; use MediaWiki\Extension\Notifications\Mapper\NotificationMapper; +use MediaWiki\Extension\Notifications\Model\Notification; use MediaWiki\Title\Title; use MediaWikiIntegrationTestCase; use Wikimedia\Rdbms\Platform\ISQLPlatform; @@ -29,17 +30,43 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase { ->execute(); } + public function provideFirstEditRequestModes() { + return [ + [ 'web' ], + [ 'cli' ] + ]; + } + /** * @covers \MediaWiki\Extension\Notifications\Hooks::onPageSaveComplete + * @dataProvider provideFirstEditRequestModes + * @param string $mode */ - public function testFirstEdit() { + public function testFirstEdit( $mode ) { + // TODO: re-renable once I50aa9fe9387c9b7b7ff97dfd39a2830bce647db8 is merged. + // That is, after, endAtomic() in PageUpdater::doCreate() is tweaked + if ( $mode === 'cli' ) { + $this->markTestSkipped(); + } + // setup $this->deleteEchoData(); $user = $this->getMutableTestUser()->getUser(); $title = Title::newFromText( 'Help:MWEchoThankYouEditTest_testFirstEdit' ); // action + $db = $this->getDb(); + // Web requests wrap the edit in a broader transaction via DBO_TRX and commit it in + // MediaWikiEntryPoint::commitMainTransaction(). We can largely simulate that by just + // using atomic sections. + $useAtomicSection = ( $mode === 'web' ); + if ( $useAtomicSection ) { + $db->startAtomic( __METHOD__ ); + } $this->editPage( $title, 'this is my first edit', '', NS_MAIN, $user ); + if ( $useAtomicSection ) { + $db->endAtomic( __METHOD__ ); + } DeferredUpdates::tryOpportunisticExecute(); // assertions @@ -52,10 +79,24 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase { $this->assertSame( 1, $notification->getEvent()->getExtraParam( 'editCount', 'not found' ) ); } + public function provideTenthEditRequestModes() { + return [ + [ 'web' ], + [ 'cli' ] + ]; + } + /** * @covers \MediaWiki\Extension\Notifications\Hooks::onPageSaveComplete + * @dataProvider provideTenthEditRequestModes + * @param string $mode */ - public function testTenthEdit() { + public function testTenthEdit( $mode ) { + // TODO: re-renable once endAtomic() in PageUpdater::doCreate() is tweaked + if ( $mode === 'cli' ) { + $this->markTestSkipped(); + } + // setup $this->deleteEchoData(); $user = $this->getMutableTestUser()->getUser(); @@ -66,10 +107,21 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase { // we could fast-forward the edit-count to speed things up // but this is the only way to make sure duplicate notifications // are not generated + $db = $this->getDb(); + // Web requests wrap the edit in a broader transaction via DBO_TRX and commit it in + // MediaWikiEntryPoint::commitMainTransaction(). We can largely simulate that by just + // using atomic sections. + $useAtomicSection = ( $mode === 'web' ); for ( $i = 0; $i < 12; $i++ ) { + if ( $useAtomicSection ) { + $db->startAtomic( __METHOD__ ); + } $this->editPage( $page, "this is edit #$i", '', NS_MAIN, $user ); + if ( $useAtomicSection ) { + $db->endAtomic( __METHOD__ ); + } + DeferredUpdates::tryOpportunisticExecute(); } - DeferredUpdates::tryOpportunisticExecute(); $user->clearInstanceCache(); // assertions