From 933351f7aff3d3d779741d24fe04929fbd793415 Mon Sep 17 00:00:00 2001 From: Oliver <20591763+Cryptoom@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:39:39 +0300 Subject: [PATCH] fix(posts): per-platform media validation instead of shared list ContentTypeCompatibleWithMedia::entriesForUpdate() now resolves each platform entry's own effective media list (request media if resubmitted, else PostPlatform::scopedMediaItems(), else the post's full stored media) instead of validating every platform against one shared list. errorsFor() consumes each entry's own media accordingly. Wires A1's scopedMediaItems() helper into validation for the first time. A content_type that requires media whose scoped selection resolves to none still correctly fails, deliberately not softened. Web UpdatePostRequest now runs the check via an after() validator (like the API request already did) instead of a field-level Rule bound to a single shared $data['media'], closing the same "publish without resubmitting content_type" gap the API path already covers. Co-Authored-By: Claude Sonnet 5 --- .../Requests/Api/Post/UpdatePostRequest.php | 19 +- .../Requests/App/Post/UpdatePostRequest.php | 35 +++- app/Mcp/Tools/Post/UpdatePostTool.php | 1 - app/Rules/ContentTypeCompatibleWithMedia.php | 110 ++++++++--- .../ContentTypeCompatibleWithMediaTest.php | 174 ++++++++++++++++++ 5 files changed, 302 insertions(+), 37 deletions(-) diff --git a/app/Http/Requests/Api/Post/UpdatePostRequest.php b/app/Http/Requests/Api/Post/UpdatePostRequest.php index 6e80f6cf0..ce54d6f43 100644 --- a/app/Http/Requests/Api/Post/UpdatePostRequest.php +++ b/app/Http/Requests/Api/Post/UpdatePostRequest.php @@ -109,25 +109,28 @@ public function withValidator(Validator $validator): void /** * On publish/schedule, validate every platform's *effective* content_type - * (resubmitted in this request, or its stored value) against the *effective* - * media (the request's media when sent, otherwise the post's stored media). - * This closes the gap where a client publishes a misconfigured post — e.g. a - * PDF on a regular LinkedIn post — without resubmitting content_type, which a - * field-level rule on `platforms.*.content_type` would skip. + * (resubmitted in this request, or its stored value) against its own + * *effective* media: the request's media when resubmitted (applied to + * every platform, there's no per-platform media field in the request + * today), otherwise that platform's own scoped media + * (PostPlatform::scopedMediaItems()), falling back further to the post's + * full stored media. This closes the gap where a client publishes a + * misconfigured post, e.g. a PDF on a regular LinkedIn post, without + * resubmitting content_type, which a field-level rule on + * `platforms.*.content_type` would skip. */ private function addMediaCompatibilityErrors(Validator $validator): void { /** @var Post $post */ $post = $this->route('post'); - $media = $this->has('media') ? (array) $this->input('media', []) : (array) ($post->media ?? []); - $entries = ContentTypeCompatibleWithMedia::entriesForUpdate( $post, $this->has('platforms') ? (array) $this->input('platforms', []) : null, + $this->has('media') ? (array) $this->input('media', []) : null, ); - foreach (ContentTypeCompatibleWithMedia::errorsFor($entries, $media) as $key => $message) { + foreach (ContentTypeCompatibleWithMedia::errorsFor($entries) as $key => $message) { $validator->errors()->add($key, $message); } } diff --git a/app/Http/Requests/App/Post/UpdatePostRequest.php b/app/Http/Requests/App/Post/UpdatePostRequest.php index 64a40b2a7..81e1e6666 100644 --- a/app/Http/Requests/App/Post/UpdatePostRequest.php +++ b/app/Http/Requests/App/Post/UpdatePostRequest.php @@ -7,6 +7,7 @@ use App\Enums\Post\Status; use App\Enums\PostPlatform\ContentType; use App\Enums\SocialAccount\Platform; +use App\Models\Post; use App\Rules\ContentFitsPlatformLimits; use App\Rules\ContentTypeCompatibleWithMedia; use App\Support\PostMediaRules; @@ -53,7 +54,6 @@ public function rules(): array $enforcesMediaCompatibility ? 'required' : 'sometimes', 'string', Rule::in(array_column(ContentType::cases(), 'value')), - Rule::when($enforcesMediaCompatibility, [new ContentTypeCompatibleWithMedia]), ], ...PostPlatformMetaRules::rules(), 'label_ids' => ['sometimes', 'array'], @@ -87,6 +87,14 @@ public function withValidator(Validator $validator): void ); }); + $validator->after(function (Validator $validator): void { + if (! $this->isPublishingOrScheduling()) { + return; + } + + $this->addMediaCompatibilityErrors($validator); + }); + $validator->after(function (Validator $validator): void { if (! $this->isPublishingOrScheduling()) { return; @@ -117,6 +125,31 @@ private function isPublishingOrScheduling(): bool ); } + /** + * Validate every platform's *effective* content_type (resubmitted in this + * request, or its stored value) against its own *effective* media: the + * request's media when resubmitted (applied to every platform, there's no + * per-platform media field in the request today), otherwise that + * platform's own scoped media (PostPlatform::scopedMediaItems()), falling + * back further to the post's full stored media. Mirrors the public API's + * withValidator check (App\Http\Requests\Api\Post\UpdatePostRequest). + */ + private function addMediaCompatibilityErrors(Validator $validator): void + { + /** @var Post $post */ + $post = $this->route('post'); + + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate( + $post, + $this->has('platforms') ? (array) $this->input('platforms', []) : null, + $this->has('media') ? (array) $this->input('media', []) : null, + ); + + foreach (ContentTypeCompatibleWithMedia::errorsFor($entries) as $key => $message) { + $validator->errors()->add($key, $message); + } + } + /** * @return Collection */ diff --git a/app/Mcp/Tools/Post/UpdatePostTool.php b/app/Mcp/Tools/Post/UpdatePostTool.php index c48885578..1e7a48ba5 100644 --- a/app/Mcp/Tools/Post/UpdatePostTool.php +++ b/app/Mcp/Tools/Post/UpdatePostTool.php @@ -80,7 +80,6 @@ public function handle(Request $request): Response|ResponseFactory if ($status === Status::Scheduled->value) { $errors = ContentTypeCompatibleWithMedia::errorsFor( ContentTypeCompatibleWithMedia::entriesForUpdate($post, data_get($validated, 'platforms')), - (array) ($post->media ?? []), ); if ($errors !== []) { diff --git a/app/Rules/ContentTypeCompatibleWithMedia.php b/app/Rules/ContentTypeCompatibleWithMedia.php index a172344b4..0f51b3a0c 100644 --- a/app/Rules/ContentTypeCompatibleWithMedia.php +++ b/app/Rules/ContentTypeCompatibleWithMedia.php @@ -4,12 +4,15 @@ namespace App\Rules; +use App\Dto\MediaItem; use App\Enums\Media\Type as MediaType; use App\Enums\PostPlatform\ContentType; use App\Models\Post; +use App\Models\PostPlatform; use Closure; use Illuminate\Contracts\Validation\DataAwareRule; use Illuminate\Contracts\Validation\ValidationRule; +use Illuminate\Support\Collection; use Illuminate\Support\Number; use Illuminate\Translation\PotentiallyTranslatedString; use Illuminate\Validation\ValidationException; @@ -40,19 +43,18 @@ public function setData(array $data): static } /** - * Validate every enabled platform's stored content_type against the post's - * stored media. Used by publish flows that don't resubmit media (e.g. the - * MCP publish tool) — the media-side mirror of + * Validate every enabled platform's stored content_type against its own + * scoped media (falling back to the post's full media when the platform + * has no per-platform selection, see PostPlatform::scopedMediaItems()). + * Used by publish flows that don't resubmit media (e.g. the MCP publish + * tool), the media-side mirror of * PostPlatformMetaRules::assertStoredPostPublishable(). * * @throws ValidationException */ public static function assertStoredPostCompatible(Post $post): void { - $errors = self::errorsFor( - self::entriesForUpdate($post, null), - (array) ($post->media ?? []), - ); + $errors = self::errorsFor(self::entriesForUpdate($post, null)); if ($errors !== []) { throw ValidationException::withMessages($errors); @@ -62,50 +64,104 @@ public static function assertStoredPostCompatible(Post $post): void /** * The per-platform entries to validate for a post update: each platform's * effective content_type (resubmitted in this request, else its stored - * value), keyed by the error path the caller surfaces. When $requestPlatforms - * is null, the post's currently-enabled platforms are used. + * value) paired with the media it publishes, keyed by the error path the + * caller surfaces. When $requestPlatforms is null, the post's + * currently-enabled platforms are used. + * + * Per entry, the media to validate against is resolved in this order: + * 1. $requestMedia: the request's resubmitted media, when present. This + * applies uniformly to every entry (there's no per-platform media field + * in the request today), matching what the client is about to save. + * 2. The stored PostPlatform's own scoped media + * (PostPlatform::scopedMediaItems()), the per-platform selection, once + * one exists. An empty selection already falls back to every post media + * item inside that helper. + * 3. The post's full stored media, only reached when no PostPlatform row + * could be resolved for the entry at all (defensive, every real caller + * today has one). * * @param array|null $requestPlatforms - * @return array + * @param array|null $requestMedia + * @return array>}> */ - public static function entriesForUpdate(Post $post, ?array $requestPlatforms): array + public static function entriesForUpdate(Post $post, ?array $requestPlatforms, ?array $requestMedia = null): array { if (is_array($requestPlatforms)) { - $stored = $post->postPlatforms()->get()->keyBy('id'); - - return collect($requestPlatforms)->map(fn ($platform, $index): array => [ - 'key' => "platforms.{$index}.content_type", - 'content_type' => data_get($platform, 'content_type') - ?? $stored->get(data_get($platform, 'id'))?->content_type?->value, - ])->all(); + $stored = self::postPlatformsFor($post)->keyBy('id'); + + return collect($requestPlatforms)->map(function ($platform, $index) use ($stored, $post, $requestMedia): array { + $storedPlatform = $stored->get(data_get($platform, 'id')); + + return [ + 'key' => "platforms.{$index}.content_type", + 'content_type' => data_get($platform, 'content_type') + ?? $storedPlatform?->content_type?->value, + 'media' => self::resolveMedia($storedPlatform, $post, $requestMedia), + ]; + })->all(); } - return $post->postPlatforms()->enabled()->get()->values() + return self::postPlatformsFor($post) + ->filter(fn (PostPlatform $postPlatform): bool => $postPlatform->enabled) + ->values() ->map(fn ($postPlatform, $index): array => [ 'key' => "platforms.{$index}.content_type", 'content_type' => $postPlatform->content_type?->value, + 'media' => self::resolveMedia($postPlatform, $post, $requestMedia), ])->all(); } /** - * Validate a set of platform entries against the given media, returning - * `[errorKey => message]` for each incompatible content_type. + * The post's platforms with the inverse `post` relation pre-set, so + * PostPlatform::scopedMediaItems() (which reads `$this->post->mediaItems`) + * never triggers a lazy load under Model::shouldBeStrict() in tests/local. + * + * @return Collection + */ + private static function postPlatformsFor(Post $post): Collection + { + return $post->postPlatforms()->get() + ->each(fn (PostPlatform $postPlatform) => $postPlatform->setRelation('post', $post)); + } + + /** + * @param array|null $requestMedia + * @return array> + */ + private static function resolveMedia(?PostPlatform $storedPlatform, Post $post, ?array $requestMedia): array + { + if ($requestMedia !== null) { + return collect($requestMedia)->map(fn (mixed $item): array => (array) $item)->all(); + } + + if ($storedPlatform !== null) { + return $storedPlatform->scopedMediaItems() + ->map(fn (MediaItem $item): array => $item->toArray()) + ->all(); + } + + return (array) ($post->media ?? []); + } + + /** + * Validate a set of platform entries, each against its own media, returning + * `[errorKey => message]` for each incompatible content_type. A kind + * violation ("does not accept GIF") is the root cause, so a size violation + * on the same item must not overwrite it, one message per entry. * - * @param array $entries - * @param array $media + * @param array>}> $entries * @return array */ - public static function errorsFor(array $entries, array $media): array + public static function errorsFor(array $entries): array { $errors = []; - $rule = new self($media); - foreach ($entries as ['key' => $key, 'content_type' => $contentType]) { + foreach ($entries as ['key' => $key, 'content_type' => $contentType, 'media' => $media]) { if ($contentType === null) { continue; } - $rule->validate($key, $contentType, function (string $message) use (&$errors, $key): void { + (new self($media))->validate($key, $contentType, function (string $message) use (&$errors, $key): void { $errors[$key] = $message; }); } diff --git a/tests/Unit/Rules/ContentTypeCompatibleWithMediaTest.php b/tests/Unit/Rules/ContentTypeCompatibleWithMediaTest.php index ed33a668b..419592126 100644 --- a/tests/Unit/Rules/ContentTypeCompatibleWithMediaTest.php +++ b/tests/Unit/Rules/ContentTypeCompatibleWithMediaTest.php @@ -2,9 +2,11 @@ declare(strict_types=1); +use App\Dto\MediaItem; use App\Enums\Media\Type as MediaType; use App\Enums\PostPlatform\ContentType; use App\Rules\ContentTypeCompatibleWithMedia; +use Illuminate\Support\Str; function runMediaRule(string $contentType, array $media): array { @@ -391,3 +393,175 @@ function runMediaRule(string $contentType, array $media): array test('does nothing for invalid content type values', function () { expect(runMediaRule('not_a_real_content_type', []))->toBe([]); }); + +// entriesForUpdate() / errorsFor(): the per-platform pipeline that resolves +// each entry's own effective media before validating it, instead of every +// platform sharing one media list. + +use App\Models\Media; +use App\Models\Post; +use App\Models\PostPlatform; +use App\Models\SocialAccount; +use App\Models\Workspace; + +/** + * A post with the given media (already-hosted Media rows, each mirrored into + * `posts.media` so PostPlatform::scopedMediaItems() has something to filter). + */ +function postWithMedia(array $mediaModels): Post +{ + $workspace = Workspace::factory()->create(); + + return Post::factory()->create([ + 'workspace_id' => $workspace->id, + 'media' => collect($mediaModels)->map(fn (Media $media) => MediaItem::fromMedia($media)->toArray())->all(), + ]); +} + +function makeMedia(Workspace $workspace, string $type = 'image'): Media +{ + $factory = match ($type) { + 'video' => Media::factory()->video(), + 'document' => Media::factory()->document(), + default => Media::factory(), + }; + + return $factory->create(['mediable_type' => Workspace::class, 'mediable_id' => $workspace->id]); +} + +function makePostPlatform(Post $post, ContentType $contentType): PostPlatform +{ + return PostPlatform::factory()->create([ + 'post_id' => $post->id, + 'social_account_id' => SocialAccount::factory()->create(['workspace_id' => $post->workspace_id]), + 'content_type' => $contentType, + ]); +} + +test('each platform is validated against its own scoped media, not a list shared with every other platform', function () { + $workspace = Workspace::factory()->create(); + $image = makeMedia($workspace, 'image'); + $video = makeMedia($workspace, 'video'); + $post = postWithMedia([$image, $video]); + + // TikTok requires video and rejects images; Pinterest rejects video. With + // the OLD shared-media behaviour both would fail because the other + // platform's item is also present in the pool. + $tiktok = makePostPlatform($post, ContentType::TikTokVideo); + $tiktok->media()->attach($video->id); + + $pinterest = makePostPlatform($post, ContentType::PinterestPin); + $pinterest->media()->attach($image->id); + + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate($post, [ + ['id' => $tiktok->id, 'content_type' => ContentType::TikTokVideo->value], + ['id' => $pinterest->id, 'content_type' => ContentType::PinterestPin->value], + ]); + + expect(ContentTypeCompatibleWithMedia::errorsFor($entries))->toBe([]); +}); + +test('falls back to the platform own scoped media when the request omits media', function () { + $workspace = Workspace::factory()->create(); + $image = makeMedia($workspace, 'image'); + $video = makeMedia($workspace, 'video'); + $post = postWithMedia([$image, $video]); + + $linkedin = makePostPlatform($post, ContentType::LinkedInPost); + $linkedin->media()->attach($image->id); + + // No $requestMedia argument: entriesForUpdate must read the platform's + // own scoped media (image only), not the post's full image+video list, + // which LinkedIn would reject as mixed media. + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate($post, [ + ['id' => $linkedin->id, 'content_type' => ContentType::LinkedInPost->value], + ]); + + expect(ContentTypeCompatibleWithMedia::errorsFor($entries))->toBe([]); +}); + +test('request media overrides every platform own scoped media', function () { + $workspace = Workspace::factory()->create(); + $image = makeMedia($workspace, 'image'); + $video = makeMedia($workspace, 'video'); + $post = postWithMedia([$image, $video]); + + $linkedin = makePostPlatform($post, ContentType::LinkedInPost); + $linkedin->media()->attach($image->id); + + // The platform is scoped to the image alone (would pass on its own), but + // the request resubmits both items as the new post media, so LinkedIn + // must reject the mix. + $requestMedia = [ + MediaItem::fromMedia($image)->toArray(), + MediaItem::fromMedia($video)->toArray(), + ]; + + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate( + $post, + [['id' => $linkedin->id, 'content_type' => ContentType::LinkedInPost->value]], + $requestMedia, + ); + + $errors = ContentTypeCompatibleWithMedia::errorsFor($entries); + + expect($errors)->toHaveCount(1); + expect($errors['platforms.0.content_type'])->toContain("can't be combined in the same post"); +}); + +test('falls back to the full post media when no stored platform can be resolved for the entry', function () { + $workspace = Workspace::factory()->create(); + $image = makeMedia($workspace, 'image'); + $video = makeMedia($workspace, 'video'); + $post = postWithMedia([$image, $video]); + + // No PostPlatform row exists for this id at all (defensive path). + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate($post, [ + ['id' => (string) Str::uuid(), 'content_type' => ContentType::LinkedInPost->value], + ]); + + $errors = ContentTypeCompatibleWithMedia::errorsFor($entries); + + expect($errors)->toHaveCount(1); + expect($errors['platforms.0.content_type'])->toContain("can't be combined in the same post"); +}); + +test('assertStoredPostCompatible validates every enabled platform against its own scoped media', function () { + $workspace = Workspace::factory()->create(); + $image = makeMedia($workspace, 'image'); + $video = makeMedia($workspace, 'video'); + $post = postWithMedia([$image, $video]); + + $tiktok = makePostPlatform($post, ContentType::TikTokVideo); + $tiktok->media()->attach($video->id); + + $pinterest = makePostPlatform($post, ContentType::PinterestPin); + $pinterest->media()->attach($image->id); + + ContentTypeCompatibleWithMedia::assertStoredPostCompatible($post); +})->throwsNoExceptions(); + +test('a content type requiring media correctly fails when its scoped media resolves to none, this is the correct error, not a bug to soften', function () { + // Randfall from the plan: a platform's per-platform selection ends up + // empty (here: it was assigned media that has since been removed from + // the post's media list, e.g. everything else got reassigned/dropped). + // Content types that require media must still fail, not be softened. + $workspace = Workspace::factory()->create(); + $keptOnPost = makeMedia($workspace, 'image'); + $removedFromPost = makeMedia($workspace, 'video'); + $post = postWithMedia([$keptOnPost]); // $removedFromPost is NOT in posts.media + + $facebookStory = makePostPlatform($post, ContentType::FacebookStory); + $facebookStory->media()->attach($removedFromPost->id); + + $entries = ContentTypeCompatibleWithMedia::entriesForUpdate($post, [ + ['id' => $facebookStory->id, 'content_type' => ContentType::FacebookStory->value], + ]); + + expect($entries[0]['media'])->toBe([]); + + $errors = ContentTypeCompatibleWithMedia::errorsFor($entries); + + expect($errors)->toHaveCount(1); + expect($errors['platforms.0.content_type'])->toContain('requires at least one image or video'); +});