Skip to content

Make incident status optional in create validation#358

Open
jbrooksuk wants to merge 1 commit into
mainfrom
fix/incident-status-not-required
Open

Make incident status optional in create validation#358
jbrooksuk wants to merge 1 commit into
mainfrom
fix/incident-status-not-required

Conversation

@jbrooksuk

Copy link
Copy Markdown
Member

Problem

The nightly test run started failing intermittently (run 27315738081) with:

FAILED  Tests\Unit\Actions\Incident\CreateIncidentTest  ValidationException
The status field is required.

at the two tests that create an incident without a status (CreateIncidentTest.php:17 and :99). It reproduced only under the full parallel suite, not in isolation.

Root cause

CreateIncidentRequestData::rules() marked status as required, but status is optional everywhere else:

  • the constructor defaults it to null (?IncidentStatusEnum $status = null)
  • the incidents.status column is nullable (2023_08_21_..._make_incident_columns_nullable)
  • UpdateIncidentRequestData treats it as optional
  • a null status is a valid, supported state (rendered as "reported")

The workbench config sets validation_strategy: Always, so from() validates on every call, and spatie/laravel-data's structure cache is shared across the 10 parallel test workers via the file store. Once the status => required rule was resolved/cached from a status-bearing payload, it could be applied to the status-less unit-test payloads — surfacing the failure intermittently depending on worker assignment and ordering.

Fix

Make status nullable instead of required, aligning the create rules with the constructor, the column, and the update request. Status-less incident creation now validates deterministically.

Verification

Full suite green in parallel on both dependency sets:

  • --prefer-lowest (spatie/laravel-data 4.11): 408 passed
  • --prefer-stable (spatie/laravel-data 4.23): 408 passed

🤖 Generated with Claude Code

The CreateIncidentRequestData validation rules marked `status` as
`required`, but the field is optional everywhere else: the constructor
defaults it to null, the `incidents.status` column is nullable, and
UpdateIncidentRequestData treats it as optional. Incidents created
without a status are a valid state (rendered as "reported").

With the workbench's `validation_strategy: Always` and spatie/laravel-data
structure caching shared across parallel test workers, the `required`
rule could intermittently apply to status-less payloads, failing
CreateIncidentTest with "The status field is required." in CI.

Aligns the rule with the rest of the codebase by making status nullable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant