Bug 35588

Summary: marcrecord2csv retrieves authorised values incorrectly for fields
Product: Koha Reporter: David Cook <dcook>
Component: ToolsAssignee: David Cook <dcook>
Status: Pushed to oldstable --- QA Contact: Julian Maurice <julian.maurice>
Severity: normal    
Priority: P5 - low CC: david, fridolin.somers, julian.maurice, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35579
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the CSV export of records so that authorized values are exported correctly. It ensures that the authorized value descriptions looked up are for the correct field/subfield designated in the CSV profile. Example: If the 942$s (Serial record flag) for a record has a value of "1", it was previously exported as "Yes" even though it wasn't an authorized value.
Version(s) released in:
24.05.00,23.11.02,23.05.08
Bug Depends on: 35579    
Bug Blocks:    
Attachments: Bug 35588: Scope authorised value desc lookup according to field/subfield
Bug 35588: Scope authorised value desc lookup according to field/subfield
Bug 35588: Scope authorised value desc lookup according to field/subfield
Bug 35588: Tidy up t/db_dependent/AuthorisedValues.t

Description David Cook 2023-12-18 00:01:10 UTC
If marcrecord2csv takes a field and not a field/subfield combination, it will fetch all the authorised values for all subfields.

However, since the hashref is keyed against the authorised values themselves, authorised values for different categories can overwrite each other, which means the wrong description can potentially be output.
Comment 1 David Cook 2023-12-18 00:40:15 UTC
Created attachment 159937 [details] [review]
Bug 35588: Scope authorised value desc lookup according to field/subfield

This patch ensures that the authorised value descriptions looked up
are for the correct field/subfield designated in the CSV profile.

Test plan:
0. Do not apply the patch yet!
1. Go to http://localhost:8081/cgi-bin/koha/tools/csv-profiles.pl
2. Add a CSV profile with the default values and using the following
for the "Profile MARC fields":
000|001|003|005|006|007|008|010|015|016|020|022|040|050|082|100|110|
111|130|240|243|245|246|247|250|260|300|336|337|338|440|490|500|501|
505|520|530|600|610|611|630|648|650|651|690|700|710|711|856|887|942|995

3. Create a new List
4. Add all the database's bibs to that list using SQL like the following (where the shelfnumber equals the number for your list):
insert into virtualshelfcontents (shelfnumber,biblionumber,borrowernumber) select 1,biblionumber,51 from biblio;

5. Go to that list in the staff interface
6. Download that list using your CSV profile

7. Apply the patch
8. koha-plack --reload kohadev

9. Download that list using your CSV profile
10. Note that the output is a little slower, but still quick enough.
11. Note that the output is a bit different. For records like "Gairm",
the output will end in "1" instead of "Yes". This is correct, because
this subfield isn't an authorised value!!!
Comment 2 David Nind 2023-12-18 19:12:11 UTC
Created attachment 159978 [details] [review]
Bug 35588: Scope authorised value desc lookup according to field/subfield

This patch ensures that the authorised value descriptions looked up
are for the correct field/subfield designated in the CSV profile.

Test plan:
0. Do not apply the patch yet!
1. Go to http://localhost:8081/cgi-bin/koha/tools/csv-profiles.pl
2. Add a CSV profile with the default values and using the following
for the "Profile MARC fields":
000|001|003|005|006|007|008|010|015|016|020|022|040|050|082|100|110|
111|130|240|243|245|246|247|250|260|300|336|337|338|440|490|500|501|
505|520|530|600|610|611|630|648|650|651|690|700|710|711|856|887|942|995

3. Create a new List
4. Add all the database's bibs to that list using SQL like the following (where the shelfnumber equals the number for your list):
insert into virtualshelfcontents (shelfnumber,biblionumber,borrowernumber) select 1,biblionumber,51 from biblio;

5. Go to that list in the staff interface
6. Download that list using your CSV profile

7. Apply the patch
8. koha-plack --reload kohadev

9. Download that list using your CSV profile
10. Note that the output is a little slower, but still quick enough.
11. Note that the output is a bit different. For records like "Gairm",
the output will end in "1" instead of "Yes". This is correct, because
this subfield isn't an authorised value!!!

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2023-12-18 19:24:51 UTC
Testing notes (using KTD):

1. Download speeds (using Web Developer Tools > Network feature in Firefox):
   . Before: 1 request  270.81 kB / 336 B transferred  Finish: 47.14 s
   . Refreshed browser, flush_memcached, restart_all (so everything fresh)
   . After : 1 request  270.80 kB / 336 B transferred  Finish: 4.23 s

2. Compared downloaded files using a diff tool - differences as indicated in step 11 noted.

I had a go at the release notes, but this may need some more work!
Comment 4 David Cook 2023-12-19 03:06:24 UTC
(In reply to David Nind from comment #3)
> Testing notes (using KTD):
> 
> 1. Download speeds (using Web Developer Tools > Network feature in Firefox):
>    . Before: 1 request  270.81 kB / 336 B transferred  Finish: 47.14 s
>    . Refreshed browser, flush_memcached, restart_all (so everything fresh)
>    . After : 1 request  270.80 kB / 336 B transferred  Finish: 4.23 s
> 
> 2. Compared downloaded files using a diff tool - differences as indicated in
> step 11 noted.
> 
> I had a go at the release notes, but this may need some more work!

I think you did a great job on the release notes. Thanks for taking care of that!
Comment 5 Julian Maurice 2023-12-22 13:33:03 UTC
Created attachment 160237 [details] [review]
Bug 35588: Scope authorised value desc lookup according to field/subfield

This patch ensures that the authorised value descriptions looked up
are for the correct field/subfield designated in the CSV profile.

Test plan:
0. Do not apply the patch yet!
1. Go to http://localhost:8081/cgi-bin/koha/tools/csv-profiles.pl
2. Add a CSV profile with the default values and using the following
for the "Profile MARC fields":
000|001|003|005|006|007|008|010|015|016|020|022|040|050|082|100|110|
111|130|240|243|245|246|247|250|260|300|336|337|338|440|490|500|501|
505|520|530|600|610|611|630|648|650|651|690|700|710|711|856|887|942|995

3. Create a new List
4. Add all the database's bibs to that list using SQL like the following (where the shelfnumber equals the number for your list):
insert into virtualshelfcontents (shelfnumber,biblionumber,borrowernumber) select 1,biblionumber,51 from biblio;

5. Go to that list in the staff interface
6. Download that list using your CSV profile

7. Apply the patch
8. koha-plack --reload kohadev

9. Download that list using your CSV profile
10. Note that the output is a little slower, but still quick enough.
11. Note that the output is a bit different. For records like "Gairm",
the output will end in "1" instead of "Yes". This is correct, because
this subfield isn't an authorised value!!!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 6 Julian Maurice 2023-12-22 13:33:05 UTC
Created attachment 160238 [details] [review]
Bug 35588: Tidy up t/db_dependent/AuthorisedValues.t

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 7 Katrin Fischer 2023-12-28 11:17:14 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 8 Fridolin Somers 2024-01-03 16:24:08 UTC
Pushed to 23.11.x for 23.11.02
Comment 9 Lucas Gass 2024-01-12 22:05:49 UTC
Backported to 23.05.x for upcoming 23.05.08