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'); +});