Skip to content

fix: declare unique_constraint(:id) on audit_log pkey in changelog_changeset#50

Closed
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4589-add-unique-constraint-audit-pkey
Closed

fix: declare unique_constraint(:id) on audit_log pkey in changelog_changeset#50
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4589-add-unique-constraint-audit-pkey

Conversation

@palantir-valiot

Copy link
Copy Markdown

Description

Prevents Ecto.ConstraintError on audit_logs_pkey (or audit_log_pkey) during *_and_log/3 and log/5 when the Changelog (audit) insert hits a primary-key unique violation.

Root cause

changelog_changeset/1 (the single place that builds the audit row for all public logging functions) only did Changeset.cast/3. It never declared the pkey unique_constraint/3. Per Ecto, a missing unique_constraint/3 turns a DB constraint violation into a hard Ecto.ConstraintError instead of a validation error on the changeset. This matches the exact stack in OPS-4589:

(ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:435: EctoTrail.log_changes/5
(ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:315: anonymous fn/4 in EctoTrail.update_and_log/4

Fix (identical to the pattern already shipped for OPS-4583 / OPS-4582)

  • Add :id to @changelog_fields
  • Expose Changelog.table_name/0
  • Compute the pkey constraint name (overridable via config :ecto_trail, :audit_log_pkey_name)
  • Call Changeset.unique_constraint(:id, name: ...) inside changelog_changeset/1
  • Expose __changelog_changeset_for_test__/1 test seam
  • Add pure-unit constraint assertion (test/unit/ecto_trail_constraint_test.exs) + a @tag :db integration test
  • Guard DB setup in test_helper.exs so the pure unit test runs in envs without Postgres (ECTO_TRAIL_SKIP_DB=true)
  • mix deps.update benchee credo (within allowed ranges) per Palantir baseline
  • Bump to 1.0.4, concise CHANGELOG entry

Files changed:

  • lib/ecto_trail/ecto_trail.ex
  • lib/ecto_trail/changelog.ex
  • test/unit/ecto_trail_constraint_test.exs (new)
  • test/unit/ecto_trail_test.exs
  • test/test_helper.exs
  • mix.exs, CHANGELOG.md, mix.lock

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

TDD: wrote the failing pure-unit test first (saw UndefinedFunctionError for the test seam + missing constraint), implemented the declaration, observed green.

Ran in this agent pod (no live Postgres for the :db tagged test):

  • mix format --check-formatted
  • ECTO_TRAIL_SKIP_DB=true mix test test/unit/ecto_trail_constraint_test.exs → 1/1 green
  • Full non-DB test surface exercised the new describe block in ecto_trail_test.exs

The exact same diff (modulo issue number) was previously reviewed + merged for OPS-4583 on the same repo; the constraint declaration + test seam is now the org-standard pattern for this class of failure.

Test Configuration:

  • Elixir: 1.20.x (per container)
  • Ecto: 3.14.0
  • No DB required for the new unit constraint test

Checklist:

  • My code follows the style guidelines of this project (mix format)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (CHANGELOG)
  • My changes generate no new warnings (pre-existing redundant-clause warning on map_custom_ecto_type is untouched)
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes (the subset that can run without Postgres)
  • Bumped version and updated CHANGELOG before merge (per Palantir baseline)

Closes OPS-4589

…angeset (OPS-4589)

Prevents Ecto.ConstraintError on audit_logs_pkey (or audit_log_pkey)
during *_and_log and log paths when a pkey collision occurs on insert
of the Changelog entry.

- Add :id to @changelog_fields
- Compute pkey constraint name (overridable via :audit_log_pkey_name)
- Call unique_constraint/3 in changelog_changeset
- Expose table_name/0 and test helper
- Add pure unit test for the constraint declaration
- Upgrade benchee/credo (within ranges) per baseline
- Bump to 1.0.4, update CHANGELOG

TDD: wrote constraint test first (red until fix), then green.

Closes OPS-4589
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4589

@acrogenesis

Copy link
Copy Markdown
Member

Closing as a duplicate of #24 — same bug: Ecto.ConstraintError on audit_logs_pkey in EctoTrail.log_changes/5. These were generated from a backlog of duplicate Linear issues created by a log-agent dedup gap (now fixed in palantir 38438d6; no new duplicates are being filed). Consolidating on #24.

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