[FINNA-3708] Refactor LIDO record handling to use VuFindXml.#199
[FINNA-3708] Refactor LIDO record handling to use VuFindXml.#199EreMaijala wants to merge 2 commits into
Conversation
This improves namespace support for LIDO records.
mshroom
left a comment
mshroom
left a comment
There was a problem hiding this comment.
Muutama kohta tässä, mutta en ehtinyt katsoa vielä kokonaan
| if (!empty($result)) { | ||
| return $result; | ||
| } | ||
| if ('' !== ($result = $this->xmlDoc->firstValue($place, 'partOfPlace') ?? '')) { |
There was a problem hiding this comment.
partOfPlace ei sisällä tekstiä vaan pitää kutsua tätä getSubLocationia uudestaan niinkuin aiemmin. Tyyliin:
if ($partOfNode = $this->xmlDoc->first($place, 'partOfPlace') {
$result = $this->getSubLocation($partOfNode, true);
if ('' !== $result) {
return $result;
}
}
| = $this->xmlDoc->firstValue($placeNode, 'place/namePlaceSet/appellationValue') ?? ''; | ||
| if ('' !== $appellationValue) { | ||
| $mainPlace = $appellationValue; | ||
| $placeNode = $this->xmlDoc->first($placeNode, 'place'); |
There was a problem hiding this comment.
voisiko tässä $placeNode olla vaikka $subPlaceNode, ei toi nyt taida sotkea mitään mutta vaikeuttaa ymmärtämistä kun $placeNode on jo tuossa käytössä eri elementillä
| $path = 'lido/descriptiveMetadata/objectIdentificationWrap/repositoryWrap/repositorySet/workID'; | ||
| return $this->xmlDoc->firstValue(path: $path) ?? ''; |
There was a problem hiding this comment.
Nyt tämä palauttaa ekan workID:n vaikka se olisi tyhjä, kun aiemmin käytiin läpi kaikkien repositorySettien ensimmäinen workID
| $appellationValueNode = $this->xmlDoc->first($roleNode, 'actor/nameActorSet/appellationValue'); | ||
| if ($appellationValueNode) { |
There was a problem hiding this comment.
Vois yksinkertaistaa tarkistukseen jo tässä että '' !== firstValue($roleNode, 'actor/nameActorSet/appellationValue') koska appellationValueNodesta ei kuitenkaan myöhemmin tsekata mitään muuta
| yield 'lido 1.0 with lidoWrap' => [ | ||
| "<lidoWrap $schema10><lido><lidoRecID type=\"ITEM\">123</lidoRecID></lido></lidoWrap>", | ||
| "<lidoWrap $schema10><lido><lidoRecID type=\"ITEM\">123</lidoRecID></lido></lidoWrap>", | ||
| <<<XML |
There was a problem hiding this comment.
Tähän testiin vois lisätä myös keissin jossa lähtödatassa on namespacet
|
Huomiona vaan tähän että namespaceista LIDOssa voi esiintyä lisäksi
Voisi myös vielä sitten testata haravointia myös ilman namespacejen siivoamista, että toimiiko kaikki oletetusti... |
Co-authored-by: Minna Rönkä <32199029+mshroom@users.noreply.github.com>
2 participants
This improves namespace support for LIDO records.