Skip to content

Conversation

@technicaldirector
Copy link
Contributor

Fixes #14350

@technicaldirector technicaldirector changed the title fix(drizzle): Skip foreign key for blocks that are shared across collections fix(drizzle): skip foreign key for blocks that are shared across collections Oct 29, 2025
@DanRibbens DanRibbens requested review from Copilot and r1tsuu October 30, 2025 14:31
Copy link
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

This PR fixes an issue where foreign keys were incorrectly created for blocks that are shared across collections. The fix identifies blocks with custom database names and skips creating the parent ID foreign key for them.

  • Adds logic to detect blocks with custom database names that indicate shared usage across collections
  • Conditionally creates foreign keys only for non-shared blocks
  • Prevents database constraint errors for shared block configurations

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

const baseForeignKeys: Record<string, RawForeignKey> = {
_parentIdFk: {
// Skip creating a parent_id foreign key for blocks that are shared across collections (have a custom dbName)
const hasCustomDbName = block.dbName !== undefined && block.dbName !== null
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Consider using a more concise nullish coalescing check. The condition can be simplified to block.dbName != null which checks for both undefined and null in a single comparison.

Suggested change
const hasCustomDbName = block.dbName !== undefined && block.dbName !== null
const hasCustomDbName = block.dbName != null

Copilot uses AI. Check for mistakes.
Copy link
Member

@r1tsuu r1tsuu left a comment

Choose a reason for hiding this comment

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

I wonder if you have tried to use Block References for this case and if that feature doesn't work here we should fix it.

I don't think that this PR is a good solution since just checking for block.dbName does not really tell us that this block is shared across collections. Additionally, it'd cause a schema regression for every block that has dbName with removing a foreign key.
If we really want to check if a block is "shared", we'd have to traverse fields of every collection and see if we can find at least 2 the same JS references to that block.

@technicaldirector
Copy link
Contributor Author

technicaldirector commented Oct 30, 2025

I wonder if you have tried to use Block References for this case and if that feature doesn't work here we should fix it.

I don't think that this PR is a good solution since just checking for block.dbName does not really tell us that this block is shared across collections. Additionally, it'd cause a schema regression for every block that has dbName with removing a foreign key. If we really want to check if a block is "shared", we'd have to traverse fields of every collection and see if we can find at least 2 the same JS references to that block.

If I remember correctly I tried it with Block referneces and received the same problem. In addition to that block references had the problem that when you have a custom field with imports these haven't been added to the import map.

@technicaldirector
Copy link
Contributor Author

Yes - just confirmed it. Same issue when using blockReferences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Foreign Key Constraint Violation with Blocks Used Across Multiple Collections

2 participants