From 690222de8b6eb8f30934374daafaa9de5440695b Mon Sep 17 00:00:00 2001 From: James Brooks Date: Wed, 15 Jul 2026 10:06:29 +0100 Subject: [PATCH] Fix API documentation and request handling mismatches Fixes cachethq/cachet#4618 by correcting the generated OpenAPI docs and the API behavior they describe: - Force JSON responses on all API routes so validation and auth errors return JSON instead of HTML redirects, without requiring an Accept header - Accept ISO 8601 dates (as documented) for schedule and schedule update date fields via a new FlexibleDateTimeCast matching the "date" validation rule - Mark components.*.id and components.*.status as required so the generated schema and examples include them - Document meta and template_vars as key/value objects instead of arrays of strings - Validate and document calc_type, display_chart and places on metric creation - Describe accepted date formats and the metric point timestamp field - Remove leftover Scribe bodyParameters() methods Co-Authored-By: Claude Fable 5 --- config/cachet.php | 2 + src/Data/Casts/FlexibleDateTimeCast.php | 35 ++++++++++++ .../Component/CreateComponentRequestData.php | 25 +++------ .../Component/UpdateComponentRequestData.php | 7 +++ .../CreateComponentGroupRequestData.php | 22 +++----- .../UpdateComponentGroupRequestData.php | 22 +++----- .../Incident/CreateIncidentRequestData.php | 21 ++++++- .../Incident/UpdateIncidentRequestData.php | 10 ++++ .../Metric/CreateMetricPointRequestData.php | 5 ++ .../Metric/CreateMetricRequestData.php | 4 ++ .../Schedule/CreateScheduleRequestData.php | 23 ++++++-- .../Schedule/UpdateScheduleRequestData.php | 23 ++++++-- .../CreateScheduleUpdateRequestData.php | 7 ++- .../CreateSubscriberRequestData.php | 7 +++ .../UpdateSubscriberRequestData.php | 7 +++ src/Http/Middleware/ForceJsonResponse.php | 20 +++++++ tests/Architecture/DataTest.php | 2 + tests/Feature/Api/MetricTest.php | 33 +++++++++++ tests/Feature/Api/ScheduleTest.php | 55 +++++++++++++++++++ 19 files changed, 268 insertions(+), 62 deletions(-) create mode 100644 src/Data/Casts/FlexibleDateTimeCast.php create mode 100644 src/Http/Middleware/ForceJsonResponse.php diff --git a/config/cachet.php b/config/cachet.php index 557f5f56..69ad202a 100644 --- a/config/cachet.php +++ b/config/cachet.php @@ -3,6 +3,7 @@ use App\Models\User; use Cachet\Http\Middleware\AuthenticateApiIfProtected; use Cachet\Http\Middleware\EnsureApiIsEnabled; +use Cachet\Http\Middleware\ForceJsonResponse; use Illuminate\Routing\Middleware\SubstituteBindings; return [ @@ -91,6 +92,7 @@ | */ 'api_middleware' => [ + ForceJsonResponse::class, EnsureApiIsEnabled::class, AuthenticateApiIfProtected::class, SubstituteBindings::class, diff --git a/src/Data/Casts/FlexibleDateTimeCast.php b/src/Data/Casts/FlexibleDateTimeCast.php new file mode 100644 index 00000000..9c8eaf7d --- /dev/null +++ b/src/Data/Casts/FlexibleDateTimeCast.php @@ -0,0 +1,35 @@ + ['int', 'min:0'], 'enabled' => ['boolean'], 'component_group_id' => ['int', 'min:0', Rule::exists('component_groups', 'id')], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } - - /** - * Specify body parameter documentation for Scribe. - */ - public function bodyParameters(): array - { - return [ - 'status' => [ - 'description' => 'The status of the component. See [Component Statuses](/v3.x/guide/components#component-statuses) for more information.', - 'example' => '1', - 'required' => false, - 'schema' => [ - 'type' => 'integer', - 'enum' => ComponentStatusEnum::cases(), - ], - ], - ]; - } } diff --git a/src/Data/Requests/Component/UpdateComponentRequestData.php b/src/Data/Requests/Component/UpdateComponentRequestData.php index 5a9890a2..0de51897 100644 --- a/src/Data/Requests/Component/UpdateComponentRequestData.php +++ b/src/Data/Requests/Component/UpdateComponentRequestData.php @@ -31,6 +31,13 @@ public static function rules(ValidationContext $context): array 'order' => ['int', 'min:0'], 'component_group_id' => ['int', 'min:0', Rule::exists('component_groups', 'id')], 'enabled' => ['boolean'], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/ComponentGroup/CreateComponentGroupRequestData.php b/src/Data/Requests/ComponentGroup/CreateComponentGroupRequestData.php index bc80d9a2..2875e6fc 100644 --- a/src/Data/Requests/ComponentGroup/CreateComponentGroupRequestData.php +++ b/src/Data/Requests/ComponentGroup/CreateComponentGroupRequestData.php @@ -43,22 +43,14 @@ public static function rules(ValidationContext $context): array ], 'components' => ['array'], 'components.*' => ['int', 'min:0', Rule::exists('components', 'id')], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } - - public function bodyParameters(): array - { - return [ - 'collapsed' => [ - 'description' => 'The collapsed state of the component group on the status page.', - 'example' => '0', - 'required' => false, - 'schema' => [ - 'type' => 'integer', - 'enum' => ComponentGroupVisibilityEnum::cases(), - ], - ], - ]; - } } diff --git a/src/Data/Requests/ComponentGroup/UpdateComponentGroupRequestData.php b/src/Data/Requests/ComponentGroup/UpdateComponentGroupRequestData.php index be60cc29..b603b21d 100644 --- a/src/Data/Requests/ComponentGroup/UpdateComponentGroupRequestData.php +++ b/src/Data/Requests/ComponentGroup/UpdateComponentGroupRequestData.php @@ -42,22 +42,14 @@ public static function rules(ValidationContext $context): array ], 'components' => ['array'], 'components.*' => ['int', 'min:0', Rule::exists('components', 'id')], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } - - public function bodyParameters(): array - { - return [ - 'collapsed' => [ - 'description' => 'The collapsed state of the component group on the status page.', - 'example' => '0', - 'required' => false, - 'schema' => [ - 'type' => 'integer', - 'enum' => ComponentGroupVisibilityEnum::cases(), - ], - ], - ]; - } } diff --git a/src/Data/Requests/Incident/CreateIncidentRequestData.php b/src/Data/Requests/Incident/CreateIncidentRequestData.php index bf7553bb..748e4d92 100644 --- a/src/Data/Requests/Incident/CreateIncidentRequestData.php +++ b/src/Data/Requests/Incident/CreateIncidentRequestData.php @@ -50,13 +50,30 @@ public static function rules(ValidationContext $context): array 'visible' => ['boolean'], 'stickied' => ['boolean'], 'notifications' => ['boolean'], + /** + * The date/time the incident occurred, e.g. "2023-11-07 05:31:56" or ISO 8601. Defaults to now. + */ 'occurred_at' => ['nullable', 'date'], + /** + * Key/value variables passed to the incident template when rendering the message. + * + * @var array + * + * @example {"reason": "scheduled maintenance"} + */ 'template_vars' => ['array'], 'component_id' => [Rule::exists('components', 'id')], 'component_status' => ['nullable', Rule::enum(ComponentStatusEnum::class), 'required_with:component_id'], 'components' => ['array'], - 'components.*.id' => ['required_with:components', 'int', 'exists:components,id'], - 'components.*.status' => ['required_with:components', 'int', Rule::enum(ComponentStatusEnum::class)], + 'components.*.id' => ['required', 'int', 'exists:components,id'], + 'components.*.status' => ['required', 'int', Rule::enum(ComponentStatusEnum::class)], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/Incident/UpdateIncidentRequestData.php b/src/Data/Requests/Incident/UpdateIncidentRequestData.php index 6b18d33f..1060ad20 100644 --- a/src/Data/Requests/Incident/UpdateIncidentRequestData.php +++ b/src/Data/Requests/Incident/UpdateIncidentRequestData.php @@ -31,7 +31,17 @@ public static function rules(ValidationContext $context): array 'visible' => ['boolean'], 'stickied' => ['boolean'], 'notifications' => ['boolean'], + /** + * The date/time the incident occurred, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'occurred_at' => ['nullable', 'date'], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/Metric/CreateMetricPointRequestData.php b/src/Data/Requests/Metric/CreateMetricPointRequestData.php index 10376730..c455cfa9 100644 --- a/src/Data/Requests/Metric/CreateMetricPointRequestData.php +++ b/src/Data/Requests/Metric/CreateMetricPointRequestData.php @@ -17,6 +17,11 @@ public static function rules(ValidationContext $context): array { return [ 'value' => ['required', 'numeric'], + /** + * The date/time or Unix timestamp the metric point was recorded at. Defaults to now. + * + * @example "2023-11-07 05:31:56" + */ 'timestamp' => ['nullable', new ValidTimestamp], ]; } diff --git a/src/Data/Requests/Metric/CreateMetricRequestData.php b/src/Data/Requests/Metric/CreateMetricRequestData.php index 618717d1..ebcba0bf 100644 --- a/src/Data/Requests/Metric/CreateMetricRequestData.php +++ b/src/Data/Requests/Metric/CreateMetricRequestData.php @@ -5,6 +5,7 @@ use Cachet\Data\BaseData; use Cachet\Enums\MetricTypeEnum; use Cachet\Rules\FactorOfSixty; +use Illuminate\Validation\Rule; use Spatie\LaravelData\Support\Validation\ValidationContext; final class CreateMetricRequestData extends BaseData @@ -25,9 +26,12 @@ public static function rules(ValidationContext $context): array return [ 'name' => ['required', 'string', 'max:255'], 'suffix' => ['required', 'string', 'max:255'], + 'calc_type' => ['nullable', Rule::enum(MetricTypeEnum::class)], 'description' => ['string'], 'default_value' => ['decimal:1,2'], + 'display_chart' => ['nullable', 'boolean'], 'threshold' => ['int', 'min:0', 'max:60', new FactorOfSixty], + 'places' => ['int', 'min:0', 'max:4'], ]; } } diff --git a/src/Data/Requests/Schedule/CreateScheduleRequestData.php b/src/Data/Requests/Schedule/CreateScheduleRequestData.php index c6545e61..2aacac0e 100644 --- a/src/Data/Requests/Schedule/CreateScheduleRequestData.php +++ b/src/Data/Requests/Schedule/CreateScheduleRequestData.php @@ -3,13 +3,13 @@ namespace Cachet\Data\Requests\Schedule; use Cachet\Data\BaseData; +use Cachet\Data\Casts\FlexibleDateTimeCast; use Cachet\Enums\ComponentStatusEnum; use Cachet\Enums\ScheduleStatusEnum; use Carbon\Carbon; use Illuminate\Validation\Rule; use Spatie\LaravelData\Attributes\DataCollectionOf; use Spatie\LaravelData\Attributes\WithCast; -use Spatie\LaravelData\Casts\DateTimeInterfaceCast; use Spatie\LaravelData\Support\Validation\ValidationContext; final class CreateScheduleRequestData extends BaseData @@ -17,9 +17,9 @@ final class CreateScheduleRequestData extends BaseData public function __construct( public readonly string $name, public readonly string $message, - #[WithCast(DateTimeInterfaceCast::class, format: 'Y-m-d H:i:s')] + #[WithCast(FlexibleDateTimeCast::class)] public readonly Carbon $scheduledAt, - #[WithCast(DateTimeInterfaceCast::class, format: 'Y-m-d H:i:s')] + #[WithCast(FlexibleDateTimeCast::class)] public readonly ?Carbon $completedAt = null, public readonly ?ScheduleStatusEnum $status = null, public readonly bool $notifications = false, @@ -34,12 +34,25 @@ public static function rules(ValidationContext $context): array return [ 'name' => ['required', 'string', 'max:255'], 'message' => ['required', 'string'], + /** + * The date/time the maintenance window starts, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'scheduled_at' => ['required', 'date'], + /** + * The date/time the maintenance window ends, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'completed_at' => ['nullable', 'date'], 'notifications' => ['boolean'], 'components' => ['array'], - 'components.*.id' => ['required_with:components', 'int', 'exists:components,id'], - 'components.*.status' => ['required_with:components', 'int', Rule::enum(ComponentStatusEnum::class)], + 'components.*.id' => ['required', 'int', 'exists:components,id'], + 'components.*.status' => ['required', 'int', Rule::enum(ComponentStatusEnum::class)], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/Schedule/UpdateScheduleRequestData.php b/src/Data/Requests/Schedule/UpdateScheduleRequestData.php index e5afc24c..14585f7a 100644 --- a/src/Data/Requests/Schedule/UpdateScheduleRequestData.php +++ b/src/Data/Requests/Schedule/UpdateScheduleRequestData.php @@ -3,13 +3,13 @@ namespace Cachet\Data\Requests\Schedule; use Cachet\Data\BaseData; +use Cachet\Data\Casts\FlexibleDateTimeCast; use Cachet\Enums\ComponentStatusEnum; use Cachet\Enums\ScheduleStatusEnum; use Carbon\Carbon; use Illuminate\Validation\Rule; use Spatie\LaravelData\Attributes\DataCollectionOf; use Spatie\LaravelData\Attributes\WithCast; -use Spatie\LaravelData\Casts\DateTimeInterfaceCast; use Spatie\LaravelData\Support\Validation\ValidationContext; final class UpdateScheduleRequestData extends BaseData @@ -18,9 +18,9 @@ public function __construct( public readonly ?string $name = null, public readonly ?string $message = null, public readonly ?ScheduleStatusEnum $status = null, - #[WithCast(DateTimeInterfaceCast::class, format: 'Y-m-d H:i:s')] + #[WithCast(FlexibleDateTimeCast::class)] public readonly ?Carbon $scheduledAt = null, - #[WithCast(DateTimeInterfaceCast::class, format: 'Y-m-d H:i:s')] + #[WithCast(FlexibleDateTimeCast::class)] public readonly ?Carbon $completedAt = null, #[DataCollectionOf(ScheduleComponentRequestData::class)] public readonly ?array $components = null, @@ -33,11 +33,24 @@ public static function rules(ValidationContext $context): array return [ 'name' => ['string', 'max:255'], 'message' => ['string'], + /** + * The date/time the maintenance window starts, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'scheduled_at' => ['nullable', 'date'], + /** + * The date/time the maintenance window ends, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'completed_at' => ['nullable', 'date'], 'components' => ['array'], - 'components.*.id' => ['required_with:components', 'int', 'exists:components,id'], - 'components.*.status' => ['required_with:components', Rule::enum(ComponentStatusEnum::class)], + 'components.*.id' => ['required', 'int', 'exists:components,id'], + 'components.*.status' => ['required', Rule::enum(ComponentStatusEnum::class)], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/ScheduleUpdate/CreateScheduleUpdateRequestData.php b/src/Data/Requests/ScheduleUpdate/CreateScheduleUpdateRequestData.php index 47a51c81..a127193c 100644 --- a/src/Data/Requests/ScheduleUpdate/CreateScheduleUpdateRequestData.php +++ b/src/Data/Requests/ScheduleUpdate/CreateScheduleUpdateRequestData.php @@ -3,16 +3,16 @@ namespace Cachet\Data\Requests\ScheduleUpdate; use Cachet\Data\BaseData; +use Cachet\Data\Casts\FlexibleDateTimeCast; use Carbon\Carbon; use Spatie\LaravelData\Attributes\WithCast; -use Spatie\LaravelData\Casts\DateTimeInterfaceCast; use Spatie\LaravelData\Support\Validation\ValidationContext; final class CreateScheduleUpdateRequestData extends BaseData { public function __construct( public readonly string $message, - #[WithCast(DateTimeInterfaceCast::class, format: 'Y-m-d H:i:s')] + #[WithCast(FlexibleDateTimeCast::class)] public readonly ?Carbon $completedAt = null, ) {} @@ -20,6 +20,9 @@ public static function rules(ValidationContext $context): array { return [ 'message' => ['required', 'string'], + /** + * The date/time the maintenance window ended, e.g. "2023-11-07 05:31:56" or ISO 8601. + */ 'completed_at' => ['nullable', 'date'], ]; } diff --git a/src/Data/Requests/Subscriber/CreateSubscriberRequestData.php b/src/Data/Requests/Subscriber/CreateSubscriberRequestData.php index ddd3a801..b80f46ea 100644 --- a/src/Data/Requests/Subscriber/CreateSubscriberRequestData.php +++ b/src/Data/Requests/Subscriber/CreateSubscriberRequestData.php @@ -25,6 +25,13 @@ public static function rules(ValidationContext $context): array 'components' => ['array'], 'components.*' => ['int', 'min:0', Rule::exists('components', 'id')], 'verified' => ['bool'], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Data/Requests/Subscriber/UpdateSubscriberRequestData.php b/src/Data/Requests/Subscriber/UpdateSubscriberRequestData.php index 66584770..fc14a71d 100644 --- a/src/Data/Requests/Subscriber/UpdateSubscriberRequestData.php +++ b/src/Data/Requests/Subscriber/UpdateSubscriberRequestData.php @@ -23,6 +23,13 @@ public static function rules(ValidationContext $context): array 'global' => ['bool'], 'components' => ['array'], 'components.*' => ['int', 'min:0', Rule::exists('components', 'id')], + /** + * Key/value metadata to store against the resource. + * + * @var array|null + * + * @example {"cluster": "eu-west"} + */ 'meta' => ['nullable', 'array'], ]; } diff --git a/src/Http/Middleware/ForceJsonResponse.php b/src/Http/Middleware/ForceJsonResponse.php new file mode 100644 index 00000000..5d470c00 --- /dev/null +++ b/src/Http/Middleware/ForceJsonResponse.php @@ -0,0 +1,20 @@ +headers->set('Accept', 'application/json'); + + return $next($request); + } +} diff --git a/tests/Architecture/DataTest.php b/tests/Architecture/DataTest.php index c2aaa720..f193d60f 100644 --- a/tests/Architecture/DataTest.php +++ b/tests/Architecture/DataTest.php @@ -1,12 +1,14 @@ expect('Cachet\Data') ->toBeClasses() ->toExtend(BaseData::class) + ->ignoring(FlexibleDateTimeCast::class) ->toBeFinal() ->ignoring(BaseData::class); diff --git a/tests/Feature/Api/MetricTest.php b/tests/Feature/Api/MetricTest.php index 088bee0b..3189fff1 100644 --- a/tests/Feature/Api/MetricTest.php +++ b/tests/Feature/Api/MetricTest.php @@ -192,6 +192,39 @@ ]); }); +it('can create a metric with a calc type, display chart and places', function () { + Sanctum::actingAs(User::factory()->create(), ['metrics.manage']); + + $response = postJson('/status/api/metrics', [ + 'name' => 'New Metric', + 'suffix' => 'cups of tea', + 'calc_type' => 1, + 'display_chart' => true, + 'places' => 3, + ]); + + $response->assertCreated(); + $this->assertDatabaseHas('metrics', [ + 'name' => 'New Metric', + 'calc_type' => 1, + 'display_chart' => true, + 'places' => 3, + ]); +}); + +it('cannot create a metric with invalid places', function () { + Sanctum::actingAs(User::factory()->create(), ['metrics.manage']); + + $response = postJson('/status/api/metrics', [ + 'name' => 'New Metric', + 'suffix' => 'cups of tea', + 'places' => 10, + ]); + + $response->assertUnprocessable(); + $response->assertJsonValidationErrors(['places']); +}); + it('cannot update a metric if not authenticated', function () { $metric = Metric::factory()->create(); diff --git a/tests/Feature/Api/ScheduleTest.php b/tests/Feature/Api/ScheduleTest.php index 7bd5cf44..5f9ec561 100644 --- a/tests/Feature/Api/ScheduleTest.php +++ b/tests/Feature/Api/ScheduleTest.php @@ -322,6 +322,61 @@ $this->assertDatabaseCount('schedule_components', 0); }); +it('can create a schedule with ISO 8601 dates', function () { + Sanctum::actingAs(User::factory()->create(), ['schedules.manage']); + + $response = postJson('/status/api/schedules', [ + 'name' => 'New Scheduled Maintenance', + 'message' => 'Something will go wrong.', + 'scheduled_at' => '2033-11-07T05:31:56Z', + 'completed_at' => '2033-11-08T05:31:56Z', + ]); + + $response->assertCreated(); + $response->assertJson([ + 'data' => [ + 'attributes' => [ + 'scheduled' => [ + 'string' => '2033-11-07 05:31:56', + ], + 'completed' => [ + 'string' => '2033-11-08 05:31:56', + ], + ], + ], + ]); +}); + +it('responds with a JSON validation error when the Accept header is not set', function () { + Sanctum::actingAs(User::factory()->create(), ['schedules.manage']); + + $response = $this->post('/status/api/schedules', [ + 'name' => 'Missing Message and Scheduled At', + ]); + + $response->assertUnprocessable(); + $response->assertHeader('Content-Type', 'application/json'); + $response->assertJsonValidationErrors(['message', 'scheduled_at']); +}); + +it('cannot create a schedule with a component missing a status', function () { + Sanctum::actingAs(User::factory()->create(), ['schedules.manage']); + + $component = Component::factory()->create(); + + $response = postJson('/status/api/schedules', [ + 'name' => 'New Scheduled Maintenance', + 'message' => 'Something will go wrong.', + 'scheduled_at' => now()->addWeek()->toDateTimeString(), + 'components' => [ + ['id' => $component->id], + ], + ]); + + $response->assertUnprocessable(); + $response->assertJsonValidationErrors(['components.0.status']); +}); + it('can create a schedule with components', function () { Sanctum::actingAs(User::factory()->create(), ['schedules.manage']);