-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add title filter in admin using LexikFormFilterBundle #280
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
Conversation
I'm not sure about this feature. The purpose of the Symfony Demo is to show as much built-in Symfony features as possible. We don't want to develop an advanced blog engine. Before merging or closing this proposal, I'll wait to read more opinions. Thanks! |
{ | ||
$entityManager = $this->getDoctrine()->getManager(); | ||
$posts = $entityManager->getRepository('AppBundle:Post')->findAll(); | ||
$form = $this->get('form.factory')->create(new PostFilterType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't pass a type instance. This is deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How it is done now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sergiu-popa use FQCN instead. In your project you could use ...->create(PostFilterType::class)
but in Symfony Demo we still need to use: ...->create('AppBundle\Filter\PostFilterType')
for BC.
I think @javiereguiluz is right. Third-party bundles mostly are developer specific, some developers use this bundle, some of them could use other. |
@@ -26,7 +26,8 @@ | |||
"symfony/monolog-bundle" : "~2.7", | |||
"symfony/swiftmailer-bundle" : "~2.3", | |||
"symfony/symfony" : "~2.8", | |||
"twig/extensions" : "~1.2" | |||
"twig/extensions" : "~1.2", | |||
"lexik/form-filter-bundle": "dev-master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
depending on dev-master is a no-go. If the bundle does not have stable releases (which should be considered though), a branch alias should at least be used to avoid getting bumped to a new major version if they start developing it. If they don't support installing it the bundle other than with a dev-master
requirement, I vote against including it in the demo.
And if they don't have a branch alias, you should open a PR adding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
composer.phar require lexik/form-filter-bundle ~4.0
And btw, this bundle is used only to perform a |
Provided types: https://github.com/lexik/LexikFormFilterBundle/blob/master/Resources/doc/provided-types.md. I could add this functionality without the Lexik bundle if we decide it should be in the Demo App. |
@sergiu-popa and all these custom types are necessary only because of all the complex logic the bundle contains just to perform this alteration of the query builder. If you just build it the simple way, using the |
@javiereguiluz, I don't consider by adding a filter would make the demo an advanced blog engine. I think we should add a filter for a table because it's not something unusual for an application. If not using LexikBundle, I could create it using Symfony's built in functionality. What do you think? |
I'm closing this pull request for being too complex for the purpose of the Symfony Demo app. However, @sergiu-popa is right and we could add some filter in the backend without using this filter. My question: could we adapt the (still not merged) #173 to use it also in the backend? |
Since the admin backend doesn't use KnpPaginator, we could add a filter for title field using LexikFormFilterBundle.