Execute default_scope defined by abstract class within the scope of subclass #23666
Conversation
|
r? @pixeltrix (@rails-bot has picked a reviewer for you, use r? to override) |
meinac
force-pushed
the
fix_abstract_default_scope_bug
branch
from
8debebd
to
2add7c0
Compare
|
I've fixed the failing tests but travis is not working, maybe we need to close and reopen this. |
|
👏 |
|
r? @sgrif |
| @@ -115,7 +115,10 @@ def build_default_scope(base_rel = nil) # :nodoc: | |||
| base_rel ||= relation | |||
| evaluate_default_scope do | |||
| default_scopes.inject(base_rel) do |default_scope, scope| | |||
| default_scope.merge(base_rel.scoping { scope.call }) | |||
| scoped = base_rel.scoping do | |||
| scope.is_a?(Proc) ? instance_exec(&scope) : scope.call | |||
There was a problem hiding this comment.
Is there any case where scope is not a Proc?
There was a problem hiding this comment.
Yes, any kind of object can be used in default scope if the object is callable. I saw this in Rails tests, for example in this case it uses OpenStruct.
But now I think that I have to handle the call method case also in correct context. Change is on the way.
meinac
force-pushed
the
fix_abstract_default_scope_bug
branch
from
2add7c0
to
6600bd7
Compare
|
@rafaelfranca I've changed the logic. If you have a look at the line you can see that we can use any kind of object which respond to call method as default scope and now we can handle it in correct context. |
|
This looks good to me. r? @sgrif WDYT? |
meinac
force-pushed
the
fix_abstract_default_scope_bug
branch
from
6600bd7
to
85555ad
Compare
| @@ -115,7 +115,8 @@ def build_default_scope(base_rel = nil) # :nodoc: | |||
| base_rel ||= relation | |||
| evaluate_default_scope do | |||
| default_scopes.inject(base_rel) do |default_scope, scope| | |||
| default_scope.merge(base_rel.scoping { scope.call }) | |||
| scope = scope.is_a?(Proc) ? scope : scope.method(:call) | |||
There was a problem hiding this comment.
Should this be respond_to?(:to_proc)?
meinac
force-pushed
the
fix_abstract_default_scope_bug
branch
from
85555ad
to
b5c7dfe
Compare
meinac
force-pushed
the
fix_abstract_default_scope_bug
branch
from
b5c7dfe
to
0784dcc
Compare
|
@sgrif rebased & updated this. |
Execute default_scope defined by abstract class within the scope of subclass
|
@sgrif Should this be backported to |
Execute default_scope defined by abstract class within the scope of subclass
…ope_bug" This reverts commit bd44967. Reason: tests are broken after this change.
…_bug Execute default_scope defined by abstract class within the scope of subclass
…lt_scope_bug" This reverts commit bd44967. Reason: tests are broken after this change.
This commit will fix #23413 and #10658. In old behaviour it tries to execute block given by
default_scopewithin the context of abstract class and in this case it can't find the table name. Now it tries to execute the block within subclass and everything works.