Update searchManager() access #366
Conversation
Calling behavior methods directly on the table instance is deprecated in CakPHP 5.3
|
|
||
| // Setup search filter using search manager | ||
| $this->searchManager() | ||
| $this->getBehavior('Search')->searchManager() |
There was a problem hiding this comment.
But isnt that then executed always? Whereas the current searchManager() is only executed once you actually invoke it?
There was a problem hiding this comment.
My main concern is the length of initialize() though.
It often already is a lot of lines long for all the relation setup and behavior adding and more.
This adds also the whole search config into it.
So the extra method wasnt too bad after all in terms of readability of the code.
It seems the only alternative really is then to use the collections?
There was a problem hiding this comment.
I just found out:
Additionally, it breaks all usage of dynamic attachment if one is forced to to abandon this method:
$this->Users->addBehavior('Search.Search');
wouldnt work anymore, so far did by using the method when needed.
There was a problem hiding this comment.
But isnt that then executed always?
In the updated example, the $this->searchManager() call was also from table's initialize() so it too was executed always.
My main concern is the length of initialize() though.
Yes that's a concern, that is why filter collections were added.
it breaks all usage of dynamic attachment
Yes it does, the solution is to use filter collections. In general I don't think configuring tables from outside is a good idea.
Anyway, we can undo the deprecation of searchManager() on table classes if you prefer.
There was a problem hiding this comment.
We could make it a soft deprecation for now, this wouldnt make all CI builds report it right away, and people have time to migrate.
This was a patch release after all.
There was a problem hiding this comment.
This was a patch release after all.
It was 1st patch release of a new minor :)
There was a problem hiding this comment.
Besides there's no noticeable way to soft-deprecated this feature as it's a methods that's added to the table class, so we can't have any deprecation annotation for it.
There was a problem hiding this comment.
Its visible in the IDE (strike though), that could be enough for the time being IMO.
There was a problem hiding this comment.
But how to achieve that? As I said this is a method added to app code, not the plugin itself.
Calling behavior methods directly on the table instance is deprecated in CakPHP 5.3