fix: Property label showed in schema changed form #1091
Conversation
yaraslau-kavaliou
force-pushed
the
fix/REL-1370/property-lable-in-change-schema-form
branch
2 times, most recently
from
f99f4f7 to
77b9037
Compare
yaraslau-kavaliou
force-pushed
the
fix/REL-1370/property-lable-in-change-schema-form
branch
from
77b9037 to
d8a9f48
Compare
Version
There are 0 BREAKING CHANGE, 0 feature, 1 fix |
yaraslau-kavaliou
changed the title
There was a problem hiding this comment.
@yaraslau-kavaliou good work catching and fixing this issue. In general no anything blocking just a few things for a better look and feel.
| @@ -639,4 +645,15 @@ private function buildTriplesFromNode(core_kernel_classes_ContainerCollection $t | |||
|
|
|||
| return $tripleCollection; | |||
| } | |||
|
|
|||
| private function formatValue($value, array $literalValueProcessingCallback = []) | |||
There was a problem hiding this comment.
suggestion: can we use callable type hint here instead?
| return common_Utils::isUri($value) ? | ||
| $this->getModel()->getResource($value) | ||
| : new core_kernel_classes_Literal( | ||
| !empty($literalValueProcessingCallback) |
There was a problem hiding this comment.
nitpick: it would be more readable if we convert this ternary operator to nested IFs.
| : new core_kernel_classes_Literal($this->getLanguageProcessor()->parseTranslatedValue($value)); | ||
| $returnValue[$key][] = $this->formatValue( | ||
| $value, | ||
| [$this->getLanguageProcessor(), 'parseTranslatedValue'] |
There was a problem hiding this comment.
question: do we need to check if a specific property is language-dependent before trying to apply regexp to trim it? I'm wondering as the regexp pattern could be costly on big chunks of text. Of course, it is a tradeoff here as calling check for language dependency will also cost something.
| $returnValue[$key] ?? [], | ||
| $this->getLanguageProcessor()->filterByLanguage($value, [$dataLanguage, $defaultLanguage]) | ||
| ); | ||
| $returnValue[$key] = |
There was a problem hiding this comment.
nitpick: could you consider refactoring this part (extracting method refactoring in particular) as it becomes a bit difficult to read?
|
Review Checklist
|
augustas
deleted the
fix/REL-1370/property-lable-in-change-schema-form
branch
Ticket - https://oat-sa.atlassian.net/browse/REL-1370
Bug root cause:
stringtypes instead ofcore_kernel_classes_Literalfor literal values returning bygetPropertiesValuesmethod (in case of value itself is iterable)How to test: