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

Active Record get_all Fix #327

Merged
merged 4 commits into from Jan 7, 2018
Merged

Active Record get_all Fix #327

merged 4 commits into from Jan 7, 2018

Conversation

@jnunemaker
Copy link
Owner

@jnunemaker jnunemaker commented Jan 6, 2018

Fixes the sequel and active record adapters to include even disabled features in get_all. The issue was using an inner join between feature and gate tables, which means features in the feature table but not in the gate table (because they were fully disabled) were not represented in get_all.

fixes #324

@jnunemaker jnunemaker added the bug label Jan 6, 2018
@jnunemaker jnunemaker self-assigned this Jan 6, 2018
@jnunemaker
Copy link
Owner Author

@jnunemaker jnunemaker commented Jan 6, 2018

I think I should probably make this an adapter shared spec/test so we can catch this for all of them.

Loading

@jnunemaker
Copy link
Owner Author

@jnunemaker jnunemaker commented Jan 6, 2018

The sequel adapter also has this problem. I'll move this to a shared test/spec for all adapters and then merge it.

Loading

@jnunemaker jnunemaker merged commit 5026e12 into master Jan 7, 2018
1 of 2 checks passed
Loading
@jnunemaker jnunemaker deleted the get_all_fix branch Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

1 participant