ENG-3083: AccessPolicy + AccessPolicyVersion models and migration

[thabofletcher]

Ticket ENG-3083

Description Of Changes

Adds the data model and Alembic migration for the PBAC Access Policy feature. This is the OSS-side foundation that the fidesplus API layer builds on.

Two-table design for transparent policy versioning:

• plus_access_policy — Stable entity the FE interacts with (name, description, controls, enabled, soft-delete flag)
• plus_access_policy_version — Immutable version rows. Each policy update creates a new version row rather than overwriting, preserving full audit history for compliance

The FE (merged in #7725 by @lucanovera) never sees version IDs directly — it uses the stable access_policy.id for all CRUD. Versioning happens transparently on the backend.

Code Changes

• New model file src/fides/api/models/access_policy.py with AccessPolicy and AccessPolicyVersion
• Alembic migration a8f3b2c1d5e7 creating both tables with indexes and constraints
• Registered both models in sql_model_map in sql_models.py

Steps to Confirm

1. Run alembic upgrade head — both tables should be created
2. Verify plus_access_policy has: id, name, description, controls, enabled, is_deleted, created_at, updated_at
3. Verify plus_access_policy_version has: id, access_policy_id (FK), version, yaml, created_at, updated_at
4. Verify unique constraint on (access_policy_id, version) prevents duplicate versions
5. Run alembic downgrade -1 — both tables should be dropped cleanly

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration label to the entry
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • No UX review needed
• Database migrations:
  • Ensure that your downrev is up to date with the latest revision on main
  • Ensure that your downgrade() migration is correct and works
  • No migrations
• Documentation:
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Mar 28, 2026 0:37am
fides-privacy-center	Ignored		Mar 28, 2026 0:37am

[claude]

Code Review — ENG-3083: AccessPolicy + AccessPolicyVersion models and migration

The two-table design is clean and the intent (stable entity ID + immutable version rows) is well-documented. The migration covers the full round-trip (upgrade + downgrade) and the FK/cascade is correct. A few things worth addressing before merge:

Suggestions

1. Migration docstring vs down_revision mismatch — the Revises: comment says c7e3a9b1d4f2 but down_revision = "29113e44faec". Functionally harmless since Alembic reads the variable, but the stale comment is confusing. (line 13 in the migration)

2. Redundant index on access_policy_id — ix_plus_access_policy_version_access_policy_id duplicates coverage already provided by the composite unique index (access_policy_id, version). PostgreSQL can use a composite index for prefix-column lookups, so the single-column index is pure overhead. (lines ~83 in the migration; index=True on the column in the model)

3. Missing Python-side default for enabled / is_deleted — server_default only applies after a DB round-trip. A freshly constructed but un-flushed AccessPolicy will have enabled = None. Pairing with default=True / default=False matches the pattern used elsewhere in the codebase (e.g. privacy_experience.py).

4. lazy="selectin" on versions — loads all version rows on every AccessPolicy fetch. If policies accumulate many versions over time, this is an N-row load where only 1 row is typically needed (latest_version). Worth confirming this is intentional given the append-only nature of the versions table.

5. No auto-increment logic for version — default=1 means every new AccessPolicyVersion starts at version 1. The unique constraint prevents duplicates, but the service layer must compute MAX(version) + 1 inside a transaction. Just make sure the future service implementation does this atomically.

6. controls column intent — the field is an unbounded nullable String with no docstring. A brief comment explaining what it stores (and whether it might eventually need a structured type like JSONB) would help future readers.

Nice to Have

• No model-level tests are included. Even basic integration tests (create policy → create version → verify FK cascade on delete) would guard against migration drift in the future.

Overall the foundation looks solid — the issues above are all relatively small and most are in the suggestion/nice-to-have range.

[claude]

version = Column(Integer, nullable=False, default=1) — every new AccessPolicyVersion instance will default to version 1. The unique constraint on (access_policy_id, version) will correctly reject duplicates at the DB level, but the service layer is entirely responsible for computing the next version number. There's no helper or guard here to make that easy to get right.

This is fine as a pure model layer, but worth a note: whichever service creates a new version will need to SELECT MAX(version) + 1 (ideally inside the same transaction) to avoid a race. Just flagging so the service implementation doesn't assume this is automatic.

[thabofletcher]

The service layer handles it correctly. What is the suggestion you are making for this code review @claude ?

[thabofletcher]

@claude The associated PR includes the tests you are suggesting. What are you suggesting for this PR for testing? I'm not sure what you are suggesting for integration tests on a standalone model PR that needs to be merged before the service layer will work. Please explain