Conversation
lchrusciel
left a comment
lchrusciel
left a comment
There was a problem hiding this comment.
First of all: Thanks for your tremendous work!
Your implementation is great. It follows many best practices. You have used strict types and code is well documented. Also, it is great, that we can learn and share some implementation with API platform. It is awesome!
But on the other hand, many classes are really complex. I know that is the result of libraries limitation, but I'm afraid that I won't be able to provide proper support for this logic in the long term.
What would you say about creating another library on a top of this one, with this implementation? It will be a pity if we lose all this work. Also, a separate library may convince other contributors involved in API platform to help us maintain the project. And it will be a good alternative for people who don't want to use elastic search in their applications.
I really regrate, that I'm not able to help you because of lack of time :(
| $conditions = ['boolean'=>['enabled'=>true]]; | ||
| $resourceClass = Product::class; | ||
|
|
||
|
|
| $this->applyFilter($conditions, $resourceClass, $queryBuilder); | ||
| } | ||
|
|
||
| function it_should_apply_a_boolean_condition(ManagerRegistry $managerRegistry, ObjectManager $om, ClassMetadata $classMetadata, LoggerInterface $logger, QueryBuilder $queryBuilder) |
There was a problem hiding this comment.
It would be more readable if we use full name instead of abbreviation for 'om'
| * | ||
| * @author Grégoire Hébert <gregoire@les-tilleuls.coop> | ||
| */ | ||
| class FiltersDefinitionPass implements CompilerPassInterface |
| /** | ||
| * Gets every filter definition and adds them to the Filter Extension. | ||
| * | ||
| * @author Grégoire Hébert <gregoire@les-tilleuls.coop> |
There was a problem hiding this comment.
I hope you don't mind, but we have resigned from author blocks as they are redundant (you will be mentioned in code contributors) and not obvious to maintain.
| * | ||
| * @author Grégoire Hébert <gregoire@les-tilleuls.coop> | ||
| */ | ||
| class InvalidArgumentException extends \InvalidArgumentException |
There was a problem hiding this comment.
A little bit redundant ;)
| } | ||
|
|
||
| /** | ||
| * Add a filter |
| } | ||
|
|
||
| /** | ||
| * Applies the filters. |
| * | ||
| * @see https://github.com/api-platform/core for the canonical source repository | ||
| * | ||
| * @copyright Copyright (c) 2015-present Kévin Dunglas |
There was a problem hiding this comment.
I hope Kevin will not mind ;)
| * @author Amrouche Hamza <hamza.simperfit@gmail.com> | ||
| * @author Teoh Han Hui <teohhanhui@gmail.com> | ||
| */ | ||
| class BooleanFilter extends AbstractFilter |
There was a problem hiding this comment.
WDYT about making this class final?
| * | ||
| * @throws \invalidArgumentException | ||
| */ | ||
| // protected function applyFilter(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null) |
There was a problem hiding this comment.
Should be removed I suppose
|
Thanks for your time ! AH ah no Kevin doesn't mind :) |
|
If you need some help or explanation, you can always try to reach me on Sylius or Symfony slack. |
3 participants
Add a filtering system based on api platform work.
To begin with I have implemented the boolean filter.
Todo: