35588 – marcrecord2csv retrieves authorised values incorrectly for fields

Bug 35588 - marcrecord2csv retrieves authorised values incorrectly for fields

Summary: marcrecord2csv retrieves authorised values incorrectly for fields

Status:	Pushed to oldstable

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Tools (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low normal
Assignee:	David Cook
QA Contact:	Julian Maurice

URL:
Keywords:

Depends on:	35579
Blocks:
	Show dependency tree / graph

Reported:	2023-12-18 00:01 UTC by David Cook
Modified:	2024-01-12 22:05 UTC (History)
CC List:	4 users (show)

See Also:	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
Circulation function:

Attachments
Bug 35588: Scope authorised value desc lookup according to field/subfield (7.55 KB, patch) 2023-12-18 00:40 UTC, David Cook	Details \| Diff \| Splinter Review
Bug 35588: Scope authorised value desc lookup according to field/subfield (7.60 KB, patch) 2023-12-18 19:12 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 35588: Scope authorised value desc lookup according to field/subfield (7.66 KB, patch) 2023-12-22 13:33 UTC, Julian Maurice	Details \| Diff \| Splinter Review
Bug 35588: Tidy up t/db_dependent/AuthorisedValues.t (4.35 KB, patch) 2023-12-22 13:33 UTC, Julian Maurice	Details \| Diff \| Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

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 (lukeg) 2024-01-12 22:05:49 UTC

Backported to 23.05.x for upcoming 23.05.08