Skip to content

refactor: use enums instead of string comparison across the codebase - #689

Open
Santhu32144 wants to merge 1 commit into
fireform-core:development-approach-cfrom
Santhu32144:684-use-enums
Open

Santhu32144 wants to merge 1 commit into
fireform-core:development-approach-cfrom
Santhu32144:684-use-enums

Conversation

@Santhu32144

Copy link
Copy Markdown

Closes #684

What

  • Replaced the hard-coded status strings with the enums from app/api/schemas/enums.py across routes, services, workers, tasks, and schemas. This includes job status/type, extraction status, batch status, health status, and incident sort order.
  • Added JobType, HealthState, and IncidentSort to contracts/schemas/enums.yaml.
  • Updated common.yaml, system.yaml, and path/incidents.yaml to reference those enums instead of defining the same values again. This keeps each set of values in one place.
  • Updated the Literal["..."] status fields to use enum members, for example Literal[ExtractionStatus.completed]. The generated JSON schema remains the same.
  • Added a test to make sure every enum defined in enums.yaml also exists in enums.py. The existing generator sync check only covers the enums used by the incident contract, so health, job type, and incident sort were not covered before.

Wire format

  • All of these are str enums, so the response values are unchanged. Existing tests that check the JSON values continue to pass without any changes.

  • The generated OpenAPI schema is only more precise in a few places:

    • Health status now references HealthState.
    • ExtractionJobResponse.job_type now references JobType.
    • The incidents sort query parameter now references IncidentSort instead of using a regex pattern.
  • An invalid/unknown sort value still returns 422.

  • Job.status and Job.job_type still use str as their database column type. Only the Python-side defaults were changed, so no database migration is required.

Notes

  • On Python 3.11, using a str enum inside an f-string or %s log call displays it as JobStatus.failed instead of just failed. I checked the changed code and none of the replaced values are used in those situations.

  • I intentionally left "missing" in the extraction worker's result summary unchanged because it is not an extraction status.

  • I also left the enums that currently exist only in enums.py and are not yet present in enums.yaml:

    • InputType
    • DetectionStatus
    • TemplateStatus
    • TextAlign
    • TemplateFieldType

    These can be added to the contract in a follow-up if needed.

Testing

  • pytest tests/ — 559 passed (544 before + 15 new tests)
  • python3 scripts/generate_contract_models.py — sync check passes and incident_contract.py is unchanged
  • ruff check on the changed files — clean

Status values were compared and assigned as bare strings across routes,
services, workers, tasks and schemas, so a typo like "complete" passed
unchecked. Replace them with the enums in app/api/schemas/enums.py.

Add JobType, HealthState and IncidentSort to contracts/schemas/enums.yaml
and point the inline copies in common.yaml, system.yaml and
path/incidents.yaml at them, so each set of values is defined once.

Literal status fields now use enum members; the generated JSON schema is
unchanged. Job.status and Job.job_type keep their str column type, only
their defaults change, so no migration is needed.

Add a test that every enum in enums.yaml matches enums.py: the
generator's sync check only covers the enums the incident contract pulls
in, which left health, job type and sort order unguarded.

This branch has not been deployed

No deployments
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