3.0 dynamic finders #2409
3.0 dynamic finders #2409
Conversation
* Support generating conditions based on method. * Allow first and all to be used as find types. * Allow 'and' and 'or' operators.
Previous implementations of magic finders would just do the wrong thing. Throwing an exceptino feels safer and more explicit.
| foreach ($fields as $field) { | ||
| $conditions[$field] = array_shift($args); | ||
| } | ||
| $order = array_shift($args); |
There was a problem hiding this comment.
I don't think supporting order in dynamic finders is a good idea. It makes for some really unreadable code, findByName('jose')->order(['name' => 'DESC]) is a better alternative
There was a problem hiding this comment.
I'll take it out then 😄
| $this->assertInstanceOf('Cake\ORM\Query', $result); | ||
| $this->assertEquals(1, $result->clause('limit')); | ||
| $expected = new QueryExpression(['username' => 'garrett'], ['username' => 'string']); | ||
| $this->assertEquals($expected, $result->clause('where')); |
There was a problem hiding this comment.
I'm really happy to see the unexecuted query being asserted in these tests =)
There was a problem hiding this comment.
Me too, I would feel dirty executing the query here. There are numerous integration tests elsewhere that check that queries do the right thing.
|
I think we should drop |
|
@lorenzo So only have |
|
@markstory yeah, that is how I'd like it to work |
|
Ok, I'll get that working tonight. |
Instead you can call ->first() on the query object returned.
| $fields = substr($method, 8); | ||
| $findType = 'all'; | ||
| } else { | ||
| $fields = substr($method, strlen($matches[0])); |
There was a problem hiding this comment.
Late to the party, but there's no test showing that findTrollsBy works - using findAll to test this chunk where all is the default type anyway could hide a bug in the future.
There was a problem hiding this comment.
Good point, I'll add a test case.
As per #2212 I've implemented dynamic finder methods. They are mostly backwards compatible with previous versions. They are intentionally less powerful, in hopes of directing people to use custom finders instead. I've also taken Andy's suggestion and made it possible to combine dynamic finders + custom finders which is kind of nice.