with_scope and :create
Phillip Kast
I've been using AR::Base.with_scope lately, and it's great. I'm finding
it especially useful with the new around_filters that let you specify a
block and yield to the filter chain.
But, when you pass with_scope a :create hash, there is some strange
behavior: Base.create respects the scoping, but Base.save does not, even
if you are saving a new record.
This feels inconsistent. A before_create filter in a model will fire in
both cases; shouldn't scoping behave the same?
I'd be happy to provide tests and a patch, but wanted to check before I
put those together. Is anybody working on this? Do people feel that this
is the right behavior for with_scope to have?
Thanks,
phil
--
Phillip Kast
(909)630-9562
ph...@unimedia.org
Phillip Kast
http://dev.rubyonrails.org/ticket/3734
Should have dug in trac more before posting -- sorry folks.
Any chance this will make it into core? It has tests and cleans up a
surprise lurking in with_scope.
Keith Morrison
I would love to see this patch applied. The current behavior has been irritating me
for a long time now.
Thanks,
Keith
Michael Koziarski
> http://dev.rubyonrails.org/ticket/3734
> Should have dug in trac more before posting -- sorry folks.
>
> Any chance this will make it into core? It has tests and cleans up a
> surprise lurking in with_scope.
Could you give us an example of what you're doing with with_scope that
needs this. The example in the trac ticket is definitely not
something that needs with_scope.
login = Login.new({:name=>"anonymous"}.merge(params[:login]))
login.save
--
Cheers
Koz
Phillip Kast
I'm using with_scope in an around_filter, similar to the way the
scoped_access plugin does.
The idea is to cleanly ensure that a number of models are always
associated with a user's account.
But, we have some places where stuff needs to be done to a new model
object before it's ready to save.
Something along the lines of:
class ApplicationController < AC::Base
def scope_story_by_account
Story.with_scope(:find => {:conditions => {:account_id =>
current_account.id}, :create => {:account_id => current_account.id}) {
yield }
end
around_filter :scope_by_account
end
class StoryController < ApplicationController
def create
@story = Story.new(params[:story])
# do some stuff to @story here...
@story.save #@story.account_id won't be set, without the patch
we're discussing.
end
end
...But with a bunch of models, controllers, and actions thrown into the mix.
Philosophically, it also seems to me that with_scope and ActiveRecord
callbacks should have the same behavior: if new_record is true, the
create callbacks/scoping should happen, regardless of whether you called
create or save.
phil
Michael Koziarski wrote:
>> http://dev.rubyonrails.org/ticket/3734
>>
> Could you give us an example of what you're doing with with_scope that
> needs this. The example in the trac ticket is definitely not
> something that needs with_scope.
>
> login = Login.new({:name=>"anonymous"}.merge(params[:login]))
> login.save
--
Phillip Kast
(909)630-9562
ph...@unimedia.org
Michael Koziarski
> def create
> @story = Story.new(params[:story])
> # do some stuff to @story here...
>
> @story.save #@story.account_id won't be set, without the patch
> we're discussing.
> end
> end
The 'right' way to do this would be
class StoryController < ApplicationController
def create
@story = current_account.stories.build params[:story]
# ...
@story.save # account_id set as required.
end
end
with_scope was originally just an implementation method for the
association proxies, and any time you find yourself using it, you're
probably missing out on a much much simpler option.
As it stands, I think I'd prefer the philosophical inconsistencies, if
it points people at the right way to do it.
--
Cheers
Koz
DHH
> > def create
> > @story = Story.new(params[:story])
> > # do some stuff to @story here...
> >
> > @story.save #@story.account_id won't be set, without the patch
> > we're discussing.
> > end
> > end
This is the same example I see for with_scope over and over again,
which troubles me greatly. I've only ever seen one example of
with_scope that seemed reasonable. Mike Clark had something in his
model class to merge multiple conditions into one. Even though that
seemed sorta legit, I really don't like features that point people in
the wrong direction 95% of the time. Having with_scope publicly
available seems to be exactly that.
I'm seriously considering that we should deprecate with_scope as a
public method and make it private by 2.0.
Michael Koziarski
> which troubles me greatly. I've only ever seen one example of
> with_scope that seemed reasonable. Mike Clark had something in his
> model class to merge multiple conditions into one. Even though that
> seemed sorta legit, I really don't like features that point people in
> the wrong direction 95% of the time. Having with_scope publicly
> available seems to be exactly that.
>
> I'm seriously considering that we should deprecate with_scope as a
> public method and make it private by 2.0.
Deprecation is a step too far, there are legitimate uses of
with_scope, particularly around encapsulated finds for permissions
models. I just think that there needs to be some serious warnings in
the documentation:
"You almost certainly can find a nicer way to do this"
The documentation should be updated to reflected it's intended use as well.
That being said, encapsulated finds with permissions models don't
need create or new or any of that stuff. As it stands, this is a
wontfix for me.
--
Cheers
Koz
dwebs...@gmail.com
Maybe just putting some warnings in the documentation would be nice.
I used the feature in some limited case. I found that sometimes it
helps keeping a block of the code readable and easy to understand.
That been said with_scope has its limitations.
Diego
On Oct 15, 7:29 pm, "Michael Koziarski" <mich...@koziarski.com> wrote:
> > This is the same example I see for with_scope over and over again,
> > which troubles me greatly. I've only ever seen one example of
> > with_scope that seemed reasonable. Mike Clark had something in his
> > model class to merge multiple conditions into one. Even though that
> > seemed sorta legit, I really don't like features that point people in
> > the wrong direction 95% of the time. Having with_scope publicly
> > available seems to be exactly that.
>
> > I'm seriously considering that we should deprecate with_scope as a
> > public method and make it private by 2.0.Deprecation is a step too far, there are legitimate uses of
Phillip Kast
I think what I'm ultimately trying to do is legit though, so I want to
try and make the case one more time.
So what I want is to be able to say something in a controller like:
scoped_by_current_account :story
with the result that all find calls on a Story model from that
controller will only return stories that belong to the current account,
and any new stories will always be associated with the current account.
I've done this. It's easy to whip up using with_scope and around
filters, and it seems like a clean and useful abstraction to me. Among
other things, it reduces the risk that a hole in test coverage and/or a
missing find condition could result in, say, returning stories for the
wrong account. You can nest scoping and do this for several models at a
time too, no problem.
This works great for finds. The trouble is that you can't do the same
thing for creates, because the association won't be set if you call save
instead of create, which is too fragile. But, the abstraction makes the
most sense if it works in both directions.
It seems like the legit uses of with_scope that folks have mentioned all
involve passing :find, and the bad uses I've given all use :create.
Documentation would help, but the bigger issue is consistency. I think
either with_scope/:create should treat new records the same way the rest
of ActiveRecord does, or it should go away. Perhaps the way to make it
go away would be to leave with_scope public, but extract the part that
handles the :create hash to a private method.
Thanks, and I hope I'm not ranting too much here -- I'm just interested
in seeing this little corner of Rails be less surprising.
phil
Francois Beausoleil
2006/10/15, Phillip Kast <ph...@unimedia.org>:
> So what I want is to be able to say something in a controller like:
> scoped_by_current_account :story
> with the result that all find calls on a Story model from that
> controller will only return stories that belong to the current account,
> and any new stories will always be associated with the current account.
Isn't this clearer / more understandable ?
current_user.stories.find(params[:id])
It is obvious from this that you're fetching a user story, not a
random story. If I don't see your filters (which I almost never
bother to parse unless that's what I'm working on currently), then I
won't catch your filter usage.
Same goes for creating too:
current_user.stories.create!(params[:story])
# works the same with build
Bye !
--
François Beausoleil
http://blog.teksol.info/
http://piston.rubyforge.org/
Tomas Jogin
examples of when it is particularly useful, is that the uses where
with_scope shines aren't particularly short nor simple in nature.
I used with_scope once when I made a whole lot of complex finds and I
wanted to limit all of them to finding objects which had is_published
= 1, but only in _this_ particular case. It did, in fact, make the
code easier to read, but the biggest reason I used it was to make sure
I didn't mistakenly allow one of those many finds to select
non-published objects, in one fell swoop I eliminated the possibility
of that happening by mistake.
Improved documentation seems like a better solution than to limit the
useage. This is Ruby we're talking about, there are lots of features
available to misuse already, that doesn't take away the legitimate
uses.
Best regards,
Tomas Jogin
DHH
> with_scope, particularly around encapsulated finds for permissions
> models. I just think that there needs to be some serious warnings in
> the documentation:
The decision has been rendered as follows: with_scope will go protected
from Rails 2.0. That means you'll be able to use with_scope in your own
private finders (the only legitimate use I've seen), but not in, say,
filters.
Now we just have to find a way to get those deprecation messages in
there when its being called in the public space.
Martin Emde
I wrote that filter patch specifically for using with_scope in controllers (besides that it was a much needed feature in general). I think it's incredibly useful. I have an application where 5-6 of the controllers are all scoped under an Order. I use simply restful to make a path like orders/1/controller/ so that all of those methods can be scoped by that order. I add one around_filter and it's taken care of. I also access some of those controllers separately and scoped under a person. Because I have with_scope, I don't have to write all these weird exceptions in the controller code. Just clean and simple CRUD actions.
I feel very passionately that this is an excellent use of with_scope. There were plugins before the new around_filter that did this, and as soon as it's introduced, people used it for that. Denying this usefulness is just going to irritate people. I myself will just write a public method that calls the protected with_scope if the public with_scope is deprecated. If this is viewed as illegitimate, so be it.
I urge you to reconsider your decision to deprecate public with_scope and instead give a *real argument* against it in the documentation.
Respectfully,
Martin Emde
DHH
> because it doesn't fit your idea of useful. Why not let the coder make these
> decisions?
You must be new here :). Rails is opinionated software. We make things
that we consider good style easy to do and bad style hard. Or rather,
we make good style beautiful and bad style ugly. So even though
with_scope is going protected, you can still use it in filters if you
really, really want to. Just use send(:with_scope) -- that'll side-step
the access control. Yes, that'll be ugly and it's intended to be.
Using around_filters in the manor you describe makes it really hard to
figure out what's going on from looking at the action in isolation. It
looks like you're doing a straight, in-secure find to the naked eye.
And there's no bread crumbs for others to follow that'll leave them to
your around_filter. It's a recipe for hard-to-read code. And it shields
the reader from understand your model hierarchy.
In summary, Rails will shake its head at you for using with_scope in
filters, but will ultimately leave it at your discretion to choose the
right path. We believe in encouragement and discouragement, not
allowing or forbidding.
Martin Emde
In summary, Rails will shake its head at you for using with_scope in
filters, but will ultimately leave it at your discretion to choose the
right path. We believe in encouragement and discouragement, not
allowing or forbidding.
-Martin