Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/inconvenience when baking FilterCollections #315

Closed
passchn opened this issue Feb 15, 2022 · 4 comments · Fixed by #317
Closed

Bug/inconvenience when baking FilterCollections #315

passchn opened this issue Feb 15, 2022 · 4 comments · Fixed by #317

Comments

@passchn
Copy link

passchn commented Feb 15, 2022

I you bake a FilterCollection through the CLI, e.g.:

bin/cake bake filter_collection Users

this file will be created:

class UsersFilterCollection extends FilterCollection

However, this does not seem to work. It must be UsersCollection instead.

I had quite a hard time finding out why it did not work, until I read the Docs very precisely, where there is a PostsCollection and not a PostsFilterCollection:
https://github.com/FriendsOfCake/search/tree/master/docs#filter-collection-classes

It would be great, if the Behavior would accept both versions, or if the bake command would create a Filter with proper naming.

@passchn
Copy link
Author

passchn commented Feb 15, 2022

Versions:

"cakephp/cakephp": "^4.3", (4.3.5)
"friendsofcake/search": "^6.2", (6.2.3)

@ADmad
Copy link
Member

ADmad commented Feb 16, 2022

The "Filter" in the class name when they are already under the "Filter" folder/namespace does feel unnecessary.

@passchn
Copy link
Author

passchn commented Feb 16, 2022

It comes from here:
https://github.com/FriendsOfCake/search/blob/master/templates/bake/FilterCollection/filter_collection.twig

The Behavior on line 65 ff. looks for a Class name {Alias}Collection though:
https://github.com/FriendsOfCake/search/blob/master/src/Model/Behavior/SearchBehavior.php

So it cannot work, I guess. But it might be a deliberate decision where I just did not find the reason. Both files were changed more than a year ago, so I cannot be the first person to notice that. :D

@dereuromark
Copy link
Member

Having "FooCollection" all over the code makes it harder to clarify what those are needed for, or harder to search.
So I always add the suffix, same was we do for all other classes, e.g. Table and alike
Also to avoid collisions etc.

But yeah, for now we just need to fix it then to match what is expected
What I dont get:

namespace Community\Model\Filter;

use Search\Model\Filter\FilterCollection;

class VideosFilterCollection extends FilterCollection {

I use it successfully in all my projects the "right way"...
Why does it work for me the way I added it as bake commands?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants