From b8ef3eee5fda2b2154eaf1b386fcb04254722b91 Mon Sep 17 00:00:00 2001 From: IanM Date: Mon, 11 Nov 2024 20:21:53 +0000 Subject: [PATCH 01/12] chore: Restructure files, bind repository --- extend.php | 40 ++-- src/Api/AddTagAttributes.php | 17 ++ src/{ => Api}/BasicDiscussionAttributes.php | 2 +- src/{ => Api}/DiscussionAttributes.php | 3 +- src/{ => Api}/ForumAttributes.php | 2 +- src/{ => Api}/UserBestAnswerCount.php | 3 +- src/BestAnswerRepository.php | 103 ---------- src/Console/UpdateBestAnswerCounts.php | 2 +- src/Listeners/SaveBestAnswerToDatabase.php | 123 +---------- src/Providers/BestAnswerServiceProvider.php | 14 ++ src/Repository/BestAnswerRepository.php | 213 ++++++++++++++++++++ 11 files changed, 270 insertions(+), 252 deletions(-) create mode 100644 src/Api/AddTagAttributes.php rename src/{ => Api}/BasicDiscussionAttributes.php (96%) rename src/{ => Api}/DiscussionAttributes.php (91%) rename src/{ => Api}/ForumAttributes.php (98%) rename src/{ => Api}/UserBestAnswerCount.php (90%) delete mode 100644 src/BestAnswerRepository.php create mode 100644 src/Providers/BestAnswerServiceProvider.php create mode 100644 src/Repository/BestAnswerRepository.php diff --git a/extend.php b/extend.php index cb0e77d..19e2451 100644 --- a/extend.php +++ b/extend.php @@ -11,12 +11,7 @@ namespace FoF\BestAnswer; -use Flarum\Api\Controller\ListDiscussionsController; -use Flarum\Api\Controller\ListPostsController; -use Flarum\Api\Controller\ListUsersController; -use Flarum\Api\Controller\ShowDiscussionController; -use Flarum\Api\Controller\ShowPostController; -use Flarum\Api\Controller\UpdateDiscussionController; +use Flarum\Api\Controller; use Flarum\Api\Serializer; use Flarum\Discussion\Discussion; use Flarum\Discussion\Event\Saving as DiscussionSaving; @@ -29,7 +24,6 @@ use Flarum\Tags\Api\Serializer\TagSerializer; use Flarum\Tags\Tag; use Flarum\User\User; -use FoF\BestAnswer\Events\BestAnswerSet; return [ (new Extend\Frontend('forum')) @@ -42,6 +36,9 @@ new Extend\Locales(__DIR__.'/resources/locale'), + (new Extend\ServiceProvider()) + ->register(Providers\BestAnswerServiceProvider::class), + (new Extend\Model(Discussion::class)) ->belongsTo('bestAnswerPost', Post::class, 'best_answer_post_id') ->belongsTo('bestAnswerUser', User::class, 'best_answer_user_id') @@ -62,7 +59,7 @@ (new Extend\Event()) ->listen(DiscussionSaving::class, Listeners\SaveBestAnswerToDatabase::class) - ->listen(BestAnswerSet::class, Listeners\QueueNotificationJobs::class) + ->listen(Events\BestAnswerSet::class, Listeners\QueueNotificationJobs::class) ->subscribe(Listeners\RecalculateBestAnswerCounts::class) ->listen(SettingsSaving::class, Listeners\SaveTagSettings::class), @@ -72,17 +69,17 @@ ->type(Notification\BestAnswerSetInDiscussionBlueprint::class, Serializer\BasicDiscussionSerializer::class, []), (new Extend\ApiSerializer(Serializer\DiscussionSerializer::class)) - ->attributes(DiscussionAttributes::class), + ->attributes(Api\DiscussionAttributes::class), (new Extend\ApiSerializer(Serializer\BasicDiscussionSerializer::class)) ->hasOne('bestAnswerPost', Serializer\BasicPostSerializer::class) ->hasOne('bestAnswerUser', Serializer\BasicUserSerializer::class) - ->attributes(BasicDiscussionAttributes::class), + ->attributes(Api\BasicDiscussionAttributes::class), (new Extend\ApiSerializer(Serializer\UserSerializer::class)) - ->attributes(UserBestAnswerCount::class), + ->attributes(Api\UserBestAnswerCount::class), - (new Extend\ApiController(ListUsersController::class)) + (new Extend\ApiController(Controller\ListUsersController::class)) ->addSortField('bestAnswerCount'), (new Extend\Settings()) @@ -97,23 +94,23 @@ ->serializeToForum('fof-best-answer.show_max_lines', 'fof-best-answer.show_max_lines', 'intVal'), (new Extend\ApiSerializer(Serializer\ForumSerializer::class)) - ->attributes(ForumAttributes::class), + ->attributes(Api\ForumAttributes::class), - (new Extend\ApiController(ShowDiscussionController::class)) + (new Extend\ApiController(Controller\ShowDiscussionController::class)) ->addInclude(['bestAnswerPost', 'bestAnswerUser', 'bestAnswerPost.user']) ->load(['bestAnswerPost', 'bestAnswerPost.user']), - (new Extend\ApiController(ListDiscussionsController::class)) + (new Extend\ApiController(Controller\ListDiscussionsController::class)) ->addOptionalInclude(['bestAnswerPost', 'bestAnswerUser', 'bestAnswerPost.discussion', 'bestAnswerPost.user']), - (new Extend\ApiController(UpdateDiscussionController::class)) + (new Extend\ApiController(Controller\UpdateDiscussionController::class)) ->addOptionalInclude('tags'), - (new Extend\ApiController(ListPostsController::class)) + (new Extend\ApiController(Controller\ListPostsController::class)) ->addInclude(['discussion', 'discussion.bestAnswerPost', 'discussion.bestAnswerUser', 'discussion.bestAnswerPost.user']) ->load(['discussion', 'discussion.bestAnswerUser', 'discussion.bestAnswerPost', 'discussion.bestAnswerPost.user']), - (new Extend\ApiController(ShowPostController::class)) + (new Extend\ApiController(Controller\ShowPostController::class)) ->addInclude(['discussion', 'discussion.bestAnswerPost', 'discussion.bestAnswerUser', 'discussion.bestAnswerPost.user']) ->load(['discussion', 'discussion.bestAnswerUser', 'discussion.bestAnswerPost', 'discussion.bestAnswerPost.user']), @@ -132,10 +129,5 @@ ->addFilter(Search\BestAnswerPostFilter::class), (new Extend\ApiSerializer(TagSerializer::class)) - ->attributes(function (TagSerializer $serializer, Tag $tag, array $attributes) { - $attributes['isQnA'] = (bool) $tag->is_qna; - $attributes['reminders'] = (bool) $tag->qna_reminders; - - return $attributes; - }), + ->attributes(Api\AddTagAttributes::class), ]; diff --git a/src/Api/AddTagAttributes.php b/src/Api/AddTagAttributes.php new file mode 100644 index 0000000..3a9fdd1 --- /dev/null +++ b/src/Api/AddTagAttributes.php @@ -0,0 +1,17 @@ +is_qna; + $attributes['reminders'] = (bool) $tag->qna_reminders; + + return $attributes; + } +} diff --git a/src/BasicDiscussionAttributes.php b/src/Api/BasicDiscussionAttributes.php similarity index 96% rename from src/BasicDiscussionAttributes.php rename to src/Api/BasicDiscussionAttributes.php index 2419d31..882c71b 100644 --- a/src/BasicDiscussionAttributes.php +++ b/src/Api/BasicDiscussionAttributes.php @@ -9,7 +9,7 @@ * file that was distributed with this source code. */ -namespace FoF\BestAnswer; +namespace FoF\BestAnswer\Api; use Flarum\Api\Serializer\BasicDiscussionSerializer; use Flarum\Discussion\Discussion; diff --git a/src/DiscussionAttributes.php b/src/Api/DiscussionAttributes.php similarity index 91% rename from src/DiscussionAttributes.php rename to src/Api/DiscussionAttributes.php index 05400e6..889d0e1 100644 --- a/src/DiscussionAttributes.php +++ b/src/Api/DiscussionAttributes.php @@ -9,10 +9,11 @@ * file that was distributed with this source code. */ -namespace FoF\BestAnswer; +namespace FoF\BestAnswer\Api; use Flarum\Api\Serializer\DiscussionSerializer; use Flarum\Discussion\Discussion; +use FoF\BestAnswer\Repository\BestAnswerRepository; class DiscussionAttributes { diff --git a/src/ForumAttributes.php b/src/Api/ForumAttributes.php similarity index 98% rename from src/ForumAttributes.php rename to src/Api/ForumAttributes.php index 67cf1a8..6c038a1 100644 --- a/src/ForumAttributes.php +++ b/src/Api/ForumAttributes.php @@ -9,7 +9,7 @@ * file that was distributed with this source code. */ -namespace FoF\BestAnswer; +namespace FoF\BestAnswer\Api; use Flarum\Api\Serializer\ForumSerializer; use Flarum\Settings\SettingsRepositoryInterface; diff --git a/src/UserBestAnswerCount.php b/src/Api/UserBestAnswerCount.php similarity index 90% rename from src/UserBestAnswerCount.php rename to src/Api/UserBestAnswerCount.php index 3b94001..799c5a4 100644 --- a/src/UserBestAnswerCount.php +++ b/src/Api/UserBestAnswerCount.php @@ -9,10 +9,11 @@ * file that was distributed with this source code. */ -namespace FoF\BestAnswer; +namespace FoF\BestAnswer\Api; use Flarum\Api\Serializer\UserSerializer; use Flarum\User\User; +use FoF\BestAnswer\Repository\BestAnswerRepository; class UserBestAnswerCount { diff --git a/src/BestAnswerRepository.php b/src/BestAnswerRepository.php deleted file mode 100644 index 00d4bab..0000000 --- a/src/BestAnswerRepository.php +++ /dev/null @@ -1,103 +0,0 @@ -settings = $settings; - } - - public function canSelectBestAnswer(User $user, Discussion $discussion): bool - { - // Prevent best answers being set in a private discussion (ie byobu, etc) - if ($discussion->is_private) { - return false; - } - - return self::tagEnabledForBestAnswer($discussion) && ($user->id === $discussion->user_id - ? $user->can('selectBestAnswerOwnDiscussion', $discussion) - : $user->can('selectBestAnswerNotOwnDiscussion', $discussion)); - } - - public function canSelectPostAsBestAnswer(User $user, Post $post): bool - { - if (!self::canSelectBestAnswer($user, $post->discussion)) { - return false; - } - - if ($user->id === $post->user_id) { - return (bool) $this->settings->get('fof-best-answer.allow_select_own_post'); - } - - return true; - } - - public function canRemoveBestAnswer(User $user, Discussion $discussion): bool - { - return self::canSelectBestAnswer($user, $discussion); - } - - public function tagEnabledForBestAnswer(Discussion $discussion): bool - { - $enabled = false; - - /** @phpstan-ignore-next-line */ - $discussionTags = $discussion->tags; - foreach ($discussionTags as $discussionTag) { - if ((bool) $discussionTag->is_qna) { - $enabled = true; - break; - } - } - - 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 index ff2ef56..1d7b470 100644 --- a/src/Console/UpdateBestAnswerCounts.php +++ b/src/Console/UpdateBestAnswerCounts.php @@ -12,7 +12,7 @@ namespace FoF\BestAnswer\Console; use Flarum\User\User; -use FoF\BestAnswer\BestAnswerRepository; +use FoF\BestAnswer\Repository\BestAnswerRepository; use Illuminate\Console\Command; class UpdateBestAnswerCounts extends Command diff --git a/src/Listeners/SaveBestAnswerToDatabase.php b/src/Listeners/SaveBestAnswerToDatabase.php index 4c43d3e..de10042 100644 --- a/src/Listeners/SaveBestAnswerToDatabase.php +++ b/src/Listeners/SaveBestAnswerToDatabase.php @@ -11,24 +11,11 @@ namespace FoF\BestAnswer\Listeners; -use Carbon\Carbon; -use Flarum\Discussion\Discussion; use Flarum\Discussion\Event\Saving; -use Flarum\Foundation\ValidationException; -use Flarum\Notification\Notification; use Flarum\Notification\NotificationSyncer; -use Flarum\Post\Post; -use Flarum\Settings\SettingsRepositoryInterface; -use Flarum\Tags\Tag; -use Flarum\User\Exception\PermissionDeniedException; -use Flarum\User\User; -use FoF\BestAnswer\BestAnswerRepository; -use FoF\BestAnswer\Events\BestAnswerSet; -use FoF\BestAnswer\Events\BestAnswerUnset; +use FoF\BestAnswer\Repository\BestAnswerRepository; use FoF\BestAnswer\Notification\SelectBestAnswerBlueprint; -use Illuminate\Events\Dispatcher; use Illuminate\Support\Arr; -use Symfony\Contracts\Translation\TranslatorInterface; class SaveBestAnswerToDatabase { @@ -39,33 +26,15 @@ class SaveBestAnswerToDatabase */ private $notifications; - /** - * @var Dispatcher - */ - private $bus; - - /** - * @var TranslatorInterface - */ - private $translator; - - /** - * @var SettingsRepositoryInterface - */ - private $settings; - /** * @var BestAnswerRepository */ protected $bestAnswer; - public function __construct(NotificationSyncer $notifications, Dispatcher $bus, TranslatorInterface $translator, BestAnswerRepository $bestAnswer, SettingsRepositoryInterface $settings) + public function __construct(NotificationSyncer $notifications, BestAnswerRepository $bestAnswer) { $this->notifications = $notifications; - $this->bus = $bus; - $this->translator = $translator; $this->bestAnswer = $bestAnswer; - $this->settings = $settings; } public function handle(Saving $event) @@ -86,94 +55,8 @@ public function handle(Saving $event) // If 'id' = 0, then we are removing a best answer. $function = $id === 0 ? 'removeBestAnswer' : 'setBestAnswer'; - $this->$function($discussion, $actor, $id); + $this->bestAnswer->$function($discussion, $actor, $id); $this->notifications->delete(new SelectBestAnswerBlueprint($discussion)); } - - protected function removeBestAnswer(Discussion $discussion, User $actor): void - { - if (!$this->bestAnswer->canRemoveBestAnswer($actor, $discussion)) { - throw new PermissionDeniedException(); - } - - /** @var Post|null $post */ - $post = $discussion->bestAnswerPost; - - if (!$post) { - return; - } - - $discussion->best_answer_post_id = null; - $discussion->best_answer_user_id = null; - $discussion->best_answer_set_at = null; - $discussion->unsetRelation('bestAnswerPost'); - $discussion->unsetRelation('bestAnswerUser'); - - $this->changeTags($discussion, 'detach'); - - $discussion->afterSave(function ($discussion) use ($actor, $post) { - $this->bus->dispatch(new BestAnswerUnset($discussion, $post, $actor)); - }); - } - - protected function setBestAnswer(Discussion $discussion, User $actor, int $id): void - { - /** @var Post|null $post */ - $post = $discussion->posts()->find($id); - - if ($id && !$post) { - throw new ValidationException( - [ - 'error' => $this->translator->trans('fof-best-answer.forum.errors.mismatch'), - ] - ); - } - - if ($post && (!$this->bestAnswer->canSelectPostAsBestAnswer($actor, $post) || !$post->isVisibleTo($actor))) { - throw new PermissionDeniedException(); - } - - if ($id) { - $discussion->best_answer_post_id = $post->id; - $discussion->best_answer_user_id = $actor->id; - $discussion->best_answer_set_at = Carbon::now(); - - Notification::where('type', 'selectBestAnswer')->where('subject_id', $discussion->id)->delete(); - - $this->changeTags($discussion, 'attach'); - - $discussion->afterSave(function (Discussion $discussion) use ($actor) { - $post = $discussion->bestAnswerPost; - $this->bus->dispatch(new BestAnswerSet($discussion, $post, $actor)); - }); - } - } - - protected function changeTags(Discussion $discussion, string $method) - { - $tagsToChange = @json_decode($this->settings->get('fof-best-answer.select_best_answer_tags')); - - if (empty($tagsToChange)) { - return; - } - - $validTags = Tag::query()->whereIn('id', $tagsToChange); - - // Query errors if we try to attach tags that are already attached due to the unique constraint - if ($method === 'attach') { - /** @phpstan-ignore-next-line */ - $existingTags = $discussion->tags()->pluck('id'); - $validTags = $validTags->whereNotIn('id', $existingTags); - } - - $validTagsIds = $validTags->pluck('id'); - - if ($validTagsIds->isEmpty()) { - return; - } - - /** @phpstan-ignore-next-line */ - $discussion->tags()->$method($validTagsIds); - } } diff --git a/src/Providers/BestAnswerServiceProvider.php b/src/Providers/BestAnswerServiceProvider.php new file mode 100644 index 0000000..4480b9f --- /dev/null +++ b/src/Providers/BestAnswerServiceProvider.php @@ -0,0 +1,14 @@ +container->bind(BestAnswerRepository::class); + } +} diff --git a/src/Repository/BestAnswerRepository.php b/src/Repository/BestAnswerRepository.php new file mode 100644 index 0000000..2b6d385 --- /dev/null +++ b/src/Repository/BestAnswerRepository.php @@ -0,0 +1,213 @@ +settings = $settings; + $this->events = $events; + $this->translator = $translator; + } + + public function canSelectBestAnswer(User $user, Discussion $discussion): bool + { + // Prevent best answers being set in a private discussion (ie byobu, etc) + if ($discussion->is_private) { + return false; + } + + return self::tagEnabledForBestAnswer($discussion) && ($user->id === $discussion->user_id + ? $user->can('selectBestAnswerOwnDiscussion', $discussion) + : $user->can('selectBestAnswerNotOwnDiscussion', $discussion)); + } + + public function canSelectPostAsBestAnswer(User $user, Post $post): bool + { + if (!self::canSelectBestAnswer($user, $post->discussion)) { + return false; + } + + if ($user->id === $post->user_id) { + return (bool) $this->settings->get('fof-best-answer.allow_select_own_post'); + } + + return true; + } + + public function canRemoveBestAnswer(User $user, Discussion $discussion): bool + { + return self::canSelectBestAnswer($user, $discussion); + } + + public function tagEnabledForBestAnswer(Discussion $discussion): bool + { + $enabled = false; + + /** @phpstan-ignore-next-line */ + $discussionTags = $discussion->tags; + foreach ($discussionTags as $discussionTag) { + if ((bool) $discussionTag->is_qna) { + $enabled = true; + break; + } + } + + 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; + } + + protected function removeBestAnswer(Discussion $discussion, User $actor): void + { + if (!$this->canRemoveBestAnswer($actor, $discussion)) { + throw new PermissionDeniedException(); + } + + /** @var Post|null $post */ + $post = $discussion->bestAnswerPost; + + if (!$post) { + return; + } + + $discussion->best_answer_post_id = null; + $discussion->best_answer_user_id = null; + $discussion->best_answer_set_at = null; + $discussion->unsetRelation('bestAnswerPost'); + $discussion->unsetRelation('bestAnswerUser'); + + $this->changeTags($discussion, 'detach'); + + $discussion->afterSave(function ($discussion) use ($actor, $post) { + $this->events->dispatch(new BestAnswerUnset($discussion, $post, $actor)); + }); + } + + protected function setBestAnswer(Discussion $discussion, User $actor, int $id): void + { + /** @var Post|null $post */ + $post = $discussion->posts()->find($id); + + if ($id && !$post) { + throw new ValidationException( + [ + 'error' => $this->translator->trans('fof-best-answer.forum.errors.mismatch'), + ] + ); + } + + if ($post && (!$this->canSelectPostAsBestAnswer($actor, $post) || !$post->isVisibleTo($actor))) { + throw new PermissionDeniedException(); + } + + if ($id) { + $discussion->best_answer_post_id = $post->id; + $discussion->best_answer_user_id = $actor->id; + $discussion->best_answer_set_at = Carbon::now(); + + Notification::where('type', 'selectBestAnswer')->where('subject_id', $discussion->id)->delete(); + + $this->changeTags($discussion, 'attach'); + + $discussion->afterSave(function (Discussion $discussion) use ($actor) { + $post = $discussion->bestAnswerPost; + $this->events->dispatch(new BestAnswerSet($discussion, $post, $actor)); + }); + } + } + + protected function changeTags(Discussion $discussion, string $method) + { + $tagsToChange = @json_decode($this->settings->get('fof-best-answer.select_best_answer_tags')); + + if (empty($tagsToChange)) { + return; + } + + $validTags = Tag::query()->whereIn('id', $tagsToChange); + + // Query errors if we try to attach tags that are already attached due to the unique constraint + if ($method === 'attach') { + /** @phpstan-ignore-next-line */ + $existingTags = $discussion->tags()->pluck('id'); + $validTags = $validTags->whereNotIn('id', $existingTags); + } + + $validTagsIds = $validTags->pluck('id'); + + if ($validTagsIds->isEmpty()) { + return; + } + + /** @phpstan-ignore-next-line */ + $discussion->tags()->$method($validTagsIds); + } +} From 553fc81bf722036b03d3c24eeb11a75bd9a23345 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 11 Nov 2024 20:22:07 +0000 Subject: [PATCH 02/12] Apply fixes from StyleCI --- src/Api/AddTagAttributes.php | 9 +++++++++ src/Listeners/SaveBestAnswerToDatabase.php | 2 +- src/Providers/BestAnswerServiceProvider.php | 9 +++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Api/AddTagAttributes.php b/src/Api/AddTagAttributes.php index 3a9fdd1..7e94e11 100644 --- a/src/Api/AddTagAttributes.php +++ b/src/Api/AddTagAttributes.php @@ -1,5 +1,14 @@ Date: Mon, 11 Nov 2024 20:25:09 +0000 Subject: [PATCH 03/12] chore: update namespace --- tests/unit/SaveBestAnswerToDatabaseTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index 75cd08f..4af0819 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -17,7 +17,7 @@ use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Testing\unit\TestCase; use Flarum\User\User; -use FoF\BestAnswer\BestAnswerRepository; +use FoF\BestAnswer\Repository\BestAnswerRepository; use FoF\BestAnswer\Listeners\SaveBestAnswerToDatabase; use FoF\BestAnswer\Notification\SelectBestAnswerBlueprint; use Illuminate\Events\Dispatcher; From 1d095bf34fb24c03bc7349d9a1abbcf071dc2dcc Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 11 Nov 2024 20:25:17 +0000 Subject: [PATCH 04/12] Apply fixes from StyleCI --- tests/unit/SaveBestAnswerToDatabaseTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index 4af0819..9421468 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -17,9 +17,9 @@ use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Testing\unit\TestCase; use Flarum\User\User; -use FoF\BestAnswer\Repository\BestAnswerRepository; use FoF\BestAnswer\Listeners\SaveBestAnswerToDatabase; use FoF\BestAnswer\Notification\SelectBestAnswerBlueprint; +use FoF\BestAnswer\Repository\BestAnswerRepository; use Illuminate\Events\Dispatcher; use Mockery as m; use Symfony\Contracts\Translation\TranslatorInterface; From 62924fb13cc81e269ae50dd3bb92ab93a89aeeaf Mon Sep 17 00:00:00 2001 From: IanM Date: Mon, 11 Nov 2024 20:28:53 +0000 Subject: [PATCH 05/12] update signature --- tests/unit/SaveBestAnswerToDatabaseTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index 9421468..bf4326e 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -46,10 +46,7 @@ public function testHandle_WhenKeyIsMissing_ReturnsWithoutAction() $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'); @@ -70,10 +67,7 @@ public function testHandle_DiscussionDoesNotExistOrMatches_ReturnsWithoutAction( $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'); @@ -98,9 +92,7 @@ public function testHandle_IdIsZero_CallsRemoveBestAnswer() $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(); @@ -124,9 +116,7 @@ public function testHandle_IdIsNotZero_CallsSetBestAnswer() $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(); From 597bc4adb9f63422986ceb02aec391b9d54fdce9 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 11 Nov 2024 20:29:03 +0000 Subject: [PATCH 06/12] Apply fixes from StyleCI --- tests/unit/SaveBestAnswerToDatabaseTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index bf4326e..0d8e00f 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -14,7 +14,6 @@ use Flarum\Discussion\Discussion; use Flarum\Discussion\Event\Saving; use Flarum\Notification\NotificationSyncer; -use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Testing\unit\TestCase; use Flarum\User\User; use FoF\BestAnswer\Listeners\SaveBestAnswerToDatabase; @@ -22,7 +21,6 @@ use FoF\BestAnswer\Repository\BestAnswerRepository; use Illuminate\Events\Dispatcher; use Mockery as m; -use Symfony\Contracts\Translation\TranslatorInterface; class SaveBestAnswerToDatabaseTest extends TestCase { From 02a3f7c4aa5efeeb72b78978531fa674151bf2b9 Mon Sep 17 00:00:00 2001 From: IanM Date: Mon, 11 Nov 2024 20:34:47 +0000 Subject: [PATCH 07/12] fix unit tests --- tests/unit/SaveBestAnswerToDatabaseTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index 0d8e00f..dbd04a0 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -89,7 +89,6 @@ public function testHandle_IdIsZero_CallsRemoveBestAnswer() $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[removeBestAnswer]', [ $notifications, - m::mock(Dispatcher::class), m::mock(BestAnswerRepository::class), ])->shouldAllowMockingProtectedMethods(); @@ -113,7 +112,6 @@ public function testHandle_IdIsNotZero_CallsSetBestAnswer() $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[setBestAnswer]', [ $notifications, - m::mock(Dispatcher::class), m::mock(BestAnswerRepository::class), ])->shouldAllowMockingProtectedMethods(); From f4a8a3d83e2c0b6fcc6fa0155d53b76552b05302 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 11 Nov 2024 20:34:56 +0000 Subject: [PATCH 08/12] Apply fixes from StyleCI --- tests/unit/SaveBestAnswerToDatabaseTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index dbd04a0..badc363 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -19,7 +19,6 @@ use FoF\BestAnswer\Listeners\SaveBestAnswerToDatabase; use FoF\BestAnswer\Notification\SelectBestAnswerBlueprint; use FoF\BestAnswer\Repository\BestAnswerRepository; -use Illuminate\Events\Dispatcher; use Mockery as m; class SaveBestAnswerToDatabaseTest extends TestCase From 28b240609b146c93f86f18b8d46200388621e475 Mon Sep 17 00:00:00 2001 From: IanM Date: Tue, 12 Nov 2024 08:58:32 +0000 Subject: [PATCH 09/12] chore: fix unit test --- src/Repository/BestAnswerRepository.php | 4 +- tests/unit/SaveBestAnswerToDatabaseTest.php | 54 ++++++++++----------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/Repository/BestAnswerRepository.php b/src/Repository/BestAnswerRepository.php index 2b6d385..1195e34 100644 --- a/src/Repository/BestAnswerRepository.php +++ b/src/Repository/BestAnswerRepository.php @@ -125,7 +125,7 @@ public function calculateBestAnswersForUser(User $user): int return $count; } - protected function removeBestAnswer(Discussion $discussion, User $actor): void + public function removeBestAnswer(Discussion $discussion, User $actor): void { if (!$this->canRemoveBestAnswer($actor, $discussion)) { throw new PermissionDeniedException(); @@ -151,7 +151,7 @@ protected function removeBestAnswer(Discussion $discussion, User $actor): void }); } - protected function setBestAnswer(Discussion $discussion, User $actor, int $id): void + public function setBestAnswer(Discussion $discussion, User $actor, int $id): void { /** @var Post|null $post */ $post = $discussion->posts()->find($id); diff --git a/tests/unit/SaveBestAnswerToDatabaseTest.php b/tests/unit/SaveBestAnswerToDatabaseTest.php index badc363..7ead57b 100644 --- a/tests/unit/SaveBestAnswerToDatabaseTest.php +++ b/tests/unit/SaveBestAnswerToDatabaseTest.php @@ -24,7 +24,7 @@ class SaveBestAnswerToDatabaseTest extends TestCase { /** - * @var m\MockInterface|SaveBestAnswerToDatabase + * @var SaveBestAnswerToDatabase */ protected $sut; // System Under Test @@ -34,22 +34,20 @@ public function tearDown(): void parent::tearDown(); } - // testing the `handle()` method - public function testHandle_WhenKeyIsMissing_ReturnsWithoutAction() { $event = m::mock(Saving::class); $event->data = []; - $this->sut = m::mock(SaveBestAnswerToDatabase::class.'[removeBestAnswer,setBestAnswer]', [ - m::mock(NotificationSyncer::class), - m::mock(BestAnswerRepository::class), - ])->shouldAllowMockingProtectedMethods(); + $notifications = m::mock(NotificationSyncer::class); + $repository = m::mock(BestAnswerRepository::class); + + $this->sut = new SaveBestAnswerToDatabase($notifications, $repository); - $this->sut->shouldNotReceive('removeBestAnswer'); - $this->sut->shouldNotReceive('setBestAnswer'); + $result = $this->sut->handle($event); - $this->sut->handle($event); + // Assert that no actions were taken + $this->assertNull($result); } public function testHandle_DiscussionDoesNotExistOrMatches_ReturnsWithoutAction() @@ -62,18 +60,18 @@ public function testHandle_DiscussionDoesNotExistOrMatches_ReturnsWithoutAction( $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(BestAnswerRepository::class), - ])->shouldAllowMockingProtectedMethods(); + $notifications = m::mock(NotificationSyncer::class); + $repository = m::mock(BestAnswerRepository::class); - $this->sut->shouldNotReceive('removeBestAnswer'); - $this->sut->shouldNotReceive('setBestAnswer'); + $this->sut = new SaveBestAnswerToDatabase($notifications, $repository); - $this->sut->handle($event); + $result = $this->sut->handle($event); + + // Assert that no actions were taken + $this->assertNull($result); } - public function testHandle_IdIsZero_CallsRemoveBestAnswer() + public function testHandle_IdIsZero_CallsHandleRemoveBestAnswer() { $event = m::mock(Saving::class); $event->data = ['attributes.bestAnswerPostId' => 0]; @@ -86,17 +84,16 @@ 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]', [ - $notifications, - m::mock(BestAnswerRepository::class), - ])->shouldAllowMockingProtectedMethods(); + $repository = m::mock(BestAnswerRepository::class); + $repository->shouldReceive('canRemoveBestAnswer')->with($event->actor, $event->discussion)->andReturn(true); + $repository->shouldReceive('removeBestAnswer')->with($event->discussion, $event->actor, 0)->once(); - $this->sut->shouldReceive('removeBestAnswer')->once(); + $this->sut = new SaveBestAnswerToDatabase($notifications, $repository); $this->sut->handle($event); } - public function testHandle_IdIsNotZero_CallsSetBestAnswer() + public function testHandle_IdIsNotZero_CallsHandleSetBestAnswer() { $event = m::mock(Saving::class); $event->data = ['attributes.bestAnswerPostId' => 2]; @@ -109,12 +106,11 @@ 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]', [ - $notifications, - m::mock(BestAnswerRepository::class), - ])->shouldAllowMockingProtectedMethods(); + $repository = m::mock(BestAnswerRepository::class); + $repository->shouldReceive('canSelectPostAsBestAnswer')->with($event->actor, m::type(Discussion::class))->andReturn(true); + $repository->shouldReceive('setBestAnswer')->with($event->discussion, $event->actor, 2)->once(); - $this->sut->shouldReceive('setBestAnswer')->once(); + $this->sut = new SaveBestAnswerToDatabase($notifications, $repository); $this->sut->handle($event); } From 8c17d560925c93d9389513f52dbce9ffa8953fd0 Mon Sep 17 00:00:00 2001 From: IanM Date: Tue, 12 Nov 2024 20:15:05 +0000 Subject: [PATCH 10/12] fix: typing --- js/src/forum/extend.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/forum/extend.ts b/js/src/forum/extend.ts index 27b65a2..b6fb378 100644 --- a/js/src/forum/extend.ts +++ b/js/src/forum/extend.ts @@ -11,7 +11,7 @@ export default [ new Extend.Model(Discussion) // .hasOne('bestAnswerPost') .hasOne('bestAnswerUser') - .attribute('hasBestAnswer') + .attribute('hasBestAnswer') .attribute('canSelectBestAnswer') .attribute('bestAnswerSetAt', Model.transformDate), From 715508e5bc60da67f10176d0aae02280a86c2c34 Mon Sep 17 00:00:00 2001 From: IanM Date: Tue, 12 Nov 2024 20:15:24 +0000 Subject: [PATCH 11/12] allow replacement --- src/Repository/BestAnswerRepository.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Repository/BestAnswerRepository.php b/src/Repository/BestAnswerRepository.php index 1195e34..348fb6c 100644 --- a/src/Repository/BestAnswerRepository.php +++ b/src/Repository/BestAnswerRepository.php @@ -59,14 +59,14 @@ public function canSelectBestAnswer(User $user, Discussion $discussion): bool return false; } - return self::tagEnabledForBestAnswer($discussion) && ($user->id === $discussion->user_id + return $this->tagEnabledForBestAnswer($discussion) && ($user->id === $discussion->user_id ? $user->can('selectBestAnswerOwnDiscussion', $discussion) : $user->can('selectBestAnswerNotOwnDiscussion', $discussion)); } public function canSelectPostAsBestAnswer(User $user, Post $post): bool { - if (!self::canSelectBestAnswer($user, $post->discussion)) { + if (!$this->canSelectBestAnswer($user, $post->discussion)) { return false; } @@ -79,7 +79,7 @@ public function canSelectPostAsBestAnswer(User $user, Post $post): bool public function canRemoveBestAnswer(User $user, Discussion $discussion): bool { - return self::canSelectBestAnswer($user, $discussion); + return $this->canSelectBestAnswer($user, $discussion); } public function tagEnabledForBestAnswer(Discussion $discussion): bool @@ -184,7 +184,7 @@ public function setBestAnswer(Discussion $discussion, User $actor, int $id): voi } } - protected function changeTags(Discussion $discussion, string $method) + public function changeTags(Discussion $discussion, string $method) { $tagsToChange = @json_decode($this->settings->get('fof-best-answer.select_best_answer_tags')); From 3403b99af2c31535d98158b049bb8600893e1c72 Mon Sep 17 00:00:00 2001 From: IanM Date: Tue, 12 Nov 2024 20:17:20 +0000 Subject: [PATCH 12/12] chore: format --- js/src/forum/extend.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/forum/extend.ts b/js/src/forum/extend.ts index b6fb378..38c5cd3 100644 --- a/js/src/forum/extend.ts +++ b/js/src/forum/extend.ts @@ -11,7 +11,7 @@ export default [ new Extend.Model(Discussion) // .hasOne('bestAnswerPost') .hasOne('bestAnswerUser') - .attribute('hasBestAnswer') + .attribute('hasBestAnswer') .attribute('canSelectBestAnswer') .attribute('bestAnswerSetAt', Model.transformDate),