From dad28efc6f9e373bc20fb3a74d1903fe1567c498 Mon Sep 17 00:00:00 2001
From: David Cook <dcook@prosentient.com.au>
Date: Mon, 18 Dec 2023 00:35:53 +0000
Subject: [PATCH] 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!!!
---
 C4/Record.pm                      | 10 ++++++++--
 t/db_dependent/AuthorisedValues.t | 28 +++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/C4/Record.pm b/C4/Record.pm
index 33d18b86bd..799499ec86 100644
--- a/C4/Record.pm
+++ b/C4/Record.pm
@@ -592,20 +592,26 @@ sub marcrecord2csv {
 
                 # Or a field
                 } else {
-                    my $authvalues = Koha::AuthorisedValues->get_descriptions_by_marc_field(
-                        { frameworkcode => $frameworkcode, tagfield => $tag->{fieldtag}, } );
 
                     foreach my $field ( @fields ) {
                         my $value;
 
                         # If it is a control field
                         if ($field->is_control_field) {
+                            my $authvalues = Koha::AuthorisedValues->get_descriptions_by_marc_field(
+                                { frameworkcode => $frameworkcode, tagfield => $tag->{fieldtag}, } );
                             $value = defined $authvalues->{$field->as_string} ? $authvalues->{$field->as_string} : $field->as_string;
                         } else {
                             # If it is a field, we gather all subfields, joined by the subfield separator
                             my @subvaluesarray;
                             my @subfields = $field->subfields;
                             foreach my $subfield (@subfields) {
+                                my $authvalues = Koha::AuthorisedValues->get_descriptions_by_marc_field(
+                                    {
+                                        frameworkcode => $frameworkcode, tagfield => $tag->{fieldtag},
+                                        tagsubfield   => $subfield->[0],
+                                    }
+                                );
                                 push (@subvaluesarray, defined $authvalues->{$subfield->[1]} ? $authvalues->{$subfield->[1]} : $subfield->[1]);
                             }
                             $value = join ($subfieldseparator, @subvaluesarray);
diff --git a/t/db_dependent/AuthorisedValues.t b/t/db_dependent/AuthorisedValues.t
index f4f57d1974..83332ac79b 100755
--- a/t/db_dependent/AuthorisedValues.t
+++ b/t/db_dependent/AuthorisedValues.t
@@ -156,17 +156,23 @@ subtest 'search_by_*_field + find_by_koha_field + get_description + authorised_v
     $mss->delete if $mss;
     $mss = Koha::MarcSubfieldStructures->search( { tagfield => 952, tagsubfield => '5', frameworkcode => '' } );
     $mss->delete if $mss;
+    $mss = Koha::MarcSubfieldStructures->search( { tagfield => '003', frameworkcode => '' } );
+    $mss->delete if $mss;
     Koha::AuthorisedValueCategory->new( { category_name => 'TEST' } )->store;
+    Koha::AuthorisedValueCategory->new( { category_name => 'CONTROL_TEST' } )->store;
     Koha::AuthorisedValueCategory->new( { category_name => 'ANOTHER_4_TESTS' } )->store;
     Koha::MarcSubfieldStructure->new( { tagfield => 952, tagsubfield => 'c', frameworkcode => '', authorised_value => 'TEST', kohafield => 'items.location' } )->store;
     Koha::MarcSubfieldStructure->new( { tagfield => 952, tagsubfield => 'c', frameworkcode => 'ACQ', authorised_value => 'TEST', kohafield => 'items.location' } )->store;
     Koha::MarcSubfieldStructure->new( { tagfield => 952, tagsubfield => 'd', frameworkcode => '', authorised_value => 'ANOTHER_4_TESTS', kohafield => 'items.another_field' } )->store;
     Koha::MarcSubfieldStructure->new( { tagfield => 952, tagsubfield => '5', frameworkcode => '', authorised_value => 'restricted_for_testing', kohafield => 'items.restricted' } )->store;
+    Koha::MarcSubfieldStructure->new( { tagfield => '003', frameworkcode => '', authorised_value => 'CONTROL_TEST', } )->store;
     Koha::AuthorisedValue->new( { category => 'TEST', authorised_value => 'location_1', lib => 'location_1' } )->store;
     Koha::AuthorisedValue->new( { category => 'TEST', authorised_value => 'location_2', lib => 'location_2' } )->store;
     Koha::AuthorisedValue->new( { category => 'TEST', authorised_value => 'location_3', lib => 'location_3' } )->store;
     Koha::AuthorisedValue->new( { category => 'ANOTHER_4_TESTS', authorised_value => 'an_av' } )->store;
     Koha::AuthorisedValue->new( { category => 'ANOTHER_4_TESTS', authorised_value => 'another_av' } )->store;
+    Koha::AuthorisedValue->new( { category => 'CONTROL_TEST', authorised_value => 'lib1', lib => 'lib1' } )->store;
+    Koha::AuthorisedValue->new( { category => 'CONTROL_TEST', authorised_value => 'lib2', lib => 'lib2' } )->store;
     subtest 'search_by_marc_field' => sub {
         plan tests => 4;
         my $avs;
@@ -254,7 +260,23 @@ subtest 'search_by_*_field + find_by_koha_field + get_description + authorised_v
     };
 
     subtest 'get_descriptions_by_marc_field' => sub {
-        plan tests => 1;
+        plan tests => 4;
+
+        my $control_descriptions = Koha::AuthorisedValues->get_descriptions_by_marc_field(
+            { frameworkcode => '', tagfield => '003', } );
+        is_deeply(
+            $control_descriptions,
+            {
+                'lib1' => 'lib1',
+                'lib2' => 'lib2',
+            },
+        );
+
+        my $control_descriptions_cached = Koha::AuthorisedValues->get_descriptions_by_marc_field(
+            { frameworkcode => '', tagfield => '003', } );
+
+        is("$control_descriptions","$control_descriptions_cached","Same memory address used proves cached control desc data");
+
         my $descriptions = Koha::AuthorisedValues->get_descriptions_by_marc_field(
             { frameworkcode => '', tagfield => '952', tagsubfield => 'c' } );
         is_deeply(
@@ -265,6 +287,10 @@ subtest 'search_by_*_field + find_by_koha_field + get_description + authorised_v
                 'location_3' => 'location_3',
             },
         );
+
+        my $descriptions_cached = Koha::AuthorisedValues->get_descriptions_by_marc_field(
+            { frameworkcode => '', tagfield => '952', tagsubfield => 'c' } );
+        is("$descriptions","$descriptions_cached","Same memory address used proves cached desc data");
     };
 
     subtest 'authorised_values' => sub {
-- 
2.30.2