Skip to content

fix: handle removal of :context multitenancy in migrations#752

Open
Ryan6794 wants to merge 2 commits into
ash-project:mainfrom
Ryan6794:main
Open

fix: handle removal of :context multitenancy in migrations#752
Ryan6794 wants to merge 2 commits into
ash-project:mainfrom
Ryan6794:main

Conversation

@Ryan6794
Copy link
Copy Markdown

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • [X ] I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

When a resource transitions from :context multitenancy to global?: true (or removing multitenancy entirely), the migration generator produces an invalid migration that tries to drop a foreign key constraint that doesn't exist yet.

Bugs:

  1. Wrong snapshot folder lookup - :context snapshots are stored in tenants/ but after removing multitenancy the code looks in the base folder. The old snapshot is not found so the table is treated as brand new.

  2. Incorrect FK drop generated - has_reference?/2 did not account for the case where both the table and the referenced resource previously had :context multitenancy. Since the FK was never created in the global schema, it should not be dropped.

Fix:

  1. get_existing_snapshot/2: fall back to checking the alternate folder when the primary lookup returns nil.
  2. has_reference?/2: return false when both multitenancy strategies are :context, since no global FK exists to drop.

Testing:

Reproduced using the demo repo linked in the issue: https://github.com/serpent213/ash_postgres_multitenancy_migration_demo

After the fix, all three steps complete successfully:

  1. mix ash.reset
  2. mix ash_postgres.generate_migrations --dev
  3. mix ash.migrate

@zachdaniel
Copy link
Copy Markdown
Contributor

Could we add a test for this too?

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.

2 participants