MLSS-2212 & PREF-309 | Search Criteria Validators #265
Conversation
arielallon
commented
Aug 9, 2021
•
arielallon
commented
- Add machinery for Validator and Validator Decorators
- Add code to use those
- Add docs
- See https://github.com/neighborhoods/ListingService/pull/1950 for example usage
- Also refactor to DRY up some repeated code
…ame/Map/Repository - Update src to allow building new templates correctly
- Middleware is handling throwing the exception
arielallon
changed the title
arielallon
changed the title
|
|
||
| public function addFactory(DecoratorFactoryInterface $decoratorFactory): BuilderInterface | ||
| { | ||
| $factoryKey = str_replace('\\', '', get_class($decoratorFactory)); |
There was a problem hiding this comment.
potential blocker: Is the str_replace necessary? This could theoretically cause some issues if for example there was both a FooBar\Baz and a Foo\BarBaz, which should probably be allowed. I can't necessarily think of any current examples where that would definitely be a problem, but considering how often term overloading happens, it seems like it's something that's likely to occur at some point.
If slashes are a problem, we could probably get away with replacing them with another delimiter for the key.
| Neighborhoods\BuphaloTemplateTree\PrimaryActorName\Map\Repository\Validator\Builder\FactoryInterface: | ||
| class: PREFAB_PLACEHOLDER_VENDOR\PREFAB_PLACEHOLDER_PRODUCT\Prefab5\SearchCriteria\Validator\Builder\Factory |
There was a problem hiding this comment.
potential blocker: This looks like every repo Validator Builder Factory will have their own service but use the same class. Is that intended?
There was a problem hiding this comment.
from synchronous review:
pro:
having prefab build per primary actors versions of this could avoid the covariance/contravariance problem and might be more in line with how prefab operates otherwise.
cons:
might make it harder to have shared validators?
