IBX-11959: Added message template accessors to Translation value objects - #795
IBX-11959: Added message template accessors to Translation value objects#795tbialcz wants to merge 1 commit into
Conversation
92a2f41 to
a07c105
Compare
alongosz
left a comment
alongosz
left a comment
There was a problem hiding this comment.
For QA
Internal API only
src/contracts/FieldType/ValidationError.php is definitely not an internal API. Though it doesn't mean that there's something to QA. It's rather something for Doc release notes of 6.0.0
| * @return \Ibexa\Contracts\Core\Repository\Values\Translation\Message|\Ibexa\Contracts\Core\Repository\Values\Translation\Plural | ||
| */ | ||
| public function getTranslatableMessage(): Translation; |
There was a problem hiding this comment.
This looks like a code smell to me. We're declaring it as strict Translation but then say that we return some other types and in each impl. we define Translation again and say again that it's something else.
Can't covariance/contravariance or templating be used here?
There was a problem hiding this comment.
switched to a native Message|Plural return type on the contract, implementations covariantly narrow to Message. Docblocks removed
There was a problem hiding this comment.
Is it me or this is much worse, why other Translation interfaces implementation cannont be returned now from this method?
There was a problem hiding this comment.
you couldn't really return anything else anyway, Translation has only __toString() so everyone checks for Message or Plural to do their job. Subclassing Message/Plural still works
f9fa1f4 to
27baa0d
Compare
tbialcz
changed the title
27baa0d to
dc6195e
Compare
|
Steveb-p
left a comment
Steveb-p
left a comment
There was a problem hiding this comment.
LGTM.
We should consider swapping to https://symfony.com/doc/current/translation.html#translatable-objects soon, @alongosz @konradoboza
6 participants
Description:
Added a shared contract to the
Translationbase class:getMessageTemplate()+getValues(), implemented byMessageandPlural. Consumers no longer need to distinguish the two or rely on magicValueObject::__getproperty access.Needed by the follow-up
ibexa/userPR for IBX-11959 (ibexa/user#132). No behavior change.For QA:
No user-facing change, covered by unit tests. End-to-end QA on the
ibexa/userPR.Documentation:
For 6.0.0 release notes: new abstract methods
getMessageTemplate()andgetValues()onIbexa\Contracts\Core\Repository\Values\Translation— third-party subclasses must implement them.