Bug 25082 - Unknown language code if 041 $a is linked to an authorized value list
Summary: Unknown language code if 041 $a is linked to an authorized value list
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic data support (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on: 20364
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-08 06:25 UTC by Pablo AB
Modified: 2020-05-18 09:47 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06


Attachments
Bug 25082: Make 041 descriptions display, if authorised value list is used (6.26 KB, patch)
2020-04-15 07:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25082: Make 041 descriptions display, if authorised value list is used (6.45 KB, patch)
2020-04-15 11:20 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 25082: Make 041 descriptions display, if authorised value list is used (6.52 KB, patch)
2020-05-05 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25082: Don't remove characters from string (2.26 KB, patch)
2020-05-05 21:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25082: Make 041 descriptions display, if authorised value list is used (6.59 KB, patch)
2020-05-07 11:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25082: Don't remove characters from string (2.33 KB, patch)
2020-05-07 11:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pablo AB 2020-04-08 06:25:32 UTC
From Bug 20364, comment 13:

When 041 $a (Language code of text/sound track or separate title) is associated with the LANG authorized value list, then the detail view of the record (OPAC and Intranet) show an "Unknown language code", even if the value of the subfield is a valid language code. This text comes from MARC21Languages.xsl.

This behavior was expected?

Maybe MARC21Languages.xsl code → Language list should be just a fallback if the code is not present/translated on associated list (probably LANG).
Comment 1 Katrin Fischer 2020-04-15 06:20:07 UTC
Hi Pablo,

I have no way to tell in the XSLT if yo uhave mapped the field to an authorised language list. The issue here is not that it's a valid language code - the issue is the XSLT contains some magic - so we end with the description in the XSLT that is processed (which will never be a valid language code...)

What I can do is say: if you don't know the code, display 041$a as it was stored. Someone else might be able to provide a more elaborate solution. I'll have a go and you can see if it works better for you.
Comment 2 Katrin Fischer 2020-04-15 06:21:06 UTC
> display 041$a as it was stored. 

Hm, stored is not quite right: as it was passed to the XSLT
Comment 3 Katrin Fischer 2020-04-15 06:48:10 UTC
What happens:
- When LANG (or any authorised value) is used with a subfield, Koha will replace the codes in the MARCXML with the descriptions, before it is processed by the XSLT, so

041$ager

Turns into:

041$aGerman

The XSLT currently expects language codes, not descriptions. So when it gets handed a description, it will turn it into "Unknown language".
Comment 4 Katrin Fischer 2020-04-15 07:00:05 UTC
Created attachment 102959 [details] [review]
Bug 25082: Make 041 descriptions display, if authorised value list is used

When 041$a or another subfield is linked to an authorised value list,
Koha will replace the codes with the descriptions from the authorised
value in the MARCXML before passing it to the XSLT.

The XSLT was made to translate codes into descriptions - as there is
no code now, this fails and 'unknown language' is shown.

The patch changes the handling so that when the XSLT is handled something
that doesn't match a known code, it displays the subfield without
processing.

To test:
- Create some records with 041.
  Examples: http://www.loc.gov/marc/bibliographic/bd041.html
- Verify this displays nicely in staff and OPAC
- Apply patch
- Verify display is unchanged
- Change one of your MARC framworks (use another framework than
  for your first test records)
  and link the 041 subfields to the LANG authorised value list
- Go to administration and create some entries for LANG with language
  codes and descriptions
- Catalog a 041 using the normal editor (not advanced)
- You can now use a pull down to select the values you want
- Verify the display is now 'Unknown language' for this record
- Apply patch
- Verify all records display the language descriptions now, using
  an authorised value or not
- Run misc/maintenance/generate_MARC21Languages.pl

Verify the output at the end reads like:
<xsl:otherwise>
    <!-- when 041\$a is mapped to an authorised value list, this will show the description -->
    <xsl:value-of select="\$code" />
</xsl:otherwise>
Comment 5 Bernardo Gonzalez Kriegel 2020-04-15 11:20:48 UTC
Created attachment 102988 [details] [review]
Bug 25082: Make 041 descriptions display, if authorised value list is used

When 041$a or another subfield is linked to an authorised value list,
Koha will replace the codes with the descriptions from the authorised
value in the MARCXML before passing it to the XSLT.

The XSLT was made to translate codes into descriptions - as there is
no code now, this fails and 'unknown language' is shown.

The patch changes the handling so that when the XSLT is handled something
that doesn't match a known code, it displays the subfield without
processing.

To test:
- Create some records with 041.
  Examples: http://www.loc.gov/marc/bibliographic/bd041.html
- Verify this displays nicely in staff and OPAC
- Apply patch
- Verify display is unchanged
- Remove the patch
- Change one of your MARC frameworks (use another framework than
  for your first test records)
  and link the 041 subfields to the LANG authorised value list
- Go to administration and create some entries for LANG with language
  codes and descriptions
- Catalog a 041 using the normal editor (not advanced)
- You can now use a pull down to select the values you want
- Verify the display is now 'Unknown language' for this record
- Apply patch
- Verify all records display the language descriptions now, using
  an authorised value or not
- Run misc/maintenance/generate_MARC21Languages.pl

Verify the output at the end reads like:
<xsl:otherwise>
    <!-- when 041\$a is mapped to an authorised value list, this will show the description -->
    <xsl:value-of select="\$code" />
</xsl:otherwise>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Amended test plan (add remove patch) to verify the error
Work as described following test plan, no errors.
Comment 6 Katrin Fischer 2020-04-15 11:26:14 UTC
Thx Bernardo! - already spotting more typos in the commit message *looks away quickly*
Comment 7 Pablo AB 2020-04-22 01:32:50 UTC
Thanks to both!

Will be backported to stable releases? I don't know where to check this info.

I'm not sure if this bug is related to Bug 20307.
Comment 8 Katrin Fischer 2020-04-22 06:29:02 UTC
I've marked it as a bug - if the Release maintainers agree it will be likely to be backported. But at the moment we still need it to go through QA first :)
Comment 9 Jonathan Druart 2020-05-04 13:40:16 UTC
Katrin, I see AV's code, should not it be displayed AV's description?
Comment 10 Katrin Fischer 2020-05-04 18:38:48 UTC
This should work like this:

When you don't use AVs, it will display the descriptions from the XSLT file.
If you are using AVs, it will display the descriptions of the AVs.

Does that make sense?

You should only see codes if you had entered a code that is neither in the XSLT nor in the AV list.
Comment 11 Katrin Fischer 2020-05-05 06:36:29 UTC
How did you test? Did you link your 041$a to the AV before testing this?

Note: Using the AV is not standard and there is no complete LANG list we ship with Koha, so I'd say not using the AV is more standard.
Comment 12 Jonathan Druart 2020-05-05 07:36:17 UTC
I created a new LANG code=xxx, description=description xxx
I set a 041$a to xxx and "xxx" was displayed on the detail page. I had other 041 with "correct" value (translated into correct language string).

IIRC I used
   041##$adut$afre$ager$aita$aspa$beng
and replaced fre with xxx
Comment 13 Jonathan Druart 2020-05-05 07:36:56 UTC
I may have missed something however, I can retest if you think I tested it wrong.
Comment 14 Katrin Fischer 2020-05-05 07:46:14 UTC
The AV translation will only work if you change your frameworks: You need to linkt he LANG authorised value category to 041$a
Comment 15 Jonathan Druart 2020-05-05 08:02:37 UTC
(In reply to Katrin Fischer from comment #14)
> The AV translation will only work if you change your frameworks: You need to
> linkt he LANG authorised value category to 041$a

Yes of course. I did that. Ok retrying now.
Comment 16 Jonathan Druart 2020-05-05 08:18:00 UTC
- Apply the patch
- Edit a record, add
041 _ _ ‡adut‡axxx‡ager‡aita‡aspa‡beng
- Create a LANG AV xxx="description xxx"
- link 041$a with LANG
- view record:
Language: , descriptionxxx, , , . Summary language: English. 
- remove the AV
Language: , , , , . Summary language: English. 
- remove the AV from $041$a
Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
- Create a LANG AV xxx="description xxx"
Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
(Hum??)
- restart_all!
Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
(Hum??)

I have no idea what's going on, but I never get the expected display, should be
Language: Dutch, description xxx, German, Italian, Spanish. Summary language: English. 

right?
Comment 17 Katrin Fischer 2020-05-05 09:01:27 UTC
Hm, I will have a go later.

- Apply the patch
- Edit a record, add
041 _ _ ‡adut‡axxx‡ager‡aita‡aspa‡beng
- Create a LANG AV xxx="description xxx"
- link 041$a with LANG

Correct.

- view record:
Language: , descriptionxxx, , , . Summary language: English. 

I'd have expected the codes to show here, but I wonder what is sent to the XSLT in these cases. If the code that switches the code for the description in the MARCXML record changes an unknown code to "", that would explain what you see there.

- remove the AV
Language: , , , , . Summary language: English.

See above.
 
- remove the AV from $041$a
Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 

That's the expected display. xxx is not in the XSLT file (no valid language code in MARC, so we display the code.

- Create a LANG AV xxx="description xxx"

Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
(Hum??)

Now it's not linked to the framework, so it has no effect. Displays the same as without the AV, this is expected.

- restart_all!

Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
(Hum??)

Still expected.
Comment 18 Jonathan Druart 2020-05-05 09:18:40 UTC
After this patch applied, LANG linked with 041$a, I think we are expecting

Language: Dutch, description xxx, German, Italian, Spanish. Summary language: English.
Comment 19 Katrin Fischer 2020-05-05 09:26:46 UTC
(In reply to Jonathan Druart from comment #18)
> After this patch applied, LANG linked with 041$a, I think we are expecting
> 
> Language: Dutch, description xxx, German, Italian, Spanish. Summary
> language: English.

What Koha does if you use an authorised value is a replacement in the MARCXML that is processed by XSLT.

The question here is: What happens if you haven't added a code to the AV?

Does it leave the code or does it ""?
Comment 20 Katrin Fischer 2020-05-05 09:28:33 UTC
Or in other words: What does the MARCXML look like that hits the XSLT?
Comment 21 Jonathan Druart 2020-05-05 09:49:52 UTC
Created attachment 104355 [details] [review]
Bug 25082: Make 041 descriptions display, if authorised value list is used

When 041$a or another subfield is linked to an authorised value list,
Koha will replace the codes with the descriptions from the authorised
value in the MARCXML before passing it to the XSLT.

The XSLT was made to translate codes into descriptions - as there is
no code now, this fails and 'unknown language' is shown.

The patch changes the handling so that when the XSLT is handled something
that doesn't match a known code, it displays the subfield without
processing.

To test:
- Create some records with 041.
  Examples: http://www.loc.gov/marc/bibliographic/bd041.html
- Verify this displays nicely in staff and OPAC
- Apply patch
- Verify display is unchanged
- Remove the patch
- Change one of your MARC frameworks (use another framework than
  for your first test records)
  and link the 041 subfields to the LANG authorised value list
- Go to administration and create some entries for LANG with language
  codes and descriptions
- Catalog a 041 using the normal editor (not advanced)
- You can now use a pull down to select the values you want
- Verify the display is now 'Unknown language' for this record
- Apply patch
- Verify all records display the language descriptions now, using
  an authorised value or not
- Run misc/maintenance/generate_MARC21Languages.pl

Verify the output at the end reads like:
<xsl:otherwise>
    <!-- when 041\$a is mapped to an authorised value list, this will show the description -->
    <xsl:value-of select="\$code" />
</xsl:otherwise>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Amended test plan (add remove patch) to verify the error
Work as described following test plan, no errors.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Jonathan Druart 2020-05-05 09:51:13 UTC
I now see

  "Language: descriptionxyz. Summary language: English."

Because $b is not linked. There is a missing space (should be "description xyz"), but it's something else, maybe bug 25373.
Comment 23 Jonathan Druart 2020-05-05 09:51:53 UTC
(I was editing the default framework, the biblio record was using BKS. The confusing was because the adv editor is displaying the dropdown list).
Comment 24 Jonathan Druart 2020-05-05 09:53:33 UTC
<cait1> I need to change this line here: 
<cait1> +        <xsl:with-param name="code" select="translate(., ' .-;|#', '')"/>
<cait1> please fail it - it doesn't work for language descriptions with mulitple words
<cait1> we don't want to replace the _
<cait1> the space
Comment 25 Katrin Fischer 2020-05-05 21:27:02 UTC
Created attachment 104379 [details] [review]
Bug 25082: Don't remove characters from string

At the moment, show-lang-041 will either get a code
or an AV description to deal with. If it's a description,
we don't want to remove spaces and other chars from it.

To test:
- Repeat tests from first patch with an AV in LANG category
  that consists of multiple words, has a - and other possible
  punctuation.
- The description should show correctly in OPAC and staff.
Comment 26 Jonathan Druart 2020-05-06 08:16:37 UTC
Sorry but still wrong.

Summary, using 041 _ _ ‡adut‡axxx‡ager‡aita‡aspa‡beng

* No mapping
Before:
Language: Dutch, Unknown language code, German, Italian, Spanish. Summary language: English. 
After: 
Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
Mapped with LANG (no AV)

* Mapped with LANG (no AVs yet)
Before:
Language: Unknown language code, Unknown language code, Unknown language code, Unknown language code, Unknown language code. Summary language: English. 
After:
Language: , , , , . Summary language: English. 

* Create 2 LANG (xxx=description xyz, spa=Spanish)
Before:
Language: Unknown language code, Unknown language code, Unknown language code, Unknown language code, Unknown language code. Summary language: English. 
After:
Language: , description xyz, , , Spanish. Summary language: English.
Comment 27 Katrin Fischer 2020-05-06 09:18:20 UTC
Hi Jonathan, 

trying to explain. Please have a look at

Please have a look at: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/XSLT.pm;h=1b3941c7fbed071c559d4de64c0df83c119743cc;hb=4071924f7f9438c344e878a86d29792f83c06704#l55

to start with. It explains what happens, when you map an authorised value to a MARC field.

(In reply to Jonathan Druart from comment #26)
> Sorry but still wrong.
> 
> Summary, using 041 _ _ ‡adut‡axxx‡ager‡aita‡aspa‡beng
> 
> * No mapping
> Before:
> Language: Dutch, Unknown language code, German, Italian, Spanish. Summary
> language: English. 
> After: 
> Language: Dutch, xxx, German, Italian, Spanish. Summary language: English. 
> Mapped with LANG (no AV)

We display the code instead of the "Unknown language", that was an intentional change.
The code displaying here will not always be a code, if the AV is mapped, it's the description.


> * Mapped with LANG (no AVs yet)
> Before:
> Language: Unknown language code, Unknown language code, Unknown language
> code, Unknown language code, Unknown language code. Summary language:
> English. 
> After:
> Language: , , , , . Summary language: English. 

I assume, and you might be able to verify, that transformMARCXML4XSLT is to blame for this, not the XSLT. I think if the language code doesn't exist in LANG, it gives us "". I cannot do anything about this on XSLT level.

> * Create 2 LANG (xxx=description xyz, spa=Spanish)
> Before:
> Language: Unknown language code, Unknown language code, Unknown language
> code, Unknown language code, Unknown language code. Summary language:
> English. 
> After:
> Language: , description xyz, , , Spanish. Summary language: English.

Ok, this is harder. 

What I could do, if we get "" from transformMARCXML4XSLT is a fallback if code = "" display "Unknown language". Would this be ok?
Comment 28 Jonathan Druart 2020-05-07 11:55:08 UTC
Created attachment 104497 [details] [review]
Bug 25082: Make 041 descriptions display, if authorised value list is used

When 041$a or another subfield is linked to an authorised value list,
Koha will replace the codes with the descriptions from the authorised
value in the MARCXML before passing it to the XSLT.

The XSLT was made to translate codes into descriptions - as there is
no code now, this fails and 'unknown language' is shown.

The patch changes the handling so that when the XSLT is handled something
that doesn't match a known code, it displays the subfield without
processing.

To test:
- Create some records with 041.
  Examples: http://www.loc.gov/marc/bibliographic/bd041.html
- Verify this displays nicely in staff and OPAC
- Apply patch
- Verify display is unchanged
- Remove the patch
- Change one of your MARC frameworks (use another framework than
  for your first test records)
  and link the 041 subfields to the LANG authorised value list
- Go to administration and create some entries for LANG with language
  codes and descriptions
- Catalog a 041 using the normal editor (not advanced)
- You can now use a pull down to select the values you want
- Verify the display is now 'Unknown language' for this record
- Apply patch
- Verify all records display the language descriptions now, using
  an authorised value or not
- Run misc/maintenance/generate_MARC21Languages.pl

Verify the output at the end reads like:
<xsl:otherwise>
    <!-- when 041\$a is mapped to an authorised value list, this will show the description -->
    <xsl:value-of select="\$code" />
</xsl:otherwise>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Amended test plan (add remove patch) to verify the error
Work as described following test plan, no errors.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Jonathan Druart 2020-05-07 11:55:12 UTC
Created attachment 104498 [details] [review]
Bug 25082: Don't remove characters from string

At the moment, show-lang-041 will either get a code
or an AV description to deal with. If it's a description,
we don't want to remove spaces and other chars from it.

To test:
- Repeat tests from first patch with an AV in LANG category
  that consists of multiple words, has a - and other possible
  punctuation.
- The description should show correctly in OPAC and staff.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Jonathan Druart 2020-05-07 11:55:40 UTC
Let this one go as it and see later if we need to deal with edge cases.
Comment 31 Martin Renvoize 2020-05-11 09:01:13 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 32 Joy Nelson 2020-05-11 22:08:47 UTC
Backported to 19.11.x for 19.11.06
Comment 33 Lucas Gass 2020-05-15 22:06:47 UTC
missing dependencies for 19.05.x, no backport