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

Allow reload to be default_scoped #40805

Merged
merged 1 commit into from Dec 14, 2020

Conversation

eileencodes
Copy link
Member

reload is not default_scoped by default because you could be
creating a record that does not match your default scope and therefore
reload wouldn't find the record.

However, in the case of sharding an application you may want reload to
support default_scope because you'll always have the correct scope
set. To accomplish this I added a new method that checks if there are
any default scopes defined that are set to run for all_queries. If
there are then don't unscope the find methods.

If there is a case where you do want the default scope to apply to all
queries but be able to turn that off, I've added a unscoped option to
reload. This would use the original behavior regardless of whether the
default_scope was used for all queries.

Additionally, this exposes a new public method default_scopes? that
returns true if there are default scopes. If all_queries is passed to
default_scopes? the method will only return true if there is a default
scope for the model that has all_queries set to true. If there are no
default scopes with all_queries then it will return false.

`reload` is not `default_scoped` by default because you could be
creating a record that does not match your default scope and therefore
`reload` wouldn't find the record.

However, in the case of sharding an application you may want `reload` to
support `default_scope` because you'll always have the correct scope
set. To accomplish this I added a new method that checks if there are
any default scopes defined that are set to run for `all_queries`. If
there are then don't `unscope` the find methods.

If there is a case where you do want the default scope to apply to all
queries but be able to turn that off, I've added a `unscoped` option to
`reload`. This would use the original behavior regardless of whether the
`default_scope` was used for all queries.

Additionally, this exposes a new public method `default_scopes?` that
returns true if there are default scopes. If `all_queries` is passed to
`default_scopes?` the method will only return true if there is a default
scope for the model that has `all_queries` set to true. If there are no
default scopes with `all_queries` then it will return false.
@eileencodes
Copy link
Member Author

all_queries should mean all_queries so it makes sense to add reload to this if you have that option set.

@eileencodes eileencodes merged commit a395c3a into rails:master Dec 14, 2020
@eileencodes eileencodes deleted the default-scope-on-reload branch December 14, 2020 20:20
eileencodes added a commit to eileencodes/rails that referenced this pull request Feb 11, 2021
Similar to rails#40720
and rails#40805 this change allows for the
`scoping` method to apply to all queries in the block. Previously this
would only apply to queries on the class and not the instance. Ie
`Post.create`, Post.all`, but not `post.update`, or `post.delete`.

The change here will create a global scope that is applied to all
queries for a relation for the duration of the block.

Benefits:

This change allows applications to add a scope to any query for the
duration of a block. This is useful for applications using sharding to
be able to control the query without requiring a `default_scope`. This
is useful if you want to have more control over when a `scoping` is used
on a relation. This also brings `scoping` in parity with the behavior of
`default_scope` so there are less surprises between the behavior of
these two methods.

There are a caveats to this behavior:

1) The `scoping` only applies to objects of the same type. IE you cannot
scope `Post.where(blog_id: 1).scoping` and then expect `post.comments`
will apply `blog_id = 1` to the `Comment` query. This is not possible
because the scope is `posts.blog_id = 1` and we can't apply the `posts`
scope to a `comments` query. To solve this, scopes must be nested.
2) If a block is scoped to `all_queries` it cannot be unscoped without
exiting the block. I couldn't find a way around this but ActiveRecord
scoping is a bit complex and turning off `all_queries` when it's already
on in nested scoping blocks had interesting behavior that I decided was
best left out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant