From 785459657939ef4c87d8fa8cfa6bcc0a095e29fd Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Thu, 21 May 2026 19:27:19 -0300 Subject: [PATCH] refactor(posts): centralize post editing status checks with PostStatusGuard - Replaced direct status checks in multiple controllers and actions with the PostStatusGuard utility, improving code readability and maintainability. - Updated error messages to utilize a centralized method for consistency across the application. - Removed the BrandImagePalette class, consolidating color resolution logic into the AiImageClient for better organization and type safety. --- app/Actions/Post/PostStatusGuard.php | 51 +++++++++++++++++++ app/Actions/Post/UpdatePost.php | 4 +- app/Http/Controllers/Api/PostController.php | 3 +- .../App/PostAiGenerateController.php | 4 +- .../App/PostAiRegenerateMediaController.php | 13 ++--- .../App/PostAiReviewController.php | 4 +- app/Http/Controllers/App/PostController.php | 5 +- app/Mcp/Tools/Post/PublishPostTool.php | 3 +- app/Mcp/Tools/Post/UpdatePostTool.php | 3 +- app/Services/Ai/AiImageClient.php | 50 ++++++++++++++---- app/Support/BrandImagePalette.php | 45 ---------------- tests/Feature/Ai/PostAiGenerateTest.php | 10 ++-- tests/Feature/Ai/PostAiReviewTest.php | 10 ++-- tests/Feature/Mcp/PostPublishToolTest.php | 4 +- .../Unit/Actions/Post/PostStatusGuardTest.php | 47 +++++++++++++++++ tests/Unit/Support/BrandImagePaletteTest.php | 37 -------------- 16 files changed, 165 insertions(+), 128 deletions(-) create mode 100644 app/Actions/Post/PostStatusGuard.php delete mode 100644 app/Support/BrandImagePalette.php create mode 100644 tests/Unit/Actions/Post/PostStatusGuardTest.php delete mode 100644 tests/Unit/Support/BrandImagePaletteTest.php diff --git a/app/Actions/Post/PostStatusGuard.php b/app/Actions/Post/PostStatusGuard.php new file mode 100644 index 00000000..27fbd43f --- /dev/null +++ b/app/Actions/Post/PostStatusGuard.php @@ -0,0 +1,51 @@ + + */ + private const EDIT_BLOCKED_STATUSES = [ + PostStatus::Published, + PostStatus::PartiallyPublished, + PostStatus::Failed, + PostStatus::Publishing, + ]; + + /** + * Statuses where the post can no longer be deleted. + * + * @var array + */ + private const DELETE_BLOCKED_STATUSES = [ + PostStatus::Publishing, + PostStatus::Published, + PostStatus::PartiallyPublished, + ]; + + public static function blocksEditing(Post $post): bool + { + return in_array($post->status, self::EDIT_BLOCKED_STATUSES, true); + } + + public static function blocksDeletion(Post $post): bool + { + return in_array($post->status, self::DELETE_BLOCKED_STATUSES, true); + } + + public static function editBlockedMessage(): string + { + return __(self::EDIT_BLOCKED_MESSAGE_KEY); + } +} diff --git a/app/Actions/Post/UpdatePost.php b/app/Actions/Post/UpdatePost.php index e39ca4b2..f4c36e33 100644 --- a/app/Actions/Post/UpdatePost.php +++ b/app/Actions/Post/UpdatePost.php @@ -20,9 +20,7 @@ class UpdatePost */ public static function execute(Workspace $workspace, Post $post, array $data): array { - $terminalStatuses = [PostStatus::Published, PostStatus::PartiallyPublished, PostStatus::Failed, PostStatus::Publishing]; - - if (in_array($post->status, $terminalStatuses, true)) { + if (PostStatusGuard::blocksEditing($post)) { return ['post' => $post, 'action' => PostAction::Finalized]; } diff --git a/app/Http/Controllers/Api/PostController.php b/app/Http/Controllers/Api/PostController.php index bddaa328..2044c921 100644 --- a/app/Http/Controllers/Api/PostController.php +++ b/app/Http/Controllers/Api/PostController.php @@ -6,6 +6,7 @@ use App\Actions\Post\CreatePost; use App\Actions\Post\DeletePost; +use App\Actions\Post\PostStatusGuard; use App\Actions\Post\UpdatePost; use App\Enums\Media\Type as MediaType; use App\Enums\Post\Action as PostAction; @@ -69,7 +70,7 @@ public function update(UpdatePostRequest $request, Post $post): PostResource|Jso if (data_get($result, 'action') === PostAction::Finalized) { return response()->json( - ['message' => 'Cannot edit a post in a terminal state.'], + ['message' => PostStatusGuard::editBlockedMessage()], Response::HTTP_UNPROCESSABLE_ENTITY ); } diff --git a/app/Http/Controllers/App/PostAiGenerateController.php b/app/Http/Controllers/App/PostAiGenerateController.php index 2d0cd9fa..58b7d642 100644 --- a/app/Http/Controllers/App/PostAiGenerateController.php +++ b/app/Http/Controllers/App/PostAiGenerateController.php @@ -18,9 +18,7 @@ public function generate(GeneratePostContentRequest $request, Post $post): JsonR { $workspace = $request->user()->currentWorkspace; - if ($post->workspace_id !== $workspace->id) { - abort(Response::HTTP_FORBIDDEN); - } + $this->authorize('update', $post); $gate = Gate::inspect('useAi', $workspace->account); if ($gate->denied()) { diff --git a/app/Http/Controllers/App/PostAiRegenerateMediaController.php b/app/Http/Controllers/App/PostAiRegenerateMediaController.php index 36745744..9e59d3e4 100644 --- a/app/Http/Controllers/App/PostAiRegenerateMediaController.php +++ b/app/Http/Controllers/App/PostAiRegenerateMediaController.php @@ -4,8 +4,8 @@ namespace App\Http\Controllers\App; +use App\Actions\Post\PostStatusGuard; use App\Enums\Media\Source; -use App\Enums\Post\Status as PostStatus; use App\Http\Requests\App\Ai\RegeneratePostMediaImageRequest; use App\Jobs\Ai\RegeneratePostMediaImage; use App\Models\Post; @@ -22,16 +22,9 @@ public function regenerate(RegeneratePostMediaImageRequest $request, Post $post, $workspace = $request->user()->currentWorkspace; - $terminalStatuses = [ - PostStatus::Published, - PostStatus::PartiallyPublished, - PostStatus::Failed, - PostStatus::Publishing, - ]; - - if (in_array($post->status, $terminalStatuses, true)) { + if (PostStatusGuard::blocksEditing($post)) { return response()->json([ - 'message' => __('posts.cannot_edit_finalized'), + 'message' => PostStatusGuard::editBlockedMessage(), ], Response::HTTP_UNPROCESSABLE_ENTITY); } diff --git a/app/Http/Controllers/App/PostAiReviewController.php b/app/Http/Controllers/App/PostAiReviewController.php index 4c2fb671..8912b6ff 100644 --- a/app/Http/Controllers/App/PostAiReviewController.php +++ b/app/Http/Controllers/App/PostAiReviewController.php @@ -18,9 +18,7 @@ public function review(ReviewPostContentRequest $request, Post $post): JsonRespo { $workspace = $request->user()->currentWorkspace; - if ($post->workspace_id !== $workspace->id) { - abort(Response::HTTP_FORBIDDEN); - } + $this->authorize('update', $post); $gate = Gate::inspect('useAi', $workspace->account); if ($gate->denied()) { diff --git a/app/Http/Controllers/App/PostController.php b/app/Http/Controllers/App/PostController.php index a0a52f2a..ad7baed8 100644 --- a/app/Http/Controllers/App/PostController.php +++ b/app/Http/Controllers/App/PostController.php @@ -7,6 +7,7 @@ use App\Actions\Post\CreatePost; use App\Actions\Post\DeletePost; use App\Actions\Post\DuplicatePost; +use App\Actions\Post\PostStatusGuard; use App\Actions\Post\SyncPostPlatforms; use App\Actions\Post\UpdatePost; use App\Enums\Post\Action as PostAction; @@ -219,7 +220,7 @@ public function edit(Request $request, Post $post): Response|RedirectResponse $this->authorize('update', $post); - if (in_array($post->status, [PostStatus::Publishing, PostStatus::Published, PostStatus::PartiallyPublished, PostStatus::Failed], true)) { + if (PostStatusGuard::blocksEditing($post)) { return redirect()->route('app.posts.show', $post); } @@ -313,7 +314,7 @@ public function destroy(Request $request, Post $post): RedirectResponse $this->authorize('delete', $post); - if (in_array($post->status, [PostStatus::Publishing, PostStatus::Published, PostStatus::PartiallyPublished], true)) { + if (PostStatusGuard::blocksDeletion($post)) { session()->flash('flash.banner', __('posts.flash.cannot_delete_published')); session()->flash('flash.bannerStyle', 'danger'); diff --git a/app/Mcp/Tools/Post/PublishPostTool.php b/app/Mcp/Tools/Post/PublishPostTool.php index 993fb434..1fc6c4d9 100644 --- a/app/Mcp/Tools/Post/PublishPostTool.php +++ b/app/Mcp/Tools/Post/PublishPostTool.php @@ -4,6 +4,7 @@ namespace App\Mcp\Tools\Post; +use App\Actions\Post\PostStatusGuard; use App\Actions\Post\UpdatePost; use App\Enums\Post\Action as PostAction; use App\Enums\Post\Status; @@ -48,7 +49,7 @@ public function handle(Request $request): Response|ResponseFactory ]); if (data_get($result, 'action') === PostAction::Finalized) { - return Response::error('Post is already published or in a terminal state.'); + return Response::error(PostStatusGuard::editBlockedMessage()); } /** @var Post $updated */ diff --git a/app/Mcp/Tools/Post/UpdatePostTool.php b/app/Mcp/Tools/Post/UpdatePostTool.php index d855c1b0..f83b1ff8 100644 --- a/app/Mcp/Tools/Post/UpdatePostTool.php +++ b/app/Mcp/Tools/Post/UpdatePostTool.php @@ -4,6 +4,7 @@ namespace App\Mcp\Tools\Post; +use App\Actions\Post\PostStatusGuard; use App\Actions\Post\UpdatePost; use App\Enums\Post\Action as PostAction; use App\Enums\Post\Status; @@ -55,7 +56,7 @@ public function handle(Request $request): Response|ResponseFactory $result = UpdatePost::execute($workspace, $post, $payload); if (data_get($result, 'action') === PostAction::Finalized) { - return Response::error('Cannot edit a post in a terminal state.'); + return Response::error(PostStatusGuard::editBlockedMessage()); } /** @var Post $updated */ diff --git a/app/Services/Ai/AiImageClient.php b/app/Services/Ai/AiImageClient.php index 11c2b0cb..bd7eb193 100644 --- a/app/Services/Ai/AiImageClient.php +++ b/app/Services/Ai/AiImageClient.php @@ -5,7 +5,7 @@ namespace App\Services\Ai; use App\Enums\Workspace\ImageStyle; -use App\Support\BrandImagePalette; +use App\Support\HexColorName; use Illuminate\Support\Facades\Log; use Laravel\Ai\Image; use Throwable; @@ -39,11 +39,7 @@ public function generate( return null; } - $palette = new BrandImagePalette( - brandColor: $brandColor, - backgroundColor: $backgroundColor, - textColor: $textColor, - ); + $palette = $this->buildPaletteContext($brandColor, $backgroundColor, $textColor); $brandContext = null; if ($brandDescription !== null) { @@ -59,10 +55,10 @@ public function generate( 'style' => $style->value, 'scene' => implode(', ', $clean), 'language_name' => $this->languageName($language), - 'has_brand_palette' => $palette->isDefined(), - 'brand_color_name' => $palette->brandColorName, - 'background_color_name' => $palette->backgroundColorName, - 'text_color_name' => $palette->textColorName, + 'has_brand_palette' => data_get($palette, 'is_defined', false), + 'brand_color_name' => data_get($palette, 'brand_color_name'), + 'background_color_name' => data_get($palette, 'background_color_name'), + 'text_color_name' => data_get($palette, 'text_color_name'), 'brand_context' => $brandContext, ])->render(); @@ -99,4 +95,38 @@ private function languageName(string $code): string default => 'English', }; } + + /** + * @return array{ + * is_defined: bool, + * brand_color_name: ?string, + * background_color_name: ?string, + * text_color_name: ?string + * } + */ + private function buildPaletteContext( + ?string $brandColor, + ?string $backgroundColor, + ?string $textColor, + ): array { + $brandColorName = $this->resolveColorName($brandColor); + $backgroundColorName = $this->resolveColorName($backgroundColor); + $textColorName = $this->resolveColorName($textColor); + + return [ + 'is_defined' => $brandColorName !== null || $backgroundColorName !== null || $textColorName !== null, + 'brand_color_name' => $brandColorName, + 'background_color_name' => $backgroundColorName, + 'text_color_name' => $textColorName, + ]; + } + + private function resolveColorName(?string $hex): ?string + { + if ($hex === null || trim($hex) === '') { + return null; + } + + return HexColorName::approximate($hex); + } } diff --git a/app/Support/BrandImagePalette.php b/app/Support/BrandImagePalette.php deleted file mode 100644 index 9dc2db2f..00000000 --- a/app/Support/BrandImagePalette.php +++ /dev/null @@ -1,45 +0,0 @@ -brandColorName = self::resolveName($brandColor); - $this->backgroundColorName = self::resolveName($backgroundColor); - $this->textColorName = self::resolveName($textColor); - } - - public function isDefined(): bool - { - return $this->brandColorName !== null - || $this->backgroundColorName !== null - || $this->textColorName !== null; - } - - private static function resolveName(?string $hex): ?string - { - if ($hex === null || trim($hex) === '') { - return null; - } - - return HexColorName::approximate($hex); - } -} diff --git a/tests/Feature/Ai/PostAiGenerateTest.php b/tests/Feature/Ai/PostAiGenerateTest.php index 727d1afe..ae775dfc 100644 --- a/tests/Feature/Ai/PostAiGenerateTest.php +++ b/tests/Feature/Ai/PostAiGenerateTest.php @@ -22,14 +22,14 @@ }); test('endpoint requires authentication', function () { - $this->postJson("/posts/{$this->post->id}/ai/generate", ['prompt' => 'hi']) + $this->postJson(route('app.posts.ai.generate', $this->post), ['prompt' => 'hi']) ->assertStatus(Response::HTTP_UNAUTHORIZED); }); test('endpoint validates prompt is required', function () { Bus::fake(); $this->actingAs($this->user) - ->postJson("/posts/{$this->post->id}/ai/generate", []) + ->postJson(route('app.posts.ai.generate', $this->post), []) ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) ->assertJsonValidationErrors(['prompt']); }); @@ -40,15 +40,15 @@ $foreignPost = Post::factory()->create(['workspace_id' => $otherWorkspace->id]); $this->actingAs($this->user) - ->postJson("/posts/{$foreignPost->id}/ai/generate", ['prompt' => 'hi']) - ->assertStatus(Response::HTTP_FORBIDDEN); + ->postJson(route('app.posts.ai.generate', $foreignPost), ['prompt' => 'hi']) + ->assertStatus(Response::HTTP_NOT_FOUND); }); test('endpoint dispatches StreamPostContent and returns generation id and channel', function () { Bus::fake(); $response = $this->actingAs($this->user) - ->postJson("/posts/{$this->post->id}/ai/generate", [ + ->postJson(route('app.posts.ai.generate', $this->post), [ 'prompt' => 'Write a post about Mondays', 'current_content' => 'Old content', ]) diff --git a/tests/Feature/Ai/PostAiReviewTest.php b/tests/Feature/Ai/PostAiReviewTest.php index 4b3cca22..084cdccc 100644 --- a/tests/Feature/Ai/PostAiReviewTest.php +++ b/tests/Feature/Ai/PostAiReviewTest.php @@ -21,13 +21,13 @@ }); test('endpoint requires authentication', function () { - $this->postJson("/posts/{$this->post->id}/ai/review", ['content' => 'hi']) + $this->postJson(route('app.posts.ai.review', $this->post), ['content' => 'hi']) ->assertStatus(Response::HTTP_UNAUTHORIZED); }); test('endpoint validates content required', function () { $this->actingAs($this->user) - ->postJson("/posts/{$this->post->id}/ai/review", []) + ->postJson(route('app.posts.ai.review', $this->post), []) ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) ->assertJsonValidationErrors(['content']); }); @@ -37,8 +37,8 @@ $foreignPost = Post::factory()->create(['workspace_id' => $otherWorkspace->id]); $this->actingAs($this->user) - ->postJson("/posts/{$foreignPost->id}/ai/review", ['content' => 'hi']) - ->assertStatus(Response::HTTP_FORBIDDEN); + ->postJson(route('app.posts.ai.review', $foreignPost), ['content' => 'hi']) + ->assertStatus(Response::HTTP_NOT_FOUND); }); test('endpoint returns suggestions array', function () { @@ -49,7 +49,7 @@ ]); $response = $this->actingAs($this->user) - ->postJson("/posts/{$this->post->id}/ai/review", ['content' => 'i was here']) + ->postJson(route('app.posts.ai.review', $this->post), ['content' => 'i was here']) ->assertOk(); expect($response->json('suggestions'))->toBeArray()->toHaveCount(1); diff --git a/tests/Feature/Mcp/PostPublishToolTest.php b/tests/Feature/Mcp/PostPublishToolTest.php index 9fbbe0b1..0c22c1d1 100644 --- a/tests/Feature/Mcp/PostPublishToolTest.php +++ b/tests/Feature/Mcp/PostPublishToolTest.php @@ -116,7 +116,7 @@ $response = TryPostServer::actingAs($this->user) ->tool(UpdatePostTool::class, ['post_id' => $post->id, 'content' => 'x']); - $response->assertHasErrors(['Cannot edit a post in a terminal state.']); + $response->assertHasErrors([__('posts.cannot_edit_finalized')]); })->with([ PostStatus::Published, PostStatus::PartiallyPublished, @@ -277,7 +277,7 @@ $response = TryPostServer::actingAs($this->user) ->tool(PublishPostTool::class, ['post_id' => $post->id]); - $response->assertHasErrors(['Post is already published or in a terminal state.']); + $response->assertHasErrors([__('posts.cannot_edit_finalized')]); })->with([ PostStatus::Published, PostStatus::PartiallyPublished, diff --git a/tests/Unit/Actions/Post/PostStatusGuardTest.php b/tests/Unit/Actions/Post/PostStatusGuardTest.php new file mode 100644 index 00000000..36ca42f4 --- /dev/null +++ b/tests/Unit/Actions/Post/PostStatusGuardTest.php @@ -0,0 +1,47 @@ +make(['status' => $status]); + + expect(PostStatusGuard::blocksEditing($post))->toBeTrue(); +})->with([ + PostStatus::Publishing, + PostStatus::Published, + PostStatus::PartiallyPublished, + PostStatus::Failed, +]); + +test('allows editing for non terminal statuses', function (PostStatus $status) { + $post = Post::factory()->make(['status' => $status]); + + expect(PostStatusGuard::blocksEditing($post))->toBeFalse(); +})->with([ + PostStatus::Draft, + PostStatus::Scheduled, +]); + +test('blocks deletion for published statuses', function (PostStatus $status) { + $post = Post::factory()->make(['status' => $status]); + + expect(PostStatusGuard::blocksDeletion($post))->toBeTrue(); +})->with([ + PostStatus::Publishing, + PostStatus::Published, + PostStatus::PartiallyPublished, +]); + +test('allows deletion for draft, scheduled and failed statuses', function (PostStatus $status) { + $post = Post::factory()->make(['status' => $status]); + + expect(PostStatusGuard::blocksDeletion($post))->toBeFalse(); +})->with([ + PostStatus::Draft, + PostStatus::Scheduled, + PostStatus::Failed, +]); diff --git a/tests/Unit/Support/BrandImagePaletteTest.php b/tests/Unit/Support/BrandImagePaletteTest.php deleted file mode 100644 index 3d84e4bc..00000000 --- a/tests/Unit/Support/BrandImagePaletteTest.php +++ /dev/null @@ -1,37 +0,0 @@ -isDefined())->toBeFalse(); -}); - -test('resolves all three workspace colours to approximate names', function () { - $palette = new BrandImagePalette( - brandColor: '#facc15', - backgroundColor: '#ffffff', - textColor: '#0f172a', - ); - - expect($palette->isDefined())->toBeTrue() - ->and($palette->brandColorName)->toBe('golden yellow') - ->and($palette->backgroundColorName)->toBe('off-white') - ->and($palette->textColorName)->toContain('blue'); -}); - -test('ignores malformed hex values', function () { - $palette = new BrandImagePalette( - brandColor: 'not-a-hex', - backgroundColor: '#ffffff', - textColor: '', - ); - - expect($palette->brandColorName)->toBeNull() - ->and($palette->backgroundColorName)->toBe('off-white') - ->and($palette->textColorName)->toBeNull() - ->and($palette->isDefined())->toBeTrue(); -});