diff --git a/Hooks.php b/Hooks.php index 9d94c5f36..552f80a51 100644 --- a/Hooks.php +++ b/Hooks.php @@ -503,28 +503,27 @@ class EchoHooks { // test for them reaching a congratulatory threshold $thresholds = [ 1, 10, 100, 1000, 10000, 100000, 1000000 ]; if ( $user->isLoggedIn() && $status->value['revision'] ) { - // This edit hasn't been added to the edit count yet - $thresholdCount = $user->getEditCount() + 1; + $thresholdCount = $user->getEditCount(); if ( in_array( $thresholdCount, $thresholds ) ) { - $id = $user->getId(); - DeferredUpdates::addCallableUpdate( function () use ( $id, $title, $thresholdCount ) { - // Fresh User object - $user = User::newFromId( $id ); - $userEditCount = $user->getEditCount(); - if ( (int)$userEditCount !== (int)$thresholdCount ) { - // Race condition with multiple simultaneous requests, skip - LoggerFactory::getInstance( 'Echo' )->debug( - 'thank-you-edit race condition detected: {user} (id: {id}) should ' . - 'have had {expectedCount} edits but has {actualCount}', - array( - 'user' => $user->getName(), - 'id' => $user->getId(), - 'expectedCount' => $thresholdCount, - 'actualCount' => $userEditCount, - ) - ); - return; + DeferredUpdates::addCallableUpdate( function () use ( $user, $title, $thresholdCount ) { + + $notificationMapper = new EchoNotificationMapper(); + $notifications = $notificationMapper->fetchByUser( $user, 10, null, array( 'thank-you-edit' ) ); + /** @var EchoNotification $notification */ + foreach ( $notifications as $notification ) { + if ( $notification->getEvent()->getExtraParam( 'editCount' ) === $thresholdCount ) { + LoggerFactory::getInstance( 'Echo' )->debug( + '{user} (id: {id}) has already been thanked for their {count} edit', + array( + 'user' => $user->getName(), + 'id' => $user->getId(), + 'count' => $thresholdCount, + ) + ); + return; + } } + LoggerFactory::getInstance( 'Echo' )->debug( 'Thanking {user} (id: {id}) for their {count} edit', array( diff --git a/autoload.php b/autoload.php index 152db8edb..589275885 100644 --- a/autoload.php +++ b/autoload.php @@ -105,6 +105,7 @@ $wgAutoloadClasses += [ 'MWEchoEventLogging' => __DIR__ . '/includes/EventLogging.php', 'MWEchoNotifUser' => __DIR__ . '/includes/NotifUser.php', 'MWEchoNotifUserTest' => __DIR__ . '/tests/phpunit/NotifUserTest.php', + 'MWEchoThankYouEditTest' => __DIR__ . '/tests/phpunit/ThankYouEditTest.php', 'NotificationControllerTest' => __DIR__ . '/tests/phpunit/controller/NotificationControllerTest.php', 'NotificationPager' => __DIR__ . '/includes/special/NotificationPager.php', 'NotificationsTest' => __DIR__ . '/tests/NotificationsTest.php', diff --git a/tests/phpunit/ThankYouEditTest.php b/tests/phpunit/ThankYouEditTest.php new file mode 100644 index 000000000..1df66e206 --- /dev/null +++ b/tests/phpunit/ThankYouEditTest.php @@ -0,0 +1,58 @@ +getMutableTestUser()->getUser(); + $title = Title::newFromText( 'Help:MWEchoThankYouEditTest_testFirstEdit' ); + + // action + $this->edit( $title, $user, 'this is my first edit' ); + + // assertions + $notificationMapper = new EchoNotificationMapper(); + $notifications = $notificationMapper->fetchByUser( $user, 10, null, array( 'thank-you-edit' ) ); + $this->assertCount( 1, $notifications ); + + /** @var EchoNotification $notification */ + $notification = reset( $notifications ); + $this->assertEquals( 1, $notification->getEvent()->getExtraParam( 'editCount', 'not found' ) ); + } + + public function testTenthEdit() { + // setup + $user = $this->getMutableTestUser()->getUser(); + $title = Title::newFromText( 'Help:MWEchoThankYouEditTest_testTenthEdit' ); + + // action + // 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 + for ( $i = 0; $i < 12; $i++ ) { + $this->edit( $title, $user, "this is edit #$i" ); + } + + // assertions + $notificationMapper = new EchoNotificationMapper(); + $notifications = $notificationMapper->fetchByUser( $user, 10, null, array( 'thank-you-edit' ) ); + $this->assertCount( 2, $notifications ); + + /** @var EchoNotification $notification */ + $notification = reset( $notifications ); + $this->assertEquals( 10, $notification->getEvent()->getExtraParam( 'editCount', 'not found' ) ); + } + + private function edit( Title $title, User $user, $text ) { + $page = WikiPage::factory( $title ); + $content = ContentHandler::makeContent( $text, $title ); + $page->doEditContent( $content, 'test', 0, false, $user ); + } +}