diff --git a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter.php b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter.php index c373e8e0f9dcfb6b958627efeac8a242f50d5b43..10bb8e6a7c086c5d25a8bbdc0691b6935edfed68 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter.php @@ -58,9 +58,75 @@ class RecordDataFormatter extends AbstractHelper * * @return int */ - public function specSortCallback($a, $b) + protected function sortCallback($a, $b) { - return ($a['pos'] ?? 0) <=> ($b['pos'] ?? 0); + // Sort on 'pos' with 'label' as tie-breaker. + return ($a['pos'] == $b['pos']) + ? $a['label'] <=> $b['label'] + : $a['pos'] <=> $b['pos']; + } + + /** + * Should we allow a value? (Always accepts non-empty values; for empty + * values, allows zero when configured to do so). + * + * @param mixed $value Data to check for zero value. + * @param array $options Rendering options. + * + * @return bool + */ + protected function allowValue($value, $options) + { + if (!empty($value)) { + return true; + } + $allowZero = $options['allowZero'] ?? true; + return $allowZero && ($value === 0 || $value === '0'); + } + + /** + * Return rendered text (or null if nothing to render). + * + * @param RecordDriver $driver Record driver object + * @param string $field Field being rendered (i.e. default label) + * @param mixed $data Data to render + * @param array $options Rendering options + * + * @return array + */ + protected function render($driver, $field, $data, $options) + { + // Check whether the data is worth rendering. + if (!$this->allowValue($data, $options)) { + return null; + } + + // Determine the rendering method to use, and bail out if it's illegal: + $method = empty($options['renderType']) + ? 'renderSimple' : 'render' . $options['renderType']; + if (!is_callable([$this, $method])) { + return null; + } + + // If the value evaluates false, we should double-check our zero handling: + $value = $this->$method($driver, $data, $options); + if (!$this->allowValue($value, $options)) { + return null; + } + + // Special case: if we received an array rather than a string, we should + // return it as-is (it probably came from renderMulti()). + if (is_array($value)) { + return $value; + } + + // Allow dynamic label override: + $label = is_callable($options['labelFunction'] ?? null) + ? call_user_func($options['labelFunction'], $data, $driver) + : $field; + $context = $options['context'] ?? []; + $pos = $options['pos'] ?? 0; + return [compact('label', 'value', 'context', 'pos')]; } /** @@ -73,39 +139,18 @@ class RecordDataFormatter extends AbstractHelper */ public function getData(RecordDriver $driver, array $spec) { - $result = []; - - // Sort the spec into order by position: - uasort($spec, [$this, 'specSortCallback']); - // Apply the spec: + $result = []; foreach ($spec as $field => $current) { - // Extract the relevant data from the driver. + // Extract the relevant data from the driver and try to render it. $data = $this->extractData($driver, $current); - $allowZero = $current['allowZero'] ?? true; - if (!empty($data) || ($allowZero && ($data === 0 || $data === '0'))) { - // Determine the rendering method to use with the second element - // of the current spec. - $renderMethod = empty($current['renderType']) - ? 'renderSimple' : 'render' . $current['renderType']; - - // Add the rendered data to the return value if it is non-empty: - if (is_callable([$this, $renderMethod])) { - $text = $this->$renderMethod($driver, $data, $current); - if (!$text && (!$allowZero || ($text !== 0 && $text !== '0'))) { - continue; - } - // Allow dynamic label override: - $label = is_callable($current['labelFunction'] ?? null) - ? call_user_func($current['labelFunction'], $data, $driver) - : $field; - $result[$label] = [ - 'value' => $text, - 'context' => $current['context'] ?? [], - ]; - } + $value = $this->render($driver, $field, $data, $current); + if ($value !== null) { + $result = array_merge($result, $value); } } + // Sort the result: + usort($result, [$this, 'sortCallback']); return $result; } @@ -189,6 +234,44 @@ class RecordDataFormatter extends AbstractHelper return $data; } + /** + * Render multiple lines for a single set of data. + * + * @param RecordDriver $driver Reoord driver object. + * @param mixed $data Data to render + * @param array $options Rendering options. + * + * @return array + */ + protected function renderMulti(RecordDriver $driver, $data, + array $options + ) { + // Make sure we have a callback for sorting the $data into groups... + $callback = $options['multiFunction'] ?? null; + if (!is_callable($callback)) { + throw new \Exception('Invalid multiFunction callback.'); + } + + // Adjust the options array so we can use it to call the standard + // render function on the grouped data.... + $defaultOptions = ['renderType' => $options['multiRenderType'] ?? 'Simple'] + + $options; + + // Collect the results: + $results = []; + $input = $callback($data, $options, $driver); + foreach (is_array($input) ? $input : [] as $current) { + $label = $current['label'] ?? ''; + $values = $current['values'] ?? null; + $currentOptions = ($current['options'] ?? []) + $defaultOptions; + $next = $this->render($driver, $label, $values, $currentOptions); + if ($next !== null) { + $results = array_merge($results, $next); + } + } + return $results; + } + /** * Render using the record view helper. * diff --git a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter/SpecBuilder.php b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter/SpecBuilder.php index 3d4eb622c62d521b8c4c0ef37e7d86eeb42c1e04..0f41fe528408bd6f90b2909822052c8bdbe561a8 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter/SpecBuilder.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatter/SpecBuilder.php @@ -75,7 +75,7 @@ class SpecBuilder * @param string $renderType Type of rendering to use to generate output * @param array $options Additional options * - * @return array + * @return void */ public function setLine($key, $dataMethod, $renderType = null, $options = []) { @@ -88,6 +88,22 @@ class SpecBuilder $this->spec[$key] = $options; } + /** + * Construct a multi-function template spec line. + * + * @param string $key Label to associate with this spec line + * @param string $dataMethod Method of data retrieval for rendering element + * @param Callable $callback Callback function for multi-processing + * @param array $options Additional options + * + * @return void + */ + public function setMultiLine($key, $dataMethod, $callback, $options = []) + { + $options['multiFunction'] = $callback; + return $this->setLine($key, $dataMethod, 'Multi', $options); + } + /** * Construct a record driver template spec line. * @@ -96,7 +112,7 @@ class SpecBuilder * @param string $template Record driver template to render with data * @param array $options Additional options * - * @return array + * @return void */ public function setTemplateLine($key, $dataMethod, $template, $options = []) { diff --git a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatterFactory.php b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatterFactory.php index 76ad3b7422f94e618bfe8cefa022f149f105bbbc..5f1655b8018dc917d44f993b91dc4cff445fa4c8 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatterFactory.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/RecordDataFormatterFactory.php @@ -77,6 +77,54 @@ class RecordDataFormatterFactory implements FactoryInterface return $helper; } + /** + * Get the callback function for processing authors. + * + * @return Callable + */ + protected function getAuthorFunction() + { + return function ($data, $options) { + // Lookup array of singular/plural labels (note that Other is always + // plural right now due to lack of translation strings). + $labels = [ + 'primary' => ['Main Author', 'Main Authors'], + 'corporate' => ['Corporate Author', 'Corporate Authors'], + 'secondary' => ['Other Authors', 'Other Authors'], + ]; + // Lookup array of schema labels. + $schemaLabels = [ + 'primary' => 'author', + 'corporate' => 'creator', + 'secondary' => 'contributor', + ]; + // Lookup array of sort orders. + $order = ['primary' => 1, 'corporate' => 2, 'secondary' => 3]; + + // Sort the data: + $final = []; + foreach ($data as $type => $values) { + $final[] = [ + 'label' => $labels[$type][count($values) == 1 ? 0 : 1], + 'values' => [$type => $values], + 'options' => [ + 'pos' => $options['pos'] + $order[$type], + 'renderType' => 'RecordDriverTemplate', + 'template' => 'data-authors.phtml', + 'context' => [ + 'type' => $type, + 'schemaLabel' => $schemaLabels[$type], + 'requiredDataFields' => [ + ['name' => 'role', 'prefix' => 'CreatorRoles::'] + ], + ], + ], + ]; + } + return $final; + }; + } + /** * Get default specifications for displaying data in collection-info metadata. * @@ -85,36 +133,8 @@ class RecordDataFormatterFactory implements FactoryInterface public function getDefaultCollectionInfoSpecs() { $spec = new RecordDataFormatter\SpecBuilder(); - $spec->setTemplateLine( - 'Main Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['primary']) > 1 - ? 'Main Authors' : 'Main Author'; - }, - 'context' => ['type' => 'primary', 'schemaLabel' => 'author'], - ] - ); - $spec->setTemplateLine( - 'Corporate Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['corporate']) > 1 - ? 'Corporate Authors' : 'Corporate Author'; - }, - 'context' => ['type' => 'corporate', 'schemaLabel' => 'creator'], - ] - ); - $spec->setTemplateLine( - 'Other Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'context' => [ - 'type' => 'secondary', 'schemaLabel' => 'contributor' - ], - ] + $spec->setMultiLine( + 'Authors', 'getDeduplicatedAuthors', $this->getAuthorFunction() ); $spec->setLine('Summary', 'getSummary'); $spec->setLine( @@ -153,36 +173,8 @@ class RecordDataFormatterFactory implements FactoryInterface { $spec = new RecordDataFormatter\SpecBuilder(); $spec->setLine('Summary', 'getSummary'); - $spec->setTemplateLine( - 'Main Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['primary']) > 1 - ? 'Main Authors' : 'Main Author'; - }, - 'context' => ['type' => 'primary', 'schemaLabel' => 'author'], - ] - ); - $spec->setTemplateLine( - 'Corporate Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['corporate']) > 1 - ? 'Corporate Authors' : 'Corporate Author'; - }, - 'context' => ['type' => 'corporate', 'schemaLabel' => 'creator'], - ] - ); - $spec->setTemplateLine( - 'Other Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'context' => [ - 'type' => 'secondary', 'schemaLabel' => 'contributor' - ], - ] + $spec->setMultiLine( + 'Authors', 'getDeduplicatedAuthors', $this->getAuthorFunction() ); $spec->setLine('Language', 'getLanguages'); $spec->setLine( @@ -211,52 +203,8 @@ class RecordDataFormatterFactory implements FactoryInterface $spec->setLine( 'Previous Title', 'getPreviousTitles', null, ['recordLink' => 'title'] ); - $spec->setTemplateLine( - 'Main Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['primary']) > 1 - ? 'Main Authors' : 'Main Author'; - }, - 'context' => [ - 'type' => 'primary', - 'schemaLabel' => 'author', - 'requiredDataFields' => [ - ['name' => 'role', 'prefix' => 'CreatorRoles::'] - ] - ] - ] - ); - $spec->setTemplateLine( - 'Corporate Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'labelFunction' => function ($data) { - return count($data['corporate']) > 1 - ? 'Corporate Authors' : 'Corporate Author'; - }, - 'context' => [ - 'type' => 'corporate', - 'schemaLabel' => 'creator', - 'requiredDataFields' => [ - ['name' => 'role', 'prefix' => 'CreatorRoles::'] - ] - ] - ] - ); - $spec->setTemplateLine( - 'Other Authors', 'getDeduplicatedAuthors', 'data-authors.phtml', - [ - 'useCache' => true, - 'context' => [ - 'type' => 'secondary', - 'schemaLabel' => 'contributor', - 'requiredDataFields' => [ - ['name' => 'role', 'prefix' => 'CreatorRoles::'] - ] - ], - ] + $spec->setMultiLine( + 'Authors', 'getDeduplicatedAuthors', $this->getAuthorFunction() ); $spec->setLine( 'Format', 'getFormats', 'RecordHelper', diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/RecordDataFormatterTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/RecordDataFormatterTest.php index 7bea54ce05f704fe9301feb6fe559d86e7f45e66..67a65e1db5b246570bfb4ba39e834284b1a6c512 100644 --- a/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/RecordDataFormatterTest.php +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/RecordDataFormatterTest.php @@ -146,6 +146,39 @@ class RecordDataFormatterTest extends \VuFindTest\Unit\ViewHelperTestCase return $formatter; } + /** + * Find a result in the results array. + * + * @param string $needle Result to look up. + * @param array $haystack Result set. + * + * @return mixed + */ + protected function findResult($needle, $haystack) + { + foreach ($haystack as $current) { + if ($current['label'] == $needle) { + return $current; + } + } + return null; + } + + /** + * Extract labels from a results array. + * + * @param array $results Results to process. + * + * @return array + */ + protected function getLabels($results) + { + $callback = function ($c) { + return $c['label']; + }; + return array_map($callback, $results); + } + /** * Test citation generation * @@ -157,9 +190,101 @@ class RecordDataFormatterTest extends \VuFindTest\Unit\ViewHelperTestCase $spec = $formatter->getDefaults('core'); $spec['Building'] = [ 'dataMethod' => 'getBuilding', 'pos' => 0, 'context' => ['foo' => 1], - 'translationTextDomain' => 'prefix_' + 'translationTextDomain' => 'prefix_', + ]; + $spec['MultiTest'] = [ + 'dataMethod' => 'getFormats', + 'renderType' => 'Multi', + 'pos' => 1000, + 'multiFunction' => function ($data) { + return [ + [ + 'label' => 'Multi Data', + 'values' => $data, + ], + [ + 'label' => 'Multi Count', + 'values' => count($data), + ], + ]; + } + ]; + $spec['MultiEmptyArrayTest'] = [ + 'dataMethod' => true, + 'renderType' => 'Multi', + 'pos' => 2000, + 'multiFunction' => function () { + return []; + } + ]; + $spec['MultiNullTest'] = [ + 'dataMethod' => true, + 'renderType' => 'Multi', + 'pos' => 2000, + 'multiFunction' => function () { + return null; + } + ]; + $spec['MultiNullInArrayWithZeroTest'] = [ + 'dataMethod' => true, + 'renderType' => 'Multi', + 'pos' => 2000, + 'allowZero' => false, + 'multiFunction' => function () { + return [ + [ + 'label' => 'Null', + 'values' => null, + ], + [ + 'label' => 'ZeroBlocked', + 'values' => 0, + ] + ]; + } + ]; + $spec['MultiNullInArrayWithZeroAllowedTest'] = [ + 'dataMethod' => true, + 'renderType' => 'Multi', + 'pos' => 2000, + 'allowZero' => true, + 'multiFunction' => function () { + return [ + [ + 'label' => 'Null', + 'values' => null, + ], + [ + 'label' => 'ZeroAllowed', + 'values' => 0, + ] + ]; + } + ]; + $spec['MultiWithSortPos'] = [ + 'dataMethod' => true, + 'renderType' => 'Multi', + 'pos' => 0, + 'multiFunction' => function () { + return [ + [ + 'label' => 'b', + 'values' => 'b', + 'options' => ['pos' => 3000] + ], + [ + 'label' => 'a', + 'values' => 'a', + 'options' => ['pos' => 3000] + ], + [ + 'label' => 'c', + 'values' => 'c', + 'options' => ['pos' => 2999] + ], + ]; + } ]; - $expected = [ 'Building' => 'prefix_0', 'Published in' => '0', @@ -170,28 +295,43 @@ class RecordDataFormatterTest extends \VuFindTest\Unit\ViewHelperTestCase 'Published' => 'Centro di Studi Vichiani, 1992', 'Edition' => 'Fictional edition.', 'Series' => 'Vico, Giambattista, 1668-1744. Works. 1982 ;', + 'Multi Count' => 1, + 'Multi Data' => 'Book', 'Subjects' => 'Naples (Kingdom) History Spanish rule, 1442-1707 Sources', 'Online Access' => 'http://fictional.com/sample/url', 'Tags' => 'Add Tag No Tags, Be the first to tag this record!', + 'ZeroAllowed' => 0, + 'c' => 'c', + 'a' => 'a', + 'b' => 'b', ]; $driver = $this->getDriver(); $results = $formatter->getData($driver, $spec); // Check for expected array keys - $this->assertEquals(array_keys($expected), array_keys($results)); + $this->assertEquals(array_keys($expected), $this->getLabels($results)); // Check for expected text (with markup stripped) foreach ($expected as $key => $value) { $this->assertEquals( $value, - trim(preg_replace('/\s+/', ' ', strip_tags($results[$key]['value']))) + trim( + preg_replace( + '/\s+/', ' ', + strip_tags($this->findResult($key, $results)['value']) + ) + ) ); } // Check for exact markup in representative example: - $this->assertEquals('Italian<br />Latin', $results['Language']['value']); + $this->assertEquals( + 'Italian<br />Latin', $this->findResult('Language', $results)['value'] + ); // Check for context in Building: - $this->assertEquals(['foo' => 1], $results['Building']['context']); + $this->assertEquals( + ['foo' => 1], $this->findResult('Building', $results)['context'] + ); } } diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml index 81e8d3477cea7919401fd53032385ad4addfebaa..15eeae4ae7d8e2bc28765e94403af85f3165a485 100644 --- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml +++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml @@ -46,8 +46,8 @@ <?php if (!empty($fields)): ?> <table id="collectionInfo" class="table table-striped"> <caption class="sr-only"><?=$this->transEsc('Bibliographic Details')?></caption> - <?php foreach ($fields as $key => $current): ?> - <tr><th><?=$this->transEsc($key)?>:</th><td><?=$current['value']?></td></tr> + <?php foreach ($fields as $current): ?> + <tr><th><?=$this->transEsc($current['label'])?>:</th><td><?=$current['value']?></td></tr> <?php endforeach; ?> </table> <?php endif; ?> diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-record.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-record.phtml index 993e19f0d9ac412336ab3e53087cbfbd427411af..1b3fb042a9046c16e809c4ce1192d28149c16f0b 100644 --- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-record.phtml +++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-record.phtml @@ -8,8 +8,8 @@ <?php if (!empty($fields)): ?> <table class="table table-striped"> <caption class="sr-only"><?=$this->transEsc('Bibliographic Details')?></caption> - <?php foreach ($fields as $key => $current): ?> - <tr><th><?=$this->transEsc($key)?>:</th><td><?=$current['value']?></td></tr> + <?php foreach ($fields as $current): ?> + <tr><th><?=$this->transEsc($current['label'])?>:</th><td><?=$current['value']?></td></tr> <?php endforeach; ?> </table> <?php endif; ?> diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml index 024820810d9e8629bc5936604390f01d0e4e56c9..d9aa92a1f22e205815ea9ffea168f3736580b287 100644 --- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml +++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml @@ -58,8 +58,8 @@ <?php if (!empty($coreFields)): ?> <table class="table table-striped"> <caption class="sr-only"><?=$this->transEsc('Bibliographic Details')?></caption> - <?php foreach ($coreFields as $key => $current): ?> - <tr><th><?=$this->transEsc($key)?>:</th><td><?=$current['value']?></td></tr> + <?php foreach ($coreFields as $current): ?> + <tr><th><?=$this->transEsc($current['label'])?>:</th><td><?=$current['value']?></td></tr> <?php endforeach; ?> </table> <?php endif; ?> diff --git a/themes/bootstrap3/templates/RecordTab/description.phtml b/themes/bootstrap3/templates/RecordTab/description.phtml index 646423d21b55dc4b6a9745acab20f45d92a57259..dc203a6a8e017e654acb49ba903d6e8851c9a8bf 100644 --- a/themes/bootstrap3/templates/RecordTab/description.phtml +++ b/themes/bootstrap3/templates/RecordTab/description.phtml @@ -8,8 +8,8 @@ <table class="table table-striped"> <caption class="sr-only"><?=$this->transEsc('Description')?></caption> <?php if (!empty($mainFields)): ?> - <?php foreach ($mainFields as $key => $current): ?> - <tr><th><?=$this->transEsc($key)?>:</th><td><?=$current['value']?></td></tr> + <?php foreach ($mainFields as $current): ?> + <tr><th><?=$this->transEsc($current['label'])?>:</th><td><?=$current['value']?></td></tr> <?php endforeach; ?> <?php else: ?> <tr><td><?=$this->transEsc('no_description')?></td></tr>