EZP-30457: FieldType::getSortInfo should be implemented by impl. only if the FT is sortable #2677
Conversation
…ouldn't throw an exception in default implementation
| @@ -271,7 +271,7 @@ public function applyDefaultSettings(&$fieldSettings) | |||
| */ | |||
| protected function getSortInfo(Value $value) | |||
| { | |||
| throw new \RuntimeException('Not implemented, yet.'); | |||
| return false; | |||
There was a problem hiding this comment.
Imo null would be better
@pawbuj, I agree but this has deeper consequences. Right now false is internally treated as "no sort info".
@adamwojs Does this PR block anything?
Asking because I think we need to rectify this and indeed use null.
Use case: you have boolean FT which you want to sort by false first.
There was a problem hiding this comment.
@adamwojs Does this PR block anything?
Generic Field Type depends on this change.
Asking because I think we need to rectify this and indeed use null.
Use case: you have boolean FT which you want to sort by false first.
null is also allowed value here, see \eZ\Publish\Core\FieldType\Null\Type::getSortInfo. Using false is just a "non-formal" convention for non-sortable FT.
There was a problem hiding this comment.
nullis also allowed value here, see\eZ\Publish\Core\FieldType\Null\Type::getSortInfo. Usingfalseis just a "non-formal" convention for non-sortable FT.
What are the implications of returning null then?
There was a problem hiding this comment.
There are no implications at this point. Value returned by getSortInfo is passed as \eZ\Publish\SPI\Persistence\Content\FieldValue::$sortKey to \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter implementation and needs to be handled there.
Suggestion applied in fa8fc75
…Info shouldn't throw an exception in default implementation
|
PR updated according to @pawbuj suggestion. |
masterMethod
eZ\Publish\Core\FieldType\FieldType::getSortInfoshould be implemented by Developer only if the FT is sortable.Currently, the default implementation throws a
RuntimeException: "Not implemented, yet.". Usually, the developer figures out that something is missing on the first try of adding FT to the CT ...TODO:
Implement tests.$ composer fix-cs).