Skip to content

meta(core): add regression test for foreign key duplication on schema-qualified alter#18228

Open
emmayusufu wants to merge 1 commit into
sequelize:mainfrom
emmayusufu:test/12961-fk-duplication-schema
Open

meta(core): add regression test for foreign key duplication on schema-qualified alter#18228
emmayusufu wants to merge 1 commit into
sequelize:mainfrom
emmayusufu:test/12961-fk-duplication-schema

Conversation

@emmayusufu
Copy link
Copy Markdown

@emmayusufu emmayusufu commented May 28, 2026

Closes #12961.

That issue (duplicated foreign keys on every sync({ alter: true }) when the model is in a non-default schema) is already fixed on main — the alter path now reads .tableName off the reference object instead of comparing the whole object, and matches on schema. But there was no test guarding it.

The existing "should properly alter tables when there are foreign keys" test uses the default schema and doesn't assert anything, so it wouldn't catch a regression here.

This adds a test that puts two related models in a non-default schema, syncs twice with alter: true, and asserts the foreign key count stays at 1. I confirmed it has teeth: reverting the .tableName extraction makes it fail with a count of 2. Runs on any dialect that supports schemas; verified against Postgres locally.

Summary by CodeRabbit

  • Tests
    • Added integration test to verify that Model.sync() with schema customization properly handles foreign key constraints without duplication.

Review Change Stack

…equelize#12961)

sequelize#12961 is fixed on main (the alter-sync path now reads .tableName off the
reference object instead of comparing the whole object), but there was no
test guarding it. The existing foreign-key alter test uses the default
schema and asserts nothing, so it wouldn't catch a regression here.

Add a test that defines two related models in a non-default schema, syncs
twice with alter: true, and asserts the foreign key count stays at 1.
Reverting the fix makes this fail with a count of 2.
@emmayusufu emmayusufu requested a review from a team as a code owner May 28, 2026 09:37
@emmayusufu emmayusufu requested review from ephys and sdepold May 28, 2026 09:37
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f43565e8-2416-47d8-8c53-96861a422ce3

📥 Commits

Reviewing files that changed from the base of the PR and between 8260c29 and 6f27baa.

📒 Files selected for processing (1)
  • packages/core/test/integration/model/sync.test.js

📝 Walkthrough

Walkthrough

This PR adds a single regression test for issue #12961 that verifies Model.sync({ alter: true }) does not recreate or duplicate foreign key constraints when models are defined in non-default PostgreSQL schemas. The test creates a custom schema, defines a parent-child relationship, runs sync twice with alter enabled, and asserts the child table has exactly one foreign key constraint.

Changes

Schema FK Constraint Duplication Test

Layer / File(s) Summary
Schema FK constraint non-duplication test
packages/core/test/integration/model/sync.test.js
Integration test conditionally runs on schema-supporting dialects, creates a custom schema with two associated models, performs sync({ alter: true }) twice, and validates via constraint inspection that exactly one foreign key constraint exists on the child table to detect regressions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A schema test hops into view,
Checking constraints don't split in two,
Alter syncs twice with careful sight,
One foreign key? That's just right!
No duplicates will sneak past you. 🔑

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding a test to cover foreign keys not duplicating on schema-qualified alter, with the issue reference.
Linked Issues check ✅ Passed The pull request adds a test that directly addresses issue #12961 by verifying the fix prevents foreign key duplication on schema-qualified alter sync operations.
Out of Scope Changes check ✅ Passed The pull request only adds a single integration test file with no changes outside the scope of testing the foreign key duplication fix for schema-qualified models.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@emmayusufu emmayusufu changed the title test: cover foreign keys not duplicating on schema-qualified alter (#12961) meta(core): add regression test for foreign key duplication on schema-qualified alter May 28, 2026
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.

Duplicated foreign key with alter option when using PostgreSQL Schemas

1 participant