-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Support combining scopes when loading records, use Arel instead of SQL #717
Open
mamhoff
wants to merge
12
commits into
CanCanCommunity:develop
Choose a base branch
from
mamhoff:refactor-activerecord-adapter
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Support combining scopes when loading records, use Arel instead of SQL #717
mamhoff
wants to merge
12
commits into
CanCanCommunity:develop
from
mamhoff:refactor-activerecord-adapter
Commits on Jun 21, 2021
-
Configuration menu - View commit details
-
Copy full SHA for f372180 - Browse repository at this point
Copy the full SHA f372180View commit details
Commits on Jun 28, 2021
-
Pass a AR::Relation into the ActiveRecord adapter
This breaks all kinds of things for now, but will open up using the `accessible_by` on scopes.
Configuration menu - View commit details
-
Copy full SHA for f0282ce - Browse repository at this point
Copy the full SHA f0282ceView commit details -
Rails 5.1 has been EOL for a long while, and the code hoops we'd have to jump through to keep supporting it are really quite large.
Configuration menu - View commit details
-
Copy full SHA for 79e22cc - Browse repository at this point
Copy the full SHA 79e22ccView commit details -
Configuration menu - View commit details
-
Copy full SHA for 6839d7d - Browse repository at this point
Copy the full SHA 6839d7dView commit details -
Use ActiveRecord to join conditions, not SQL
This passes all the conditions we have into ActiveRecord's `where` method and joins them using `or` and leaves the SQL generation to ActiveRecord/Arel.
Configuration menu - View commit details
-
Copy full SHA for 8ec84d6 - Browse repository at this point
Copy the full SHA 8ec84d6View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7bced0e - Browse repository at this point
Copy the full SHA 7bced0eView commit details -
Test generated SQL instead of conditions Array
The conditions array is really an artefact of the old implementation and should IMO not be tested. Let's instead test where the rubber hits the road: Which database records are returned.
Configuration menu - View commit details
-
Copy full SHA for f9aecd5 - Browse repository at this point
Copy the full SHA f9aecd5View commit details -
Remove artefacts of SQL-based implementation
Now that we're using ActiveRecord to generate and merge conditions, we do not need a lot of code any longer.
Configuration menu - View commit details
-
Copy full SHA for 2d05908 - Browse repository at this point
Copy the full SHA 2d05908View commit details -
Configuration menu - View commit details
-
Copy full SHA for b0f0436 - Browse repository at this point
Copy the full SHA b0f0436View commit details -
This extracts a lot of stuff into methods. I sure am hopeful this makes the code more readable!
Configuration menu - View commit details
-
Copy full SHA for 4f8bc1c - Browse repository at this point
Copy the full SHA 4f8bc1cView commit details -
IDs have primary key constraints, and we cannot know when our test runs respective to other tests.
Configuration menu - View commit details
-
Copy full SHA for e281cd2 - Browse repository at this point
Copy the full SHA e281cd2View commit details -
Ok, so Postgres can't order by a column that's not on the select list. Let's put the column on the select list if that's what we want to order by!
Configuration menu - View commit details
-
Copy full SHA for 5c41c53 - Browse repository at this point
Copy the full SHA 5c41c53View commit details
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.