Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly reset sequence if switching with multiple schemas #203

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

ryanswood
Copy link

The recent change to fix table sequences does not account for multi-schema switching:

def default_sequence_name(table, _column)
  res = super
  # Code expects a single schema not an array
  schema_prefix = "#{Apartment::Tenant.current}." # => "[\"tenant1\", \"tenant2\"]."

thus sequences are not scoped correctly.

Before (incorrect):

> Apartment::Tenant.switch(['tenant1', 'tenant2']) { Foo.sequence_name }
=> tenant1.foos_id_seq

After:

> Apartment::Tenant.switch(['tenant1', 'tenant2']) { Foo.sequence_name }
=> foos_id_seq

Comment on lines +160 to +159
Company.reset_sequence_name
User.reset_sequence_name
Copy link
Author

Choose a reason for hiding this comment

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

Without resetting the sequence, the spec would pass if ran with the higher level spec with the same name because the sequence_name is cached on the model. I applied the same process to the other spec to avoid false positives depending on how RSpec orders the tests.

Let me know if there is a better way to do this.

@ryanswood ryanswood force-pushed the multiple-schemas-and-sequences branch 2 times, most recently from 856e4a1 to 445b891 Compare June 16, 2022 04:34
@mnovelo
Copy link
Collaborator

mnovelo commented May 14, 2024

@ryanswood would you mind rebasing?

@ryanswood ryanswood force-pushed the multiple-schemas-and-sequences branch from 445b891 to 4da402e Compare May 21, 2024 01:02
@ryanswood
Copy link
Author

@mnovelo Sorry for the delay. I wasn't feeling well for a bit. PR has been rebased!

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.

None yet

2 participants