From 65380413cb27376c0da8d1c7ae57d8272cef03e5 Mon Sep 17 00:00:00 2001 From: Ian Morland Date: Sun, 29 Oct 2023 18:24:06 +0000 Subject: [PATCH 1/6] chore: create tests to highlight the defect (failing) --- .gitignore | 1 + composer.json | 26 ++- src/Console/NotifySchedule.php | 18 +- src/Listeners/SaveBestAnswerToDatabase.php | 4 +- tests/fixtures/.gitkeep | 0 tests/integration/api/BestAnswerTest.php | 186 ++++++++++++++++++++ tests/integration/setup.php | 16 ++ tests/phpunit.integration.xml | 25 +++ tests/phpunit.unit.xml | 27 +++ tests/unit/.gitkeep | 0 tests/unit/SaveBestAnswerToDatabaseTest.php | 127 +++++++++++++ 11 files changed, 419 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/.gitkeep create mode 100644 tests/integration/api/BestAnswerTest.php create mode 100644 tests/integration/setup.php create mode 100644 tests/phpunit.integration.xml create mode 100644 tests/phpunit.unit.xml create mode 100644 tests/unit/.gitkeep create mode 100644 tests/unit/SaveBestAnswerToDatabaseTest.php diff --git a/.gitignore b/.gitignore index 04fdede..3b19fb0 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ js/dist js/node_modules vendor composer.lock +.phpunit.result.cache diff --git a/composer.json b/composer.json index 0528e5b..f0de410 100644 --- a/composer.json +++ b/composer.json @@ -62,18 +62,36 @@ }, "flarum-cli": { "modules": { - "githubActions": true + "githubActions": true, + "backendTesting": true } } }, "scripts": { "analyse:phpstan": "phpstan analyse", - "clear-cache:phpstan": "phpstan clear-result-cache" + "clear-cache:phpstan": "phpstan clear-result-cache", + "test": [ + "@test:unit", + "@test:integration" + ], + "test:unit": "phpunit -c tests/phpunit.unit.xml", + "test:integration": "phpunit -c tests/phpunit.integration.xml", + "test:setup": "@php tests/integration/setup.php" }, "scripts-descriptions": { - "analyse:phpstan": "Run static analysis" + "analyse:phpstan": "Run static analysis", + "test": "Runs all tests.", + "test:unit": "Runs all unit tests.", + "test:integration": "Runs all integration tests.", + "test:setup": "Sets up a database for use with integration tests. Execute this only once." }, "require-dev": { - "flarum/phpstan": "^1.8" + "flarum/phpstan": "^1.8", + "flarum/testing": "^1.0.0" + }, + "autoload-dev": { + "psr-4": { + "FoF\\BestAnswer\\Tests\\": "tests/" + } } } diff --git a/src/Console/NotifySchedule.php b/src/Console/NotifySchedule.php index 8c5254d..dbbb1fc 100644 --- a/src/Console/NotifySchedule.php +++ b/src/Console/NotifySchedule.php @@ -17,23 +17,31 @@ class NotifySchedule { + /** + * @var SettingsRepositoryInterface + */ + public $settings; + + public function __construct(SettingsRepositoryInterface $settings) + { + $this->settings = $settings; + } + public function __invoke(Event $event) { - $settings = resolve(SettingsRepositoryInterface::class); - $event->hourly() ->withoutOverlapping(); - if ((bool) $settings->get('fof-best-answer.schedule_on_one_server')) { + if ((bool) $this->settings->get('fof-best-answer.schedule_on_one_server')) { $event->onOneServer(); } - if ((bool) $settings->get('fof-best-answer.stop_overnight')) { + if ((bool) $this->settings->get('fof-best-answer.stop_overnight')) { $event->between('8:00', '21:00'); //TODO expose times back to config options } - if ((bool) $settings->get('fof-best-answer.store_log_output')) { + if ((bool) $this->settings->get('fof-best-answer.store_log_output')) { $paths = resolve(Paths::class); $event->appendOutputTo($paths->storage.(DIRECTORY_SEPARATOR.'logs'.DIRECTORY_SEPARATOR.'fof-best-answer.log')); } diff --git a/src/Listeners/SaveBestAnswerToDatabase.php b/src/Listeners/SaveBestAnswerToDatabase.php index d7347ee..29e5029 100644 --- a/src/Listeners/SaveBestAnswerToDatabase.php +++ b/src/Listeners/SaveBestAnswerToDatabase.php @@ -91,7 +91,7 @@ public function handle(Saving $event) $this->notifications->delete(new SelectBestAnswerBlueprint($discussion)); } - private function removeBestAnswer(Discussion $discussion, User $actor): void + protected function removeBestAnswer(Discussion $discussion, User $actor): void { /** @var Post|null $post */ $post = $discussion->bestAnswerPost; @@ -112,7 +112,7 @@ private function removeBestAnswer(Discussion $discussion, User $actor): void }); } - private function setBestAnswer(Discussion $discussion, User $actor, int $id): void + protected function setBestAnswer(Discussion $discussion, User $actor, int $id): void { /** @var Post|null $post */ $post = $discussion->posts()->find($id); diff --git a/tests/fixtures/.gitkeep b/tests/fixtures/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/api/BestAnswerTest.php b/tests/integration/api/BestAnswerTest.php new file mode 100644 index 0000000..20a08e6 --- /dev/null +++ b/tests/integration/api/BestAnswerTest.php @@ -0,0 +1,186 @@ +extension('flarum-tags'); + $this->extension('fof-best-answer'); + + $this->prepareDatabase([ + 'users' => [ + $this->normalUser(), + ['id' => 3, 'username' => 'normal2', 'email' => 'normal2@machine.local', 'best_answer_count' => 1], + ], + 'discussions' => [ + ['id' => 1, 'title' => __CLASS__, 'user_id' => 1, 'created_at' => Carbon::now(), 'comment_count' => 2, 'best_answer_post_id' => null], + ['id' => 2, 'title' => __CLASS__, 'user_id' => 1, 'created_at' => Carbon::now(), 'comment_count' => 2, 'best_answer_post_id' => 4, 'best_answer_user_id' => 1, 'best_answer_set_at' => Carbon::now()], + ['id' => 3, 'title' => __CLASS__, 'user_id' => 1, 'created_at' => Carbon::now(), 'comment_count' => 2, 'best_answer_post_id' => 6, 'best_answer_user_id' => 1, 'best_answer_set_at' => Carbon::now()], + ], + 'posts' => [ + ['id' => 1, 'discussion_id' => 1, 'user_id' => 1, 'type' => 'comment', 'content' => 'post 1', 'created_at' => Carbon::now()], + ['id' => 2, 'discussion_id' => 1, 'user_id' => 2, 'type' => 'comment', 'content' => 'post 2', 'created_at' => Carbon::now()], + ['id' => 3, 'discussion_id' => 2, 'user_id' => 2, 'type' => 'comment', 'content' => 'post 1', 'created_at' => Carbon::now()], + ['id' => 4, 'discussion_id' => 2, 'user_id' => 1, 'type' => 'comment', 'content' => 'post 2', 'created_at' => Carbon::now()], + ['id' => 5, 'discussion_id' => 3, 'user_id' => 2, 'type' => 'comment', 'content' => 'post 1', 'created_at' => Carbon::now()], + ['id' => 6, 'discussion_id' => 3, 'user_id' => 3, 'type' => 'comment', 'content' => 'post 2', 'created_at' => Carbon::now()], + + ], + 'tags' => [ + ['id' => 1, 'name' => 'Tag 1', 'slug' => 'tag-1', 'description' => 'Tag 1 description', 'color' => '#FF0000', 'position' => 0, 'parent_id' => null, 'is_restricted' => false, 'is_hidden' => false, 'is_qna' => true], + ], + 'discussion_tag' => [ + ['discussion_id' => 1, 'tag_id' => 1], + ['discussion_id' => 2, 'tag_id' => 1], + ], + ]); + } + + /** + * @test + */ + public function default_answer_count_for_new_user_is_zero() + { + $response = $this->send( + $this->request( + 'GET', + '/api/users/2', + [ + 'authenticatedAs' => '1' + ], + ) + ); + + $this->assertEquals(200, $response->getStatusCode()); + + $data = json_decode($response->getBody()->getContents(), true); + + $this->assertEquals(0, $data['data']['attributes']['bestAnswerCount']); + } + + /** + * @test + */ + public function user_with_extising_best_answer_has_correct_count() + { + $response = $this->send( + $this->request( + 'GET', + '/api/users/1', + [ + 'authenticatedAs' => '2' + ], + ) + ); + + $this->assertEquals(200, $response->getStatusCode()); + + $data = json_decode($response->getBody()->getContents(), true); + + $this->assertEquals(1, $data['data']['attributes']['bestAnswerCount']); + } + + /** + * @test + */ + public function setting_best_answer_increases_author_best_answer_count() + { + $response = $this->send( + $this->request( + 'PATCH', + '/api/discussions/1', + [ + 'json' => [ + 'data' => [ + 'attributes' => [ + 'bestAnswerPostId' => 2, + 'bestAnswerUserId' => 1 + ], + ] + + ], + 'authenticatedAs' => '1' + ], + ) + ); + + $this->assertEquals(200, $response->getStatusCode()); + + $discussion = Discussion::find(1); + + $this->assertEquals(2, $discussion->best_answer_post_id); + + $answerAuthor = User::find(2); + + $this->assertEquals(1, $answerAuthor->best_answer_count); + } + + /** + * @test + */ + public function unsetting_best_answer_decreases_author_best_answer_count() + { + $response = $this->send( + $this->request( + 'PATCH', + '/api/discussions/2', + [ + 'json' => [ + 'data' => [ + 'attributes' => [ + 'bestAnswerPostId' => 0, + ], + ] + + ], + 'authenticatedAs' => '1' + ], + ) + ); + + $this->assertEquals(200, $response->getStatusCode()); + + $discussion = Discussion::find(2); + + $this->assertNull($discussion->best_answer_post_id); + + $answerAuthor = User::find(1); + + $this->assertEquals(0, $answerAuthor->best_answer_count); + } + + /** + * @test + */ + public function deleting_a_discussion_with_a_best_answer_reduces_author_best_answer_count() + { + $response = $this->send( + $this->request( + 'DELETE', + '/api/discussions/3', + [ + 'authenticatedAs' => '1', + 'json' => [], + ], + ) + ); + + $this->assertEquals(204, $response->getStatusCode()); + + $answerAuthor = User::find(3); + + $this->assertEquals(0, $answerAuthor->best_answer_count); + } +} diff --git a/tests/integration/setup.php b/tests/integration/setup.php new file mode 100644 index 0000000..67039c0 --- /dev/null +++ b/tests/integration/setup.php @@ -0,0 +1,16 @@ +run(); diff --git a/tests/phpunit.integration.xml b/tests/phpunit.integration.xml new file mode 100644 index 0000000..90fbbff --- /dev/null +++ b/tests/phpunit.integration.xml @@ -0,0 +1,25 @@ + + + + + ../src/ + + + + + ./integration + ./integration/tmp + + + diff --git a/tests/phpunit.unit.xml b/tests/phpunit.unit.xml new file mode 100644 index 0000000..d3a4a3e --- /dev/null +++ b/tests/phpunit.unit.xml @@ -0,0 +1,27 @@ + + + + + ../src/ + + + + + ./unit + + + + + + diff --git a/tests/unit/.gitkeep b/tests/unit/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php new file mode 100644 index 0000000..397f031 --- /dev/null +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -0,0 +1,127 @@ +data = []; + + $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer,setBestAnswer]', [ + m::mock(NotificationSyncer::class), + m::mock(Dispatcher::class), + m::mock(TranslatorInterface::class), + m::mock(BestAnswerRepository::class), + m::mock(SettingsRepositoryInterface::class) + ])->shouldAllowMockingProtectedMethods(); + + $this->sut->shouldNotReceive('removeBestAnswer'); + $this->sut->shouldNotReceive('setBestAnswer'); + + $this->sut->handle($event); + } + + public function testHandle_DiscussionDoesNotExistOrMatches_ReturnsWithoutAction() + { + $event = m::mock(Saving::class); + $event->data = ['attributes.bestAnswerPostId' => 1]; + $event->discussion = m::mock(Discussion::class)->makePartial(); + $event->actor = m::mock(User::class); + + $event->discussion->exists = false; + $event->discussion->best_answer_post_id = 1; + + $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer,setBestAnswer]', [ + m::mock(NotificationSyncer::class), + m::mock(Dispatcher::class), + m::mock(TranslatorInterface::class), + m::mock(BestAnswerRepository::class), + m::mock(SettingsRepositoryInterface::class) + ])->shouldAllowMockingProtectedMethods(); + + $this->sut->shouldNotReceive('removeBestAnswer'); + $this->sut->shouldNotReceive('setBestAnswer'); + + $this->sut->handle($event); + } + + public function testHandle_IdIsZero_CallsRemoveBestAnswer() + { + $event = m::mock(Saving::class); + $event->data = ['attributes.bestAnswerPostId' => 0]; + $event->discussion = m::mock(Discussion::class)->makePartial(); + $event->actor = m::mock(User::class); + + $event->discussion->exists = true; + $event->discussion->best_answer_post_id = 1; + + $notifications = m::mock(NotificationSyncer::class); + $notifications->shouldReceive('delete')->with(m::type(SelectBestAnswerBlueprint::class))->once(); + + $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer]', [ + $notifications, + m::mock(Dispatcher::class), + m::mock(TranslatorInterface::class), + m::mock(BestAnswerRepository::class), + m::mock(SettingsRepositoryInterface::class) + ])->shouldAllowMockingProtectedMethods(); + + $this->sut->shouldReceive('removeBestAnswer')->once(); + + $this->sut->handle($event); + } + + public function testHandle_IdIsNotZero_CallsSetBestAnswer() + { + $event = m::mock(Saving::class); + $event->data = ['attributes.bestAnswerPostId' => 2]; + $event->discussion = m::mock(Discussion::class)->makePartial(); + $event->actor = m::mock(User::class); + + $event->discussion->exists = true; + $event->discussion->best_answer_post_id = 1; + + $notifications = m::mock(NotificationSyncer::class); + $notifications->shouldReceive('delete')->with(m::type(SelectBestAnswerBlueprint::class))->once(); + + $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[setBestAnswer]', [ + $notifications, + m::mock(Dispatcher::class), + m::mock(TranslatorInterface::class), + m::mock(BestAnswerRepository::class), + m::mock(SettingsRepositoryInterface::class) + ])->shouldAllowMockingProtectedMethods(); + + $this->sut->shouldReceive('setBestAnswer')->once(); + + $this->sut->handle($event); + } +} From d001417fc8e60f46af27b9184aaab9b23e3a1533 Mon Sep 17 00:00:00 2001 From: Ian Morland Date: Sun, 29 Oct 2023 19:07:19 +0000 Subject: [PATCH 2/6] fix: listen for deletion, reduce ba count --- src/Listeners/RecalculateBestAnswerCounts.php | 40 +++++++++++++++++++ tests/integration/api/BestAnswerTest.php | 40 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/Listeners/RecalculateBestAnswerCounts.php b/src/Listeners/RecalculateBestAnswerCounts.php index a8a2aa4..502fa51 100644 --- a/src/Listeners/RecalculateBestAnswerCounts.php +++ b/src/Listeners/RecalculateBestAnswerCounts.php @@ -11,6 +11,10 @@ namespace FoF\BestAnswer\Listeners; +use Flarum\Discussion\Event\Deleting as DiscussionDeleting; +use Flarum\Post\Event\Deleting as PostDeleting; +use Flarum\Post\Post; +use Flarum\User\User; use FoF\BestAnswer\Events\BestAnswerSet; use FoF\BestAnswer\Events\BestAnswerUnset; use Illuminate\Contracts\Events\Dispatcher; @@ -21,6 +25,8 @@ public function subscribe(Dispatcher $events) { $events->listen(BestAnswerSet::class, [$this, 'addBestAnswerCount']); $events->listen(BestAnswerUnset::class, [$this, 'subtractBestAnswerCount']); + $events->listen(PostDeleting::class, [$this, 'handlePostDeletion']); + $events->listen(DiscussionDeleting::class, [$this, 'handleDiscussionDeletion']); } public function addBestAnswerCount(BestAnswerSet $event): void @@ -42,4 +48,38 @@ public function subtractBestAnswerCount(BestAnswerUnset $event): void $author->save(); } } + + public function handlePostDeletion(PostDeleting $event): void + { + $post = $event->post; + + if ($post->discussion->best_answer_post_id === $post->id) { + $post->discussion->best_answer_post_id = null; + $post->discussion->best_answer_user_id = null; + $post->discussion->best_answer_set_at = null; + $post->discussion->save(); + + $author = $post->user; + + if ($author) { + $author->best_answer_count--; + $author->save(); + } + } + } + + public function handleDiscussionDeletion(DiscussionDeleting $event): void + { + $discussion = $event->discussion; + + if ($discussion->best_answer_post_id) { + $post = Post::find($discussion->best_answer_post_id); + $author = $post->user; + + if ($author) { + $author->best_answer_count--; + $author->save(); + } + } + } } diff --git a/tests/integration/api/BestAnswerTest.php b/tests/integration/api/BestAnswerTest.php index 20a08e6..8258714 100644 --- a/tests/integration/api/BestAnswerTest.php +++ b/tests/integration/api/BestAnswerTest.php @@ -4,6 +4,7 @@ use Carbon\Carbon; use Flarum\Discussion\Discussion; +use Flarum\Post\Post; use Flarum\Testing\integration\RetrievesAuthorizedUsers; use Flarum\Testing\integration\TestCase; use Flarum\User\User; @@ -161,6 +162,37 @@ public function unsetting_best_answer_decreases_author_best_answer_count() $this->assertEquals(0, $answerAuthor->best_answer_count); } + /** + * @test + */ + public function deleting_the_best_answer_post_in_a_discussion_reduces_author_best_answer_count() + { + $response = $this->send( + $this->request( + 'DELETE', + '/api/posts/6', + [ + 'authenticatedAs' => '1', + 'json' => [], + ], + ) + ); + + $this->assertEquals(204, $response->getStatusCode()); + + $discussion = Discussion::find(3); + + $this->assertNull($discussion->best_answer_post_id); + + $post = Post::find(6); + + $this->assertNull($post); + + $answerAuthor = User::find(3); + + $this->assertEquals(0, $answerAuthor->best_answer_count); + } + /** * @test */ @@ -179,6 +211,14 @@ public function deleting_a_discussion_with_a_best_answer_reduces_author_best_ans $this->assertEquals(204, $response->getStatusCode()); + $discussion = Discussion::find(3); + + $this->assertNull($discussion); + + $post = Post::find(6); + + $this->assertNull($post); + $answerAuthor = User::find(3); $this->assertEquals(0, $answerAuthor->best_answer_count); From dbfc46afb97458f235a193e040e4c502b56da693 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sun, 29 Oct 2023 19:07:57 +0000 Subject: [PATCH 3/6] Apply fixes from StyleCI --- src/Console/NotifySchedule.php | 4 +-- src/Listeners/RecalculateBestAnswerCounts.php | 1 - tests/integration/api/BestAnswerTest.php | 33 ++++++++++++------- tests/integration/setup.php | 8 +++-- tests/unit/SaveBestAnswerToDatabaseTest.php | 25 +++++++++----- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/src/Console/NotifySchedule.php b/src/Console/NotifySchedule.php index dbbb1fc..a705851 100644 --- a/src/Console/NotifySchedule.php +++ b/src/Console/NotifySchedule.php @@ -21,12 +21,12 @@ class NotifySchedule * @var SettingsRepositoryInterface */ public $settings; - + public function __construct(SettingsRepositoryInterface $settings) { $this->settings = $settings; } - + public function __invoke(Event $event) { $event->hourly() diff --git a/src/Listeners/RecalculateBestAnswerCounts.php b/src/Listeners/RecalculateBestAnswerCounts.php index 502fa51..82d32ce 100644 --- a/src/Listeners/RecalculateBestAnswerCounts.php +++ b/src/Listeners/RecalculateBestAnswerCounts.php @@ -14,7 +14,6 @@ use Flarum\Discussion\Event\Deleting as DiscussionDeleting; use Flarum\Post\Event\Deleting as PostDeleting; use Flarum\Post\Post; -use Flarum\User\User; use FoF\BestAnswer\Events\BestAnswerSet; use FoF\BestAnswer\Events\BestAnswerUnset; use Illuminate\Contracts\Events\Dispatcher; diff --git a/tests/integration/api/BestAnswerTest.php b/tests/integration/api/BestAnswerTest.php index 8258714..610f4ac 100644 --- a/tests/integration/api/BestAnswerTest.php +++ b/tests/integration/api/BestAnswerTest.php @@ -1,5 +1,14 @@ '1' + 'authenticatedAs' => '1', ], ) ); @@ -81,7 +90,7 @@ public function user_with_extising_best_answer_has_correct_count() 'GET', '/api/users/1', [ - 'authenticatedAs' => '2' + 'authenticatedAs' => '2', ], ) ); @@ -107,12 +116,12 @@ public function setting_best_answer_increases_author_best_answer_count() 'data' => [ 'attributes' => [ 'bestAnswerPostId' => 2, - 'bestAnswerUserId' => 1 + 'bestAnswerUserId' => 1, ], - ] - + ], + ], - 'authenticatedAs' => '1' + 'authenticatedAs' => '1', ], ) ); @@ -143,10 +152,10 @@ public function unsetting_best_answer_decreases_author_best_answer_count() 'attributes' => [ 'bestAnswerPostId' => 0, ], - ] - + ], + ], - 'authenticatedAs' => '1' + 'authenticatedAs' => '1', ], ) ); @@ -173,7 +182,7 @@ public function deleting_the_best_answer_post_in_a_discussion_reduces_author_bes '/api/posts/6', [ 'authenticatedAs' => '1', - 'json' => [], + 'json' => [], ], ) ); @@ -204,7 +213,7 @@ public function deleting_a_discussion_with_a_best_answer_reduces_author_best_ans '/api/discussions/3', [ 'authenticatedAs' => '1', - 'json' => [], + 'json' => [], ], ) ); diff --git a/tests/integration/setup.php b/tests/integration/setup.php index 67039c0..259b0f0 100644 --- a/tests/integration/setup.php +++ b/tests/integration/setup.php @@ -1,10 +1,12 @@ data = []; - $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer,setBestAnswer]', [ + $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[removeBestAnswer,setBestAnswer]', [ m::mock(NotificationSyncer::class), m::mock(Dispatcher::class), m::mock(TranslatorInterface::class), m::mock(BestAnswerRepository::class), - m::mock(SettingsRepositoryInterface::class) + m::mock(SettingsRepositoryInterface::class), ])->shouldAllowMockingProtectedMethods(); $this->sut->shouldNotReceive('removeBestAnswer'); @@ -59,12 +68,12 @@ public function testHandle_DiscussionDoesNotExistOrMatches_ReturnsWithoutAction( $event->discussion->exists = false; $event->discussion->best_answer_post_id = 1; - $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer,setBestAnswer]', [ + $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[removeBestAnswer,setBestAnswer]', [ m::mock(NotificationSyncer::class), m::mock(Dispatcher::class), m::mock(TranslatorInterface::class), m::mock(BestAnswerRepository::class), - m::mock(SettingsRepositoryInterface::class) + m::mock(SettingsRepositoryInterface::class), ])->shouldAllowMockingProtectedMethods(); $this->sut->shouldNotReceive('removeBestAnswer'); @@ -86,12 +95,12 @@ public function testHandle_IdIsZero_CallsRemoveBestAnswer() $notifications = m::mock(NotificationSyncer::class); $notifications->shouldReceive('delete')->with(m::type(SelectBestAnswerBlueprint::class))->once(); - $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[removeBestAnswer]', [ + $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[removeBestAnswer]', [ $notifications, m::mock(Dispatcher::class), m::mock(TranslatorInterface::class), m::mock(BestAnswerRepository::class), - m::mock(SettingsRepositoryInterface::class) + m::mock(SettingsRepositoryInterface::class), ])->shouldAllowMockingProtectedMethods(); $this->sut->shouldReceive('removeBestAnswer')->once(); @@ -112,12 +121,12 @@ public function testHandle_IdIsNotZero_CallsSetBestAnswer() $notifications = m::mock(NotificationSyncer::class); $notifications->shouldReceive('delete')->with(m::type(SelectBestAnswerBlueprint::class))->once(); - $this->sut = m::mock(SaveBestAnswerToDatabase::class . '[setBestAnswer]', [ + $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[setBestAnswer]', [ $notifications, m::mock(Dispatcher::class), m::mock(TranslatorInterface::class), m::mock(BestAnswerRepository::class), - m::mock(SettingsRepositoryInterface::class) + m::mock(SettingsRepositoryInterface::class), ])->shouldAllowMockingProtectedMethods(); $this->sut->shouldReceive('setBestAnswer')->once(); From 00d80333fc6183e3ca0d0a4cb5e5ca9f13a2b777 Mon Sep 17 00:00:00 2001 From: Ian Morland Date: Sun, 29 Oct 2023 19:09:36 +0000 Subject: [PATCH 4/6] chore: enable backend testing --- .github/workflows/backend.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index 63be078..6fdf293 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -6,7 +6,7 @@ jobs: run: uses: flarum/framework/.github/workflows/REUSABLE_backend.yml@main with: - enable_backend_testing: false + enable_backend_testing: true enable_phpstan: true backend_directory: . From d3d00306f7549985f03a54d7a22cc0aae0e15954 Mon Sep 17 00:00:00 2001 From: Ian Morland Date: Sun, 29 Oct 2023 19:23:47 +0000 Subject: [PATCH 5/6] feat: provide cli command to re-sync all user ba counts --- extend.php | 1 + src/BestAnswerRepository.php | 27 +++++++++++++ src/Console/UpdateBestAnswerCounts.php | 53 ++++++++++++++++++++++++++ src/UserBestAnswerCount.php | 40 ++++++------------- 4 files changed, 93 insertions(+), 28 deletions(-) create mode 100644 src/Console/UpdateBestAnswerCounts.php diff --git a/extend.php b/extend.php index 4fbc36a..579d59b 100644 --- a/extend.php +++ b/extend.php @@ -113,6 +113,7 @@ (new Extend\Console()) ->command(Console\NotifyCommand::class) + ->command(Console\UpdateBestAnswerCounts::class) ->schedule(Console\NotifyCommand::class, Console\NotifySchedule::class), (new Extend\Filter(DiscussionFilterer::class)) diff --git a/src/BestAnswerRepository.php b/src/BestAnswerRepository.php index 52f6e0e..27455b9 100644 --- a/src/BestAnswerRepository.php +++ b/src/BestAnswerRepository.php @@ -68,4 +68,31 @@ public function tagEnabledForBestAnswer(Discussion $discussion): bool return $enabled; } + + /** + * Calculate the number of best answers for a user. + * This is used when `best_answer_count` is `null` on the user, usually because either the user + * has not already been awarded any best answers, or the extension was updated to include this feature + * and the user has not yet been serialized. + * + * @param User $user + * + * @return int + */ + public function calculateBestAnswersForUser(User $user): int + { + $count = Discussion::whereNotNull('best_answer_post_id') + ->leftJoin('posts', 'posts.id', '=', 'discussions.best_answer_post_id') + ->where('posts.user_id', $user->id) + ->count(); + + // Use a standalone query and not attribute update+save because otherwise data added by extensions + // with Extend\ApiController::prepareDataForSerialization() ends up being added to the SQL UPDATE clause, + // and breaks Flarum since those are often not real columns + $user->newQuery() + ->where('id', $user->id) + ->update(['best_answer_count' => $count]); + + return $count; + } } diff --git a/src/Console/UpdateBestAnswerCounts.php b/src/Console/UpdateBestAnswerCounts.php new file mode 100644 index 0000000..c47c377 --- /dev/null +++ b/src/Console/UpdateBestAnswerCounts.php @@ -0,0 +1,53 @@ +bestAnswers = $bestAnswers; + } + + /** + * @var string + */ + protected $signature = 'fof:best-answer:update-counts'; + + /** + * @var string + */ + protected $description = 'Update best answer counts for all users'; + + public function handle() + { + $this->info('Updating best answer counts...'); + + $userCount = User::count(); + + $bar = $this->output->createProgressBar($userCount); + + User::query()->chunkById(100, function ($users) use ($bar) { + foreach ($users as $user) { + $user->best_answer_count = $this->bestAnswers->calculateBestAnswersForUser($user); + $user->save(); + + $bar->advance(); + } + }); + + $bar->finish(); + + $this->info('Done!'); + } +} diff --git a/src/UserBestAnswerCount.php b/src/UserBestAnswerCount.php index 32eaee8..5abd7c5 100644 --- a/src/UserBestAnswerCount.php +++ b/src/UserBestAnswerCount.php @@ -12,42 +12,26 @@ namespace FoF\BestAnswer; use Flarum\Api\Serializer\UserSerializer; -use Flarum\Discussion\Discussion; use Flarum\User\User; class UserBestAnswerCount { + /** + * @var BestAnswerRepository + */ + public $bestAnswers; + + public function __construct(BestAnswerRepository $bestAnswers) + { + $this->bestAnswers = $bestAnswers; + } + public function __invoke(UserSerializer $serializer, User $user, array $attributes): array { - $attributes['bestAnswerCount'] = $user->best_answer_count ?? $this->calculateBestAnswersForUser($user); + $attributes['bestAnswerCount'] = $user->best_answer_count ?? $this->bestAnswers->calculateBestAnswersForUser($user); return $attributes; } - /** - * Calculate the number of best answers for a user. - * This is used when `best_answer_count` is `null` on the user, usually because either the user - * has not already been awarded any best answers, or the extension was updated to include this feature - * and the user has not yet been serialized. - * - * @param User $user - * - * @return int - */ - private function calculateBestAnswersForUser(User $user): int - { - $count = Discussion::whereNotNull('best_answer_post_id') - ->leftJoin('posts', 'posts.id', '=', 'discussions.best_answer_post_id') - ->where('posts.user_id', $user->id) - ->count(); - - // Use a standalone query and not attribute update+save because otherwise data added by extensions - // with Extend\ApiController::prepareDataForSerialization() ends up being added to the SQL UPDATE clause, - // and breaks Flarum since those are often not real columns - $user->newQuery() - ->where('id', $user->id) - ->update(['best_answer_count' => $count]); - - return $count; - } + } From e5dc28b49e42efc01a48ddf1728e65bd95e446b7 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sun, 29 Oct 2023 19:23:55 +0000 Subject: [PATCH 6/6] Apply fixes from StyleCI --- src/Console/UpdateBestAnswerCounts.php | 13 +++++++++++-- src/UserBestAnswerCount.php | 6 ++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Console/UpdateBestAnswerCounts.php b/src/Console/UpdateBestAnswerCounts.php index c47c377..ff2ef56 100644 --- a/src/Console/UpdateBestAnswerCounts.php +++ b/src/Console/UpdateBestAnswerCounts.php @@ -1,5 +1,14 @@ bestAnswers = $bestAnswers; } - + /** * @var string */ diff --git a/src/UserBestAnswerCount.php b/src/UserBestAnswerCount.php index 5abd7c5..3b94001 100644 --- a/src/UserBestAnswerCount.php +++ b/src/UserBestAnswerCount.php @@ -20,18 +20,16 @@ class UserBestAnswerCount * @var BestAnswerRepository */ public $bestAnswers; - + public function __construct(BestAnswerRepository $bestAnswers) { $this->bestAnswers = $bestAnswers; } - + public function __invoke(UserSerializer $serializer, User $user, array $attributes): array { $attributes['bestAnswerCount'] = $user->best_answer_count ?? $this->bestAnswers->calculateBestAnswersForUser($user); return $attributes; } - - }