diff --git a/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php b/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php index 4dd4a8be2..6c6ef785f 100644 --- a/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php +++ b/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php @@ -51,7 +51,7 @@ class ReplyNotificationTest extends TestCase /** @var User $mainUser */ $mainUser = User::query()->find(1); - $this->assertEquals(0, $this->getUnreadNotificationCount($mainUser)); + $this->assertEquals(0, $mainUser->getUnreadNotificationCount()); $this->send( $this->request('POST', '/api/posts', [ @@ -69,7 +69,7 @@ class ReplyNotificationTest extends TestCase ]) ); - $this->assertEquals(1, $this->getUnreadNotificationCount($mainUser)); + $this->assertEquals(1, $mainUser->getUnreadNotificationCount()); } /** @test */ @@ -108,7 +108,7 @@ class ReplyNotificationTest extends TestCase ]) ); - $this->assertEquals(0, $this->getUnreadNotificationCount($mainUser)); + $this->assertEquals(0, $mainUser->getUnreadNotificationCount()); $this->send( $this->request('POST', '/api/posts', [ @@ -126,17 +126,72 @@ class ReplyNotificationTest extends TestCase ]) ); - $this->assertEquals(1, $this->getUnreadNotificationCount($mainUser)); + $this->assertEquals(1, $mainUser->getUnreadNotificationCount()); } - /** @todo change after core no longer statically caches unread notification in the User class */ - protected function getUnreadNotificationCount(User $user) + /** + * @dataProvider deleteLastPostsProvider + * @test + */ + public function deleting_last_posts_then_posting_new_one_sends_reply_notification(array $postIds) { - return $user->notifications() - ->where('type', 'newPost') - ->whereNull('read_at') - ->where('is_deleted', false) - ->whereSubjectVisibleTo($user) - ->count(); + $this->prepareDatabase([ + 'users' => [ + ['id' => 3, 'username' => 'acme', 'email' => 'acme@machine.local', 'is_email_confirmed' => 1], + ], + 'discussions' => [ + ['id' => 3, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 5, 'last_post_number' => 5, 'last_post_id' => 10], + ], + 'posts' => [ + ['id' => 5, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 1], + ['id' => 6, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 2], + ['id' => 7, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 3], + ['id' => 8, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 4], + ['id' => 9, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 5], + ['id' => 10, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 6], + ], + 'discussion_user' => [ + ['discussion_id' => 3, 'user_id' => 2, 'last_read_post_number' => 6, 'subscription' => 'follow'], + ] + ]); + + // Delete the last 3 posts. + foreach ($postIds as $postId) { + $this->send( + $this->request('DELETE', '/api/posts/'.$postId, ['authenticatedAs' => 1]) + ); + } + + /** @var User $mainUser */ + $mainUser = User::query()->find(2); + + $this->assertEquals(0, $mainUser->getUnreadNotificationCount()); + + // Reply as another user + $this->send( + $this->request('POST', '/api/posts', [ + 'authenticatedAs' => 3, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => 'reply with predetermined content for automated testing - too-obscure', + ], + 'relationships' => [ + 'discussion' => ['data' => ['id' => 3]], + ], + ], + ], + ]) + ); + + $this->assertEquals(1, $mainUser->getUnreadNotificationCount()); + } + + public function deleteLastPostsProvider(): array + { + return [ + [[10, 9, 8]], + [[8, 9, 10]] + ]; } } diff --git a/framework/core/src/Discussion/DiscussionServiceProvider.php b/framework/core/src/Discussion/DiscussionServiceProvider.php index 453dbb4a7..d369839e8 100644 --- a/framework/core/src/Discussion/DiscussionServiceProvider.php +++ b/framework/core/src/Discussion/DiscussionServiceProvider.php @@ -19,6 +19,7 @@ class DiscussionServiceProvider extends AbstractServiceProvider public function boot(Dispatcher $events) { $events->subscribe(DiscussionMetadataUpdater::class); + $events->subscribe(UserStateUpdater::class); $events->listen( Renamed::class, diff --git a/framework/core/src/Discussion/UserState.php b/framework/core/src/Discussion/UserState.php index 3859e9e96..1fb7dd943 100644 --- a/framework/core/src/Discussion/UserState.php +++ b/framework/core/src/Discussion/UserState.php @@ -46,6 +46,13 @@ class UserState extends AbstractModel */ protected $dates = ['last_read_at']; + /** + * The attributes that are mass assignable. + * + * @var string[] + */ + protected $fillable = ['last_read_post_number']; + /** * Mark the discussion as being read up to a certain point. Raises the * DiscussionWasRead event. diff --git a/framework/core/src/Discussion/UserStateUpdater.php b/framework/core/src/Discussion/UserStateUpdater.php new file mode 100644 index 000000000..3c90d735d --- /dev/null +++ b/framework/core/src/Discussion/UserStateUpdater.php @@ -0,0 +1,39 @@ +listen(Deleted::class, [$this, 'whenPostWasDeleted']); + } + + /** + * Updates a user state relative to a discussion. + * If user A read a discussion all the way to post number N, and X last posts were deleted, + * then we need to update user A's read status to the new N-X post number so that they get notified by new posts. + */ + public function whenPostWasDeleted(Deleted $event) + { + /* + * We check if it's greater because at this point the DiscussionMetadataUpdater should have updated the last post. + */ + if ($event->post->number > $event->post->discussion->last_post_number) { + UserState::query() + ->where('discussion_id', $event->post->discussion_id) + ->where('last_read_post_number', '>', $event->post->discussion->last_post_number) + ->update(['last_read_post_number' => $event->post->discussion->last_post_number]); + } + } +} diff --git a/framework/core/tests/integration/api/posts/DeleteTest.php b/framework/core/tests/integration/api/posts/DeleteTest.php new file mode 100644 index 000000000..df347bc15 --- /dev/null +++ b/framework/core/tests/integration/api/posts/DeleteTest.php @@ -0,0 +1,94 @@ +prepareDatabase([ + 'users' => [ + ['id' => 3, 'username' => 'acme', 'email' => 'acme@machine.local', 'is_email_confirmed' => 1], + ['id' => 4, 'username' => 'acme2', 'email' => 'acme2@machine.local', 'is_email_confirmed' => 1], + ], + 'discussions' => [ + ['id' => 3, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 5, 'last_post_number' => 5, 'last_post_id' => 10], + ], + 'posts' => [ + ['id' => 5, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 1], + ['id' => 6, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 2], + ['id' => 7, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 3], + ['id' => 8, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 4], + ['id' => 9, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 5], + ['id' => 10, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

', 'number' => 6], + ], + 'discussion_user' => [ + ['discussion_id' => 3, 'user_id' => 2, 'last_read_post_number' => 6], + ['discussion_id' => 3, 'user_id' => 4, 'last_read_post_number' => 3], + ] + ]); + } + + /** + * @dataProvider deleteLastPostsProvider + * @test + */ + public function deleting_last_posts_syncs_discussion_state_for_other_users(array $postIds, int $newLastReadNumber, int $userId) + { + // Delete the last post. + foreach ($postIds as $postId) { + $this->send( + $this->request('DELETE', '/api/posts/'.$postId, ['authenticatedAs' => 1]) + ); + } + + // User 2 should now have last_read_post_number set to the new last one + $this->assertEquals( + $newLastReadNumber, + UserState::query() + ->where('discussion_id', 3) + ->where('user_id', $userId) + ->first() + ->last_read_post_number + ); + } + + public function deleteLastPostsProvider(): array + { + return [ + // User 2 + [[10], 5, 2], + [[9, 10], 4, 2], + [[10, 9, 8], 3, 2], + [[8, 9, 10], 3, 2], + [[7, 8, 9, 10], 2, 2], + + // User 4 + [[10], 3, 4], + [[9, 10], 3, 4], + [[10, 9, 8], 3, 4], + [[8, 9, 10], 3, 4], + [[10, 9, 8, 7], 2, 4], + [[7, 8, 9, 10], 2, 4], + ]; + } +}