From 096f0ada3a8205d54b1f04a7bcb620ae0091b7c5 Mon Sep 17 00:00:00 2001 From: Kyle Hall Date: Wed, 8 Mar 2023 09:53:07 -0500 Subject: [PATCH] Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial Content-Type: text/plain; charset=utf-8 There is no need for this code to have a hard coded list of fields directly in the code. Any invalid keys would be skipped anyway. If we refactor this code then adding new fields will be much simpler. Test Plan: 1) Set up your MARCItemFieldsToOrder, verify everything is working 2) Apply this patch 3) Restart all the things! 4) Verify there has been no change to the MARCItemFieldsToOrder functionality Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Marcel de Rooy --- acqui/addorderiso2709.pl | 85 ++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 55 deletions(-) diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl index 250d9c2bb3..da20c05983 100755 --- a/acqui/addorderiso2709.pl +++ b/acqui/addorderiso2709.pl @@ -507,9 +507,10 @@ sub import_biblios_list { my $sort1 = $infos->{sort1}; my $sort2 = $infos->{sort2}; my $budget_id; - if($budget_code) { + + if ($budget_code) { my $biblio_budget = GetBudgetByCode($budget_code); - if($biblio_budget) { + if ($biblio_budget) { $budget_id = $biblio_budget->{budget_id}; } } @@ -517,56 +518,30 @@ sub import_biblios_list { # Items my @itemlist = (); my $all_items_quantity = 0; - my $alliteminfos = get_infos_syspref_on_item('MarcItemFieldsToOrder', $marcrecord, ['homebranch', 'holdingbranch', 'itype', 'nonpublic_note', 'public_note', 'loc', 'ccode', 'notforloan', 'uri', 'copyno', 'price', 'replacementprice', 'itemcallnumber', 'quantity', 'budget_code']); + my $alliteminfos = get_marc_item_fields_to_order($marcrecord); if ($alliteminfos != -1) { foreach my $iteminfos (@$alliteminfos) { - my $item_homebranch = $iteminfos->{homebranch}; - my $item_holdingbranch = $iteminfos->{holdingbranch}; - my $item_itype = $iteminfos->{itype}; - my $item_nonpublic_note = $iteminfos->{nonpublic_note}; - my $item_public_note = $iteminfos->{public_note}; - my $item_loc = $iteminfos->{loc}; - my $item_ccode = $iteminfos->{ccode}; - my $item_notforloan = $iteminfos->{notforloan}; - my $item_uri = $iteminfos->{uri}; - my $item_copyno = $iteminfos->{copyno}; - my $item_quantity = $iteminfos->{quantity} || 1; - my $item_budget_code = $iteminfos->{budget_code}; - my $item_budget_id; - if ( $iteminfos->{budget_code} ) { - my $item_budget = GetBudgetByCode( $iteminfos->{budget_code} ); - if ( $item_budget ) { - $item_budget_id = $item_budget->{budget_id}; - } + # Quantity is required, default to one if not supplied + my $quantity = delete $iteminfos->{quantity} || 1; + + # Handle incorrectly named original parameters for MarcItemFieldsToOrder + $iteminfos->{location} = delete $iteminfos->{loc} if $iteminfos->{loc}; + $iteminfos->{copynumber} = delete $iteminfos->{copyno} if $iteminfos->{copyno}; + + # Convert budge code to a budget id + my $item_budget_code = delete $iteminfos->{budget_code}; + if ( $item_budget_code ) { + my $item_budget = GetBudgetByCode( $item_budget_code ); + $iteminfos->{budget_id} = $item_budget->{budget_id} || $budget_id; } - my $item_price = $iteminfos->{price}; - my $item_replacement_price = $iteminfos->{replacementprice}; - my $item_callnumber = $iteminfos->{itemcallnumber}; - - for (my $i = 0; $i < $item_quantity; $i++) { - - my %itemrecord = ( - 'item_id' => $item_id++, - 'biblio_count' => $biblio_count, - 'homebranch' => $item_homebranch, - 'holdingbranch' => $item_holdingbranch, - 'itype' => $item_itype, - 'nonpublic_note' => $item_nonpublic_note, - 'public_note' => $item_public_note, - 'loc' => $item_loc, - 'ccode' => $item_ccode, - 'notforloan' => $item_notforloan, - 'uri' => $item_uri, - 'copyno' => $item_copyno, - 'quantity' => $item_quantity, - 'budget_id' => $item_budget_id || $budget_id, - 'itemprice' => $item_price || $price, - 'replacementprice' => $item_replacement_price || $replacementprice, - 'itemcallnumber' => $item_callnumber, - ); - $all_items_quantity++; - push @itemlist, \%itemrecord; + # Clone the item data for the needed quantity + # Add the incremented item id for each item in that quantity + for (my $i = 0; $i < $quantity; $i++) { + my $itemrecord = { %$iteminfos }; + $itemrecord->{item_id} = $item_id++; + $all_items_quantity++; + push @itemlist, $itemrecord; } } @@ -699,9 +674,10 @@ sub equal_number_of_fields { return $tags_count; } -sub get_infos_syspref_on_item { - my ($syspref_name, $record, $field_list) = @_; - my $syspref = C4::Context->preference($syspref_name); +sub get_marc_item_fields_to_order { + my ($record) = @_; + + my $syspref = C4::Context->preference('MarcItemFieldsToOrder'); $syspref = "$syspref\n\n"; # YAML is anal on ending \n. Surplus does not hurt my $yaml = eval { YAML::XS::Load(Encode::encode_utf8($syspref)); @@ -710,12 +686,12 @@ sub get_infos_syspref_on_item { warn "Unable to parse $syspref syspref : $@"; return (); } + my @result; my @tags_list; # Check tags in syspref definition - for my $field_name ( @$field_list ) { - next unless exists $yaml->{$field_name}; + for my $field_name ( keys %$yaml ) { my @fields = split /\|/, $yaml->{$field_name}; for my $field ( @fields ) { my ( $f, $sf ) = split /\$/, $field; @@ -741,8 +717,7 @@ sub get_infos_syspref_on_item { for (my $i = 0; $i < $tags_count; $i++) { my $r; - for my $field_name ( @$field_list ) { - next unless exists $yaml->{$field_name}; + for my $field_name ( keys %$yaml ) { my @fields = split /\|/, $yaml->{$field_name}; for my $field ( @fields ) { my ( $f, $sf ) = split /\$/, $field; -- 2.30.2