add VAA stand reservation plan payload validation and JSON schema - #1877
Open
MrAdder wants to merge 23 commits into
Open
add VAA stand reservation plan payload validation and JSON schema#1877MrAdder wants to merge 23 commits into
MrAdder wants to merge 23 commits into
Conversation
Introduce an endpoint to accept VAA stand reservation plan submissions. Adds StandReservationPlanController@store, a FormRequest (StoreStandReservationPlan) that restricts submissions to VAA users, and a comprehensive validation rule (StandReservationPlanPayload) which enforces strict JSON schema, Zulu timestamps, CID/airport/stand rules, and rejects overlapping reservations for the same stand. Registers the route POST /stand/reservation-plan, adds documentation (docs/guides/VaaStandReservationPlans.md) and links it from the StandAllocation guide, and includes controller tests to cover successful submission, authorization, overlap detection, and multi-airport validation. On success the API creates a plan with status=submitted and returns 201 with id and status.
Breaks the large invokable validation into smaller private methods for clarity and testability. Adds validateEventTimes, extractReservations, collectStandIntervals, validateReservationAndBuildInterval and resolveStandKey to parse/validate event times, extract reservations, build per-stand intervals and resolve stand keys. Changes reservations validation to return early when invalid, collects intervalsByStand and then validates overlaps. Preserves existing validation messages while improving structure and null/edge-case handling.
Rework validation control flow to avoid early returns and consolidate result construction. Introduces an $interval result that's only returned when stand, from and to are valid; resolveStandKey no longer returns immediately on errors but sets and returns a nullable stand key. event_airport(s) handling was simplified to build an airports array, validate each entry (reporting per-item errors) and check duplicates at the end. Tighten and harden parseZuluTime to more robustly validate and parse Zulu timestamps. Overall this improves error accumulation and readability of the validation logic.
Extract validation logic from the main rule into small private helper methods to improve readability and testability. Added validateSingleStandMode, validateReservationTimes, resolveStandKeyFromId, resolveStandKeyFromIdentifier, validateSingleEventAirport and validateMultipleEventAirports. These methods encapsulate existing checks for stand selection, Zulu time parsing and ordering, stand key resolution (by id or code+airport) and event_airport(s) normalization/validation. No validation messages or behaviour were changed; logic was reorganized and returned values made explicit. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com>
MrAdder
marked this pull request as ready for review
April 17, 2026 15:36
CLC0609
reviewed
Apr 17, 2026
CLC0609
left a comment
CLC0609
left a comment
Contributor
There was a problem hiding this comment.
I've had a brief look through and the schema and validation looks good.
Can we just focus this PR on that, and leave the controllers and API endpoints for another PR please?
Contributor
Author
|
Sure will have that sorted in a few |
Delete the StandReservationPlan submission endpoint and related code: removes the controller (app/Http/Controllers/StandReservationPlanController.php), request validation (app/Http/Requests/Stand/StoreStandReservationPlan.php), and tests (tests/app/Http/Controllers/StandReservationPlanControllerTest.php). Also unregisters the POST stand/reservation-plan route in routes/api.php, removing the API for submitting stand reservation plans.
CLC0609
requested changes
Apr 17, 2026
CLC0609
left a comment
CLC0609
left a comment
Contributor
There was a problem hiding this comment.
Looking good, couple of nit picky things from me!
Co-authored-by: Coby Chapman <cobylc.121@gmail.com>
Co-authored-by: Coby Chapman <cobylc.121@gmail.com>
Co-authored-by: Coby Chapman <cobylc.121@gmail.com>
Co-authored-by: Coby Chapman <cobylc.121@gmail.com>
Co-authored-by: Coby Chapman <cobylc.121@gmail.com>
Tighten validation for stand reservation payloads: require stand_id to be a positive integer, restrict airport ICAO codes to UK prefixes (EG|EI) and update related error messages. Remove an unused isPositiveInteger helper and adjust airport normalization/validation logic. Update docs to clarify single vs multi-airport wording, event window requirements, and reservation rules. Add comprehensive unit tests covering valid/invalid UK ICAOs, overlapping reservations, out-of-window times, unknown fields, and negative stand IDs.
Contributor
|
Let's also create a JSON schema for the format - makes it much easier to catch errors right away, even with the end user |
Contributor
Author
|
Sure will do that in a little bit after dinner |
Add a JSON Schema for VAA stand reservation plans at docs/guides/schemas/vaa-stand-reservation-plan.schema.json and update the guide to reference it. The schema validates payload structure, field types, ISO 8601 Zulu timestamps, CID ranges, UK ICAO formats, and stand reference modes. The doc clarifies time formatting and lists server-side checks that JSON Schema cannot enforce (event_end > event_start, reservation timeto > timefrom, reservations within the event window, and no overlapping reservations for the same stand).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a JSON schema, server-side validation rule, and accompanying documentation/tests for VAA stand reservation plan submissions, aligning with issue #1847.
Changes:
- Added
StandReservationPlanPayloadLaravelInvokableRuleto validate stand reservation plan payloads (timestamps, airport scope, reservation windows, and overlap detection). - Added a Draft 2020-12 JSON Schema for VAA stand reservation plan submissions.
- Added a public guide and unit tests covering key acceptance/rejection scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
app/Rules/Stand/StandReservationPlanPayload.php |
Implements payload validation (structure, time parsing, airport scope, reservation constraints, overlap checks). |
tests/app/Rules/Stand/StandReservationPlanPayloadTest.php |
Adds unit tests for the new validation rule. |
docs/guides/schemas/vaa-stand-reservation-plan.schema.json |
Defines the JSON schema for pre-validation of submissions. |
docs/guides/VaaStandReservationPlans.md |
Public-facing guidance for constructing valid JSON payloads. |
docs/guides/StandAllocation.md |
Adds a pointer to the new VAA reservation plan guide. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Can look into those after this lecture
This email and any files transmitted with it are confidential and contain
privileged or copyright information. It is intended solely for the
individual(s) to whom it is addressed. You must not present this message to
another party without gaining permission from the sender. If you are not
the intended recipient you must not copy, distribute or use this email or
the information contained in it for any purpose other than to notify us.
If you have received this message in error, please notify the sender
immediately, and delete this email from your system. We do not guarantee
that this material is free from viruses or any other defects although due
care has been taken to minimise the risk.
…On Mon, 20 Apr 2026 at 12:22, Copilot ***@***.***> wrote:
***@***.**** commented on this pull request.
Pull request overview
This PR introduces a JSON schema, server-side validation rule, and
accompanying documentation/tests for VAA stand reservation plan
submissions, aligning with issue #1847
<#1847>.
*Changes:*
- Added StandReservationPlanPayload Laravel InvokableRule to validate
stand reservation plan payloads (timestamps, airport scope, reservation
windows, and overlap detection).
- Added a Draft 2020-12 JSON Schema for VAA stand reservation plan
submissions.
- Added a public guide and unit tests covering key
acceptance/rejection scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and
generated 4 comments.
Show a summary per file
File Description
app/Rules/Stand/StandReservationPlanPayload.php Implements payload
validation (structure, time parsing, airport scope, reservation
constraints, overlap checks).
tests/app/Rules/Stand/StandReservationPlanPayloadTest.php Adds unit tests
for the new validation rule.
docs/guides/schemas/vaa-stand-reservation-plan.schema.json Defines the
JSON schema for pre-validation of submissions.
docs/guides/VaaStandReservationPlans.md Public-facing guidance for
constructing valid JSON payloads.
docs/guides/StandAllocation.md Adds a pointer to the new VAA reservation
plan guide.
------------------------------
💡 Add Copilot custom instructions
<http:///VATSIM-UK/uk-controller-api/new/main?filename=.github/instructions/*.instructions.md>
for smarter, more guided reviews. Learn how to get started
<https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot>
.
------------------------------
In app/Rules/Stand/StandReservationPlanPayload.php
<#1877?email_source=notifications&email_token=AA4XK7AFKUIY3XRGAJCMAL34WYCATA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJTHE2DQMJQGMYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K64DSL5ZGK5TJMV3V6Y3MNFRWW#discussion_r3110242519>
:
> + * @PARAM Closure(string): \Illuminate\Translation\PotentiallyTranslatedString $fail
+ * @return array{0:?CarbonImmutable,1:?CarbonImmutable}
+ */
+ private function validateEventTimes(string $attribute, array $value, Closure $fail): array
+ {
+ $eventStart = $this->parseZuluTime($value['event_start'] ?? null);
+ if (!$eventStart) {
+ $fail("$attribute.event_start must be a Zulu timestamp in format YYYY-MM-DDTHH:MM:SSZ.");
+ }
+
+ $eventEnd = $this->parseZuluTime($value['event_end'] ?? null);
+ if (!$eventEnd) {
+ $fail("$attribute.event_end must be a Zulu timestamp in format YYYY-MM-DDTHH:MM:SSZ.");
+ }
+
+ if (!$eventEnd->isAfter($eventStart)) {
$eventEnd->isAfter($eventStart) will throw if either timestamp fails to
parse (both variables can be null). Please guard this comparison (only run
when both are non-null) so invalid event_start/event_end yields
validation errors instead of a fatal error.
⬇️ Suggested change
- if (!$eventEnd->isAfter($eventStart)) {
+ if ($eventStart && $eventEnd && !$eventEnd->isAfter($eventStart)) {
------------------------------
In app/Rules/Stand/StandReservationPlanPayload.php
<#1877?email_source=notifications&email_token=AA4XK7AFKUIY3XRGAJCMAL34WYCATA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJTHE2DQMJQGMYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K64DSL5ZGK5TJMV3V6Y3MNFRWW#discussion_r3110242572>
:
> + $resolvedAirport = $this->normalizeAirportCode($reservation['airport'] ?? null);
+ if (is_null($resolvedAirport) && count($eventAirports) === 1) {
+ $resolvedAirport = $eventAirports[0];
+ }
+
When stand mode is used, the resolved airport (explicit or inferred) is
not validated against the event’s airport scope (event_airport/
event_airports). As a result, a reservation can target an airport that
isn’t part of the event. After resolving $resolvedAirport, ensure it is
one of $eventAirports and fail validation if not.
------------------------------
In docs/guides/VaaStandReservationPlans.md
<#1877?email_source=notifications&email_token=AA4XK7C4BUPKGKVZTELC76T4WYCAVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJTHE2DQMJQGMYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K64DSL5ZGK5TJMV3V6Y3MNFRWW#discussion_r3110242597>
:
> +
+Use exactly one of the following:
+
+- `event_airport` (string, required if single-airport event): 4-letter ICAO.
+- `event_airports` (array of strings, required if multi-airport event): non-empty, unique 4-letter ICAOs.
+- `reservations` (array, required): One or more reservation objects. Multiple stands can be included in one plan, and the same stand can be reused at different times as long as the time windows do not overlap.
This section says “Use exactly one of the following:” but includes
reservations, which is required in both cases and not mutually exclusive
with event_airport/event_airports. Please split reservations into its own
required bullet and keep the mutual exclusivity statement only for the
airport scope fields.
⬇️ Suggested change
-
-Use exactly one of the following:
-
-- `event_airport` (string, required if single-airport event): 4-letter ICAO.
-- `event_airports` (array of strings, required if multi-airport event): non-empty, unique 4-letter ICAOs.
-- `reservations` (array, required): One or more reservation objects. Multiple stands can be included in one plan, and the same stand can be reused at different times as long as the time windows do not overlap.
+- `reservations` (array, required): One or more reservation objects. Multiple stands can be included in one plan, and the same stand can be reused at different times as long as the time windows do not overlap.
+
+Use exactly one of the following airport scope fields:
+
+- `event_airport` (string, required if single-airport event): 4-letter ICAO.
+- `event_airports` (array of strings, required if multi-airport event): non-empty, unique 4-letter ICAOs.
------------------------------
In tests/app/Rules/Stand/StandReservationPlanPayloadTest.php
<#1877?email_source=notifications&email_token=AA4XK7C4BUPKGKVZTELC76T4WYCAVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJTHE2DQMJQGMYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K64DSL5ZGK5TJMV3V6Y3MNFRWW#discussion_r3110242621>
:
> + 'reservations' => [
+ [
+ 'stand_id' => 1201,
+ 'cid' => 1203533,
+ 'timefrom' => '2026-06-12T09:00:00Z',
+ 'timeto' => '2026-06-12T10:00:00Z',
+ ],
+ ],
+ ];
+ }
+
+ public function testItAcceptsAValidStandIdPlan(): void
+ {
+ $this->assertTrue($this->validatePayload($this->validSingleAirportPayload()));
+ }
+
The rule validates additional behaviors that aren’t currently covered by
tests (e.g. rejecting event_end <= event_start, rejecting reservation
timeto <= timefrom, and requiring exactly one of stand_id/stand). Adding
explicit tests for these cases will prevent regressions (especially around
the event time ordering which is easy to break).
⬇️ Suggested change
-
+
+ public function testItRejectsAPlanWhereEventEndIsNotAfterEventStart(): void
+ {
+ $payload = $this->validSingleAirportPayload();
+ $payload['event_end'] = $payload['event_start'];
+
+ $this->assertFalse($this->validatePayload($payload));
+ }
+
+ public function testItRejectsAReservationWhereTimeToIsNotAfterTimeFrom(): void
+ {
+ $payload = $this->validSingleAirportPayload();
+ $payload['reservations'][0]['timeto'] = $payload['reservations'][0]['timefrom'];
+
+ $this->assertFalse($this->validatePayload($payload));
+ }
+
+ public function testItRejectsAReservationWhenBothStandIdAndStandAreProvided(): void
+ {
+ $payload = $this->validSingleAirportPayload();
+ $payload['reservations'][0]['stand'] = 'A23';
+
+ $this->assertFalse($this->validatePayload($payload));
+ }
+
+ public function testItRejectsAReservationWhenNeitherStandIdNorStandIsProvided(): void
+ {
+ $payload = $this->validSingleAirportPayload();
+ unset($payload['reservations'][0]['stand_id']);
+
+ $this->assertFalse($this->validatePayload($payload));
+ }
—
Reply to this email directly, view it on GitHub
<#1877?email_source=notifications&email_token=AA4XK7HPUENDXJGZ25ARZ3D4WYCAVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJTHE2DQMJQGMYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2L24DSL5ZGK5TJMV3V63TPORUWM2LDMF2GS33OONPWG3DJMNVQ#pullrequestreview-4139481030>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4XK7G7O6SFPTX2RL3YVOL4WYCAVAVCNFSM6AAAAACX44E7O6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCMZZGQ4DCMBTGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add a guard to StandReservationPlanPayload to ensure the resolved airport exists in the event's airports. If the airport is not found, the rule triggers a failure with "$itemPath.airport must be one of the event's airports." and returns null. Uses a strict in_array check to prevent invalid airport values.
Consolidate two separate validations into a single conditional: if the resolved airport is null or not in the event's airports, fail with an appropriate message chosen via a conditional expression. This reduces duplicated fail/return branches and clarifies the validation logic in StandReservationPlanPayload.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Require reservations to be a non-empty list (using array_is_list) and update the validation message accordingly. Add an early return when event_airports is an empty array to avoid emitting reservation airport mismatch errors for invalid event_airports. Include tests for missing airport on multi-airport events, airport not in event_airports, ignoring airport-mismatch when event_airports is invalid/empty, and rejecting reservations that are not a list.
Rework conditional flow for airport resolution/validation in StandReservationPlanPayload: combine the event airports check with the airport normalization/validation block, move the successful return inside that block, and unify the final return to null. Preserves existing failure behavior (calls $fail when airport is missing or not one of the event's airports) while simplifying control flow and reducing early returns.
|
Refactors stand-airport resolution for reservation stand identifiers so invalid airport values are reported correctly instead of being treated as missing. Single-airport events still infer a missing reservation airport, while multi-airport events require it explicitly. Also updates ICAO validation/error messaging to clearly require UK or Irish codes (`EG`/`EI`) with examples, and aligns docs/tests with the new behavior.
Simplifies stand reference generation and airport resolution by collapsing early returns into expression-based flow and extracting missing-airport inference into a dedicated helper. This keeps validation behavior consistent while making the control flow easier to follow, including explicit nulling when a provided airport is not in the event airport list.
MrAdder
changed the title
Extend `StandReservationPlanPayloadTest` with negative cases for unparseable `event_start` and `event_end` values, including when one or both timestamps are invalid. This strengthens coverage around date parsing validation for stand reservation plan payloads.
|
Contributor
Author
|
Sorry for the ReRequest @CLC0609 I been away and busy with Uni, resolved all issues that copilot brought up and what you both brought up on the issues |
Contributor
Author
|
The Root cause — a range mismatch between the Hold factory and the form validation:
This specific test builds a hold with That's 5 failing values out of 1001 possible, so roughly a 0.5% failure rate per run — Not from this branch.
This PR changes five files — A re-run should go green. The fix, whenever someone picks it up, is to align the factory |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defined the JSON schema for VAA stand plans and wrote the server side validation against it.
Wrote the public facing documentation that can be used to guide VAAs to make the JSON payloads
Closes #1847