Bypass the support table cache when finding on has_many associations#7
Merged
Bypass the support table cache when finding on has_many associations#7
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where find_by on has_many associations incorrectly used the cache without respecting the association's scope. The fix ensures that queries on has_many associations bypass the cache and query the database directly.
Key Changes:
- Added check to bypass cache for
ActiveRecord::Associations::CollectionProxyinstances - Enhanced YARD documentation with proper parameter and return type annotations
- Added comprehensive test coverage for the association bypass behavior
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/support_table_cache/relation_override.rb | Added logic to detect and bypass cache for has_many associations; improved YARD documentation |
| lib/support_table_cache/memory_cache.rb | Added comprehensive YARD documentation for all public methods |
| lib/support_table_cache/find_by_override.rb | Enhanced YARD documentation with return types and proper exception formatting |
| lib/support_table_cache/associations.rb | Improved YARD exception documentation formatting |
| lib/support_table_cache.rb | Enhanced YARD documentation throughout with proper return types and yield descriptions |
| spec/support_table_cache_spec.rb | Added test case for association behavior; updated to use RSpec.describe; fixed missing nil check test |
| spec/spec_helper.rb | Added Thing and OtherThing models/tables for testing; enabled RSpec best practices |
| spec/support_table_cache/memory_cache_spec.rb | Updated require path and RSpec.describe usage |
| spec/support_table_cache/associations_spec.rb | Updated require path and RSpec.describe usage |
| CHANGELOG.md | Added entry for version 1.1.4 with fix description |
| VERSION | Bumped version to 1.1.4 |
| .github/workflows/continuous_integration.yml | Added YARD documentation validation to CI pipeline |
Comments suppressed due to low confidence (1)
spec/spec_helper.rb:95
- The cleanup in the before block should include the new Thing and OtherThing models to prevent test data from leaking between tests. Consider adding Thing.delete_all and OtherThing.delete_all to maintain test isolation.
config.before do
TestModel.delete_all
ParentModel.delete_all
DefaultScopeModel.unscoped.delete_all
SupportTableCache.cache.clear
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed
find_byon ahas_manyrelation would not take the scope of the relation into account when looking up the cached record. Now chaining afind_byonto ahas_manyrelation will correcty bypass the cache and directly query the database.