Feature/2.0 return query #147

Merged
merged 2 commits into from Jul 15, 2011

Conversation

Projects
None yet
3 participants
Member

burzum commented Jul 11, 2011

Adding the "returnQuery" key to the 2nd argument of the find() method to be able to get the query array back from the before state of findMethod() calls. This was required in the past for some more complex queries and is in 2.0 no longer possible because the find methods became protected.

@burzum burzum Adding the "returnQuery" key to the 2nd argument of the find() method…
… to be able to get the query array back from the before state of findMethod() calls. This was required in the past for some more complex queries and is in 2.0 no longer possible because the find methods became protected.
9d7c97c
Owner

markstory commented Jul 12, 2011

How does the methods becoming protected make them harder to work with? Couldn't you just override them in subclasses?

Also if there is a need for this kind of feature, why not move the beforeFind + behavior logic into a separate method? Something like prepareQuery or buildQuery? Then we wouldn't need another flag option that totally changes the behavior of find() making it do things that are not returning records.

Owner

lorenzo commented Jul 12, 2011

The use case for this is one of CakeDC plugins, that eases the creation of subqueries inside a model using a behavior. In the past the were no restrictions for calling _find[Method](), but now those are protected, which makes impossible the subquery generation fur custom finds inside a behavior.

This commit aims to overcome that problem by giving the ability to get the resulting array query before sending it to the datasource.

Additionally, I can see that this change would actually helo a bit at doing features such as the named scope rails has, but I'm not sure about that one.

Member

burzum commented Jul 12, 2011

Mark, the issue is that what you propose wont work with behaviors as Jose already said. But I agree moving it to another method would be nice but then I would duplicate some code from find(). That is why I preferred to to add another key.

Owner

markstory commented Jul 12, 2011

How does having a method that extracts the code in model::find() from lines 2128 -> 2173 and converting those into a new public method not work for behaviors? A behavior could call $model->buildQuery() and get the same results of the returnQuery key.

I guess my main gripe with the 'returnQuery' param is that it changes find() to do more than one thing. Currently find() returns result arrays. Cake is littered with methods that do totally different things based on option flags, and I'm not sure its a great pattern.

Member

burzum commented Jul 12, 2011

I was referring to the suggested in-heritage of the model.

I agree with that a method should do just one specific task at a time. Going to change that.

@lorenzo lorenzo added a commit that referenced this pull request Jul 15, 2011

@lorenzo lorenzo Merge pull request #147 from burzum/feature/2.0-return-query
Feature/2.0 return query
57a8c10

@lorenzo lorenzo merged commit 57a8c10 into cakephp:2.0 Jul 15, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment