trypost/app/Actions/Post/UpdatePost.php

85 lines
2.8 KiB
PHP
Raw Normal View History

<?php
declare(strict_types=1);
namespace App\Actions\Post;
use App\Enums\Post\Action as PostAction;
use App\Enums\Post\Status as PostStatus;
use App\Jobs\PublishPost;
use App\Models\Post;
use App\Models\Workspace;
use Carbon\Carbon;
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\DB;
class UpdatePost
{
/**
* @return array{post: Post, action: PostAction|null}
*/
public static function execute(Workspace $workspace, Post $post, array $data): array
{
if ($post->status === PostStatus::Published) {
return ['post' => $post, 'action' => PostAction::AlreadyPublished];
}
$scheduledAt = $post->scheduled_at;
if (data_get($data, 'scheduled_at')) {
$scheduledAt = Carbon::parse(data_get($data, 'scheduled_at'))->utc();
}
$status = data_get($data, 'status', $post->status);
$post->update([
'content' => data_get($data, 'content', $post->content),
'media' => data_get($data, 'media', $post->media),
'status' => $status === PostStatus::Publishing->value ? PostStatus::Publishing : $status,
'scheduled_at' => $scheduledAt,
]);
if (Arr::has($data, 'label_ids')) {
$post->labels()->sync(data_get($data, 'label_ids', []));
}
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
if (Arr::has($data, 'platforms')) {
DB::transaction(function () use ($post, $data) {
$post->postPlatforms()->update(['enabled' => false]);
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
foreach (data_get($data, 'platforms', []) as $platformData) {
$updateData = ['enabled' => true];
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
if (data_get($platformData, 'content_type') !== null) {
$updateData['content_type'] = data_get($platformData, 'content_type');
}
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
if (data_get($platformData, 'meta') !== null) {
$postPlatform = $post->postPlatforms()->where('id', data_get($platformData, 'id'))->first();
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
if ($postPlatform) {
$updateData['meta'] = array_merge($postPlatform->meta ?? [], data_get($platformData, 'meta'));
}
}
fix: address PR review findings — publish, REST store, SSRF, race Code-review surfaced two correctness bugs and a security gap that needed to land before merging. - UpdatePost::execute disabled every platform when called without a `platforms` key. PublishPostTool relied on that path, so every publish-via-MCP queued a job whose handler then found nothing enabled to publish to. Wrap the platform toggle in `Arr::has($data, 'platforms')` (matches the existing label_ids guard a few lines up). Add a regression assertion to `PostPublishToolTest::publish post immediate dispatches PublishPost job` that the previously-enabled platform stays enabled. - StorePostRequest declared rules for only `platforms`, `scheduled_at`, and `status`. `validated()` then stripped `content`, `media`, and `label_ids`, so REST `POST /api/posts` silently created empty drafts. Added rules for content / media / label_ids (with workspace-scoped `Rule::exists` for labels) and dropped the unused `status` field — REST callers transition state via `PUT /posts/{id}`. Removed the dead `platforms.*.content` rule. Added a feature test that asserts content + media + labels roundtrip on create, plus a regression that an `is_active=false` social_account is rejected at validation. - CreatePost::execute now syncs label_ids itself so REST and MCP share the behavior. Removed the duplicate sync from CreatePostTool. - MCP UpdatePostTool didn't scope `platforms.*.id` to the post being updated, drifting from the REST UpdatePostRequest which adds `Rule::exists('post_platforms','id')->where('post_id', ...)`. Now it loads the post first (failing fast with `Post not found.` if the workspace check rejects), then uses the same Rule::exists. - MediaAttacher fetched any URL the caller passed, including loopback / link-local / private targets — classic SSRF pivot. Now `isPublicHttpUrl` rejects non-http(s) schemes, restricted IP ranges, and DNS hostnames whose A/AAAA records resolve into those ranges (covers DNS rebinding). Bypassed under `app()->runningUnitTests()` so `Http::fake()` keeps working. Streaming the response body lets us abort early once we exceed MAX_BYTES instead of buffering the full payload first; redirects are disabled so a 200→302 trick can't bypass the host check. - The `media[]` JSON column had a lost-update race in `attachFromUrls`: read `$post->media`, mutate in PHP, write back. Two concurrent calls clobbered each other. Now wrapped in a transaction with `lockForUpdate()`. - ESLint: `resources/js/actions/**` and `resources/js/routes/**` are auto-generated by Wayfinder on every build. Their import order matches PHP scan order, not alphabetical, so import/order fought eslint-fix forever. Added them to ignores.
2026-05-04 15:16:39 +00:00
$post->postPlatforms()
->where('id', data_get($platformData, 'id'))
->update($updateData);
}
});
}
if ($status === PostStatus::Publishing->value) {
$post->update(['scheduled_at' => now()]);
PublishPost::dispatch($post);
return ['post' => $post, 'action' => PostAction::Publishing];
}
if ($status === PostStatus::Scheduled->value) {
return ['post' => $post, 'action' => PostAction::Scheduled];
}
return ['post' => $post, 'action' => null];
}
}