Closes: #390 - Schema validation/apply endpoints

[bctiemann]

Closes: #390

Adds two new API endpoints for previewing and applying COT schema documents.

New endpoints

Method	URL	Description
POST	/api/plugins/custom-objects/schema/preview/	Diffs a schema document against live DB state; returns structured diffs without any DB changes
POST	/api/plugins/custom-objects/schema/apply/	Applies a schema document atomically; returns the applied diffs

---

POST /schema/preview/

Request body — a raw COT schema document:

{
  "schema_version": "1",
  "types": [ ... ]
}

Response 200:

{
  "diffs": [
    {
      "slug": "my-cot",
      "name": "my_cot",
      "is_new": false,
      "has_changes": true,
      "has_destructive_changes": false,
      "cot_changes": { "description": ["old", "new"] },
      "field_changes": [
        {
          "op": "add",
          "schema_id": 5,
          "db_name": null,
          "schema_def": { "id": 5, "name": "new_field", "type": "text" }
        }
      ],
      "warnings": []
    }
  ]
}

Preview never modifies the database and never returns 409 — destructive changes are flagged via has_destructive_changes: true so the caller can decide whether to proceed.

---

POST /schema/apply/

Request body — schema document wrapped with an optional allow_destructive flag:

{
  "allow_destructive": false,
  "schema": {
    "schema_version": "1",
    "types": [ ... ]
  }
}

Response 200 (success):

{
  "applied": true,
  "diffs": [ ... ]
}

Response 409 Conflict — document contains REMOVE operations and allow_destructive was not set:

{
  "error": "destructive_changes",
  "detail": "Schema contains destructive field removals for COT(s): my-cot. Pass allow_destructive=True to apply them.",
  "destructive_slugs": ["my-cot"]
}

Response 400 Bad Request — circular COT dependency, unresolvable FK / choice-set reference, missing schema key, or invalid schema document structure:

{
  "error": "circular_dependency" | "unresolvable_reference",
  "detail": "..."
}

---

Implementation notes

• Both views are backed by the comparator (diff_document) and executor (apply_document) from issues Custom Object Type state comparator functionality #387 and Custom Object Type upgrade executor #389.
• Input is validated against cot_schema_v1.json using jsonschema.Draft202012Validator before any DB access. Errors return 400 with {"schema_errors": [{"path": [...], "message": "..."}]}. Degrades gracefully if jsonschema is not installed.
• Both views use _ignore_model_permissions = True; authentication is still enforced.
• Apply is fully atomic — any error rolls back all changes including newly created COT tables.
• schema_document is persisted on each affected COT after a successful apply, so tombstones are available for future export/diff cycles.

Tests — tests/test_schema_api.py (24 tests)

SchemaPreviewTestCase (10): 200 response, diff keys present, no DB mutation, field-add/alter detection, destructive preview returns 200 (not 409), auth enforcement, invalid schema returns 400.

SchemaApplyTestCase (14): new COT creation, field add, 409 guard without flag, 200 with allow_destructive, missing/non-dict schema key returns 400, unresolvable FK/choice-set returns 400, auth enforcement, schema_document persisted.

[bctiemann]

@claude /review

[claude]

Claude finished @bctiemann's task in 1m 46s —— View job

---

Code Review: Schema validation/apply endpoints (#390)

• Gather context (read changed files)
• Analyze implementation
• Review tests
• Post review feedback

---

Overview

This PR adds two REST API endpoints (POST /schema/preview/ and POST /schema/apply/) that wrap the existing comparator and executor machinery in well-structured API views. The implementation is clean and the test coverage is solid. A few observations below.

---

api/views.py

Minor: overly broad exception handler at module level

views.py:39:

except Exception as exc:  # jsonschema not installed or schema file missing

This silently swallows any error during module-level setup — including JSON parse errors in cot_schema_v1.json itself (e.g. a malformed schema file after a bad deploy). Consider narrowing to (ImportError, OSError, json.JSONDecodeError) so structural problems surface immediately rather than degrading silently. Fix this →

Nit: redundant variable assignment

views.py:366-369:

raw_destructive = request.data.get("allow_destructive", False)
if not isinstance(raw_destructive, bool):
    raise ValidationError(...)
allow_destructive = raw_destructive   # ← same as raw_destructive at this point

allow_destructive could just be raw_destructive after the guard, or skip the intermediate name entirely.

Minor: apply_document can raise uncaught exceptions

views.py:378-398: The try/except block catches the four domain-specific errors well. However, unexpected DB errors (e.g. IntegrityError from a constraint violation not related to the COT schema logic) will surface as 500 responses. Given the view is transaction.atomic-backed inside apply_document, this is an acceptable tradeoff, but worth being intentional about — either document it or add a broad catch-and-400/500 for unexpected errors.

Question: is_in_branch check

CustomObjectViewSet.create/update checks is_in_branch() before mutating the DB. Should SchemaApplyView.post() do the same? Applying a full schema document while in a branch context seems like it could cause issues. Fix this →

Minor: delayed imports could be documented

views.py:302 and views.py:357-364: # noqa: PLC0415 suppresses the "import not at top of file" linting warning. A brief comment explaining why (circular import avoidance) would help future readers — similar to how the executor's own docstring explains the two-phase approach.

---

schema/executor.py change

The one-line change is correct and well-motivated:

- if diff.is_new or diff.has_changes:
+ if diff.is_new or diff.has_changes or not cot.schema_document:

This ensures schema_document is populated on the first apply of a COT that was created manually (with no prior schema document), which is required for future export/diff cycles. Good defensive fix.

---

tests/schema/test_schema_api.py

Good coverage overall — 24 tests covering the happy paths, error conditions, auth enforcement, and atomicity guard. A few gaps:

Missing: test for allow_destructive with non-boolean value

There's no test asserting that allow_destructive: "true" (a string) returns 400. The validation at views.py:367-368 handles it, but it's untested. Fix this →

Missing: test for noop apply (no changes)

test_preview_noop_has_no_changes covers preview correctly, but there's no parallel test_apply_noop_returns_200 that verifies a no-change apply also returns 200 with applied: True.

Fragile schema_id derivation in field-add tests

test_schema_api.py:108:

next_id = self.cot.next_schema_id + 1

This is used in both preview and apply tests to construct a new field's id. next_schema_id is read after refresh_from_db() which is correct, but the pattern next_id + 1 would be wrong if next_schema_id already accounts for the next assignable id (i.e. if it's already incremented). If the semantics of next_schema_id ever change, these tests would silently produce wrong ids. Consider using cot.next_schema_id directly (without +1) or clarifying via a comment.

---

tests/base.py

The TransactionCleanupMixin and create_api_token additions are clean and correctly placed. The MRO chain _SchemaAPIBase -> TransactionCleanupMixin -> CustomObjectsTestCase -> TransactionTestCase is correct — cleanup runs before the TransactionTestCase DB flush.

---

Summary

Solid implementation with good test coverage. The main items worth addressing:

1. is_in_branch() guard on apply endpoint — potentially important for correctness
2. Narrow the except Exception at module level — defensive improvement
3. Add test for non-bool allow_destructive — fills a gap in input validation coverage
4. Noop apply test — symmetry with preview tests

[bctiemann]

Added recommended tests and addressed all code-level suggestions. Also added a guard against is_in_branch which might be removed later with full branching support.

[arthanson]

Blocked by #449