Skip to content

feat: add workflow to validate OpenAPI schemas in reference/rest-api.json#64

Open
treyhyde wants to merge 3 commits into
v2from
debt/readme-action
Open

feat: add workflow to validate OpenAPI schemas in reference/rest-api.json#64
treyhyde wants to merge 3 commits into
v2from
debt/readme-action

Conversation

@treyhyde
Copy link
Copy Markdown
Member

@treyhyde treyhyde commented May 8, 2026

No description provided.

@treyhyde treyhyde changed the base branch from main to v2 May 8, 2026 17:49
@treyhyde treyhyde requested review from a team as code owners May 8, 2026 17:49
Copilot AI review requested due to automatic review settings May 8, 2026 19:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds automation and guidance to keep the OpenAPI spec (reference/rest-api.json) valid and consistent during changes.

Changes:

  • Introduces a GitHub Actions workflow that validates reference/rest-api.json on PRs and pushes to v2.
  • Adds a Copilot skill document describing a process for syncing form-related schemas from upstream repos.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/validate-openapi.yml Adds CI validation for JSON syntax, RAW_BODY checks, external $ref checks, and rdme openapi validate on relevant changes.
.github/skills/sync-form-schema/SKILL.md Adds a documented “skill” workflow for syncing form-related OpenAPI schemas from upstream sources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +29 to +48
All schemas live inline in `reference/rest-api.json` under `components.schemas`. The ones that
map to the form model are:

| OpenAPI Schema | Maps From |
|----------------|-----------|
| `FormBaseElement` | Common base properties on all elements (`key`, `data_name`, `label`, `required`, `hidden`, `disabled`, conditions, etc.) |
| `FormElement` | `oneOf` discriminated union — must list ALL element types from `data-elements.js` |
| `FormTextFieldElement` | `TextField` — numeric, format, min, max, pattern, min_length, max_length |
| `FormChoiceFieldElement` | `ChoiceField` — choices, choice_list_id, multiple, allow_other |
| `FormClassificationFieldElement` | `ClassificationField` — classification_set_id, classification_set_schema |
| `FormYesNoFieldElement` | `YesNoField` — positive, negative, neutral, neutral_enabled |
| `FormPhotoFieldElement` | `PhotoField` — min_length, max_length |
| `FormVideoFieldElement` | `VideoField` — track_enabled, audio_enabled |
| `FormAudioFieldElement` | `AudioField` |
| `FormSignatureFieldElement` | `SignatureField` — agreement_text |
| `FormBarcodeFieldElement` | `BarcodeField` |
| `FormDateTimeFieldElement` | `DateTimeField` |
| `FormDateFieldElement` | `DateField` |
| `FormTimeFieldElement` | `TimeField` |
| `FormAddressFieldElement` | `AddressField` — auto_populate |
## Schema Conventions

- **Naming**: `Form{TypeName}Element` for element schemas (PascalCase matching the type string)
- **Structure**: All typed elements use `allOf: [{ $ref: FormBaseElement }, { type: object, properties: { type: { enum: ["TypeName"] }, ...extra props } }]`
pedrodotavila
pedrodotavila previously approved these changes May 8, 2026
Copy link
Copy Markdown

@pedrodotavila pedrodotavila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: Good DevOps addition. The validation workflow will catch common OpenAPI spec issues (JSON syntax, RAW_BODY anti-patterns, external $ref paths). All CI checks passed.

All schemas live inline in `reference/rest-api.json` under `components.schemas`. The ones that
map to the form model are:

| OpenAPI Schema | Maps From |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part concerns me. We state that the source of truth is in fulcrum-schema. But then we show this mapping. I am assuming that they are currently in sync. But now we have two different lists to maintain. I can see the LLM ignore a field type that has been deleted or ignoring fulcrum-schema because we already have a mapping here.

I would try just telling the LLM to check fulcrum-schema and describe what it should be looking for.

| `fulcrumapp/fulcrum-schema` | `src/data-elements.js` | Complete list of element `type` enum values |
| `fulcrumapp/fulcrum-schema` | `src/schema.js` | SQL column definitions per element — reveals what fields each type stores |
| `fulcrumapp/fulcrum-schema` | `test/fixtures/form-v5.json` | Real form fixture — definitive JSON shape for all element types |
| `fulcrumapp/fulcrum-core` | `src/form.js` | Form-level attributes (geometry_types, script, projects_enabled, etc.) |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is fulcrum-core really the source of truth? We have to keep that in sync with the actual API.

@pedrodotavila pedrodotavila dismissed their stale review May 8, 2026 20:42

Dismissing automated approval per user request.

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.

4 participants