Injected RichText Validator from DIC into RichText Converter #91
Conversation
There was a problem hiding this comment.
This is rather complicated, because this bundle is supposed to work with eZ Platform 1.13 and 2.x. RichText Bundle has been introduced in v2.4. I think we need to stick to old deprecated ezpublish.fieldType.ezrichtext.validator.docbook.
When RichText bundle is enabled, this service gets replaced by the RichText one anyway.
Alternatively we could release new major here which would support ezpublish-kernel:^7.4 and require additionally ezplatform-richtext. But that's up to @andrerom or @vidarl
Anyway, it's a very good idea to inject that, just the old service. Remarks:
| public function __construct($apiRepository = null, LoggerInterface $logger = null) | ||
| { | ||
| public function __construct( | ||
| ValidatorInterface $validator = null, |
There was a problem hiding this comment.
It's safer to place new argument as a last one
| @@ -64,17 +64,22 @@ class RichText implements Converter | |||
|
|
|||
| /** | |||
| * RichText constructor. | |||
| * @param null $validator | |||
There was a problem hiding this comment.
Hmm, I see we omitted type-hinting, it should be rather null|<FQCN of an actual class>
The same goes for Repository, but this is our mistake 😅
|
@alongosz please review |
alongosz
changed the title
|
@alongosz requested changes are implemented. Please review one more time. |
// maintainer update:
Injecting proper DIC service will result in implicit swapping of this Validator when RichText bundle is enabled (it replaced
eZ\Publish\Core\FieldType\RichText).Note: if we intend to somehow support this bundle still with 3.0, then we will have to bump requirements and inject proper service from the RichText bundle