#98 - Allow applying further filters after DoctrineProxyFilter #101
Conversation
|
ping @theofidry (sorry :( ) |
There was a problem hiding this comment.
Apologize for the late review and thank you very much for your contribution!
There is two minor naming changes I would like you to make, otherwise I think it's only missing a mention in the doc. Maybe a not in the Doctrine filter doc + adding a mention in the filter doc that except a few exceptions listed in the filter descriptions, matching a filter will stop the chain of filters (i.e. the next ones will not be applied)
src/DeepCopy/Filter/ObjectFilter.php
Outdated
| /** | ||
| * Defines a filter that update the source object state only. | ||
| */ | ||
| interface ObjectFilter extends Filter |
There was a problem hiding this comment.
How about ChainableFilter as to any filter implementing this interface will not stop the chain of filters, i.e. if there is a filter registered after this one it will be applied?
src/DeepCopy/DeepCopy.php
Outdated
| @@ -207,6 +208,8 @@ private function copyObjectProperty($object, ReflectionProperty $property) | |||
| return; | |||
| } | |||
|
|
|||
| $hasAppliedFilter = false; | |||
There was a problem hiding this comment.
Minor but I would prefer$filterWasApplied
fsevestre
force-pushed
the
98-filters-doctrine-proxy
branch
from
8f3e918
to
53e84ee
Compare
|
|
||
| $copier = new DeepCopy(); | ||
| $copier->addFilter(new DoctrineProxyFilter(), new DoctrineProxyMatcher()); | ||
| $copier->addFilter(new SetNullFilter(), new PropertyNameMatcher('id')); |
There was a problem hiding this comment.
I'm not sure if I should add this filter since we are in the DoctrineProxyFilter part, but I believe that they will be used together most of the time.
|
@theofidry Comments addressed. |
fsevestre
force-pushed
the
98-filters-doctrine-proxy
branch
from
53e84ee
to
6fa0e56
Compare
|
All good :) |
|
Actually this is a BC break. I've released 1.8.0 and merge 1.x back to 2.x. 2.x is now the master in which we can merge your PR. I'll handle the merge tonight or tomorrow, I hope we can push some further changes in 2.x and have a new release soon :) |
|
It can be targeted to 2.x now :) |
fsevestre
force-pushed
the
98-filters-doctrine-proxy
branch
from
6fa0e56
to
93a74d9
Compare
|
@theofidry Done. |
|
Many thanks! |
Closes #98
This is a proposal for allowing applying further filters after
DoctrineProxyFilter.If you have a better interface name idea, I will be happy to rename it.