From bfb7ef76c0579fb917c84e511e5d7ff113ac43ce Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop Date: Thu, 28 Nov 2024 15:11:44 +0000 Subject: [PATCH] Bug 38547: Move code to Koha/Acquisitions and add more tests Test plan: 1) prove t/db_dependent/Koha/Acquisition/MarcOrder.t --- Koha/{ => Acquisition}/MarcOrder.pm | 15 +- Koha/{ => Acquisition}/MarcOrderAccount.pm | 4 +- Koha/{ => Acquisition}/MarcOrderAccounts.pm | 8 +- acqui/addorderiso2709.pl | 8 +- admin/marc_order_accounts.pl | 10 +- misc/cronjobs/marc_ordering_process.pl | 10 +- .../Koha/{ => Acquisition}/MarcOrder.t | 319 +++++++++++++++++- 7 files changed, 333 insertions(+), 41 deletions(-) rename Koha/{ => Acquisition}/MarcOrder.pm (99%) rename Koha/{ => Acquisition}/MarcOrderAccount.pm (90%) rename Koha/{ => Acquisition}/MarcOrderAccounts.pm (80%) rename t/db_dependent/Koha/{ => Acquisition}/MarcOrder.t (56%) diff --git a/Koha/MarcOrder.pm b/Koha/Acquisition/MarcOrder.pm similarity index 99% rename from Koha/MarcOrder.pm rename to Koha/Acquisition/MarcOrder.pm index eb2c13d3a3f..12ffda78f0c 100644 --- a/Koha/MarcOrder.pm +++ b/Koha/Acquisition/MarcOrder.pm @@ -1,4 +1,4 @@ -package Koha::MarcOrder; +package Koha::Acquisition::MarcOrder; # Copyright 2023, PTFS-Europe Ltd # @@ -19,7 +19,6 @@ package Koha::MarcOrder; use Modern::Perl; use Try::Tiny qw( catch try ); -use Net::FTP; use base qw(Koha::Object); @@ -64,7 +63,7 @@ use Koha::Plugins; =head1 NAME -Koha::MarcOrder - Koha Marc Order Object class +Koha::Acquisition::MarcOrder - Koha Marc Order Object class =head1 API @@ -74,7 +73,7 @@ Koha::MarcOrder - Koha Marc Order Object class =head3 create_order_lines_from_file - my $result = Koha::MarcOrder->create_order_lines_from_file($args); + my $result = Koha::Acquisition::MarcOrder->create_order_lines_from_file($args); Controller for file staging, basket creation and order line creation when using the cronjob in marc_ordering_process.pl @@ -166,7 +165,7 @@ sub create_order_lines_from_file { =head3 import_record_and_create_order_lines - my $result = Koha::MarcOrder->import_record_and_create_order_lines($args); + my $result = Koha::Acquisition::MarcOrder->import_record_and_create_order_lines($args); Controller for record import and order line creation when using the interface in addorderiso2709.pl @@ -529,7 +528,7 @@ sub add_items_from_import_record { =head3 match_file_to_account - my $file_match = Koha::MarcOrder->match_file_to_account({ + my $file_match = Koha::Acquisition::MarcOrder->match_file_to_account({ filename => $filename, filepath => $filepath, profile => $profile @@ -576,7 +575,7 @@ sub match_file_to_account { Fetches import batches matching the batch to be added to the basket and returns these to the template -Koha::MarcOrder->import_batches_list(); +Koha::Acquisition::MarcOrder->import_batches_list(); =cut @@ -626,7 +625,7 @@ sub import_batches_list { For an import batch, this function reads the files and creates all the relevant data pertaining to that file It then returns this to the template to be shown in the UI -Koha::MarcOrder->import_biblios_list( $cgiparams->{'import_batch_id'} ); +Koha::Acquisition::MarcOrder->import_biblios_list( $cgiparams->{'import_batch_id'} ); =cut diff --git a/Koha/MarcOrderAccount.pm b/Koha/Acquisition/MarcOrderAccount.pm similarity index 90% rename from Koha/MarcOrderAccount.pm rename to Koha/Acquisition/MarcOrderAccount.pm index 4c122200aa1..3ebdf070dcb 100644 --- a/Koha/MarcOrderAccount.pm +++ b/Koha/Acquisition/MarcOrderAccount.pm @@ -1,4 +1,4 @@ -package Koha::MarcOrderAccount; +package Koha::Acquisition::MarcOrderAccount; # This file is part of Koha. # @@ -23,7 +23,7 @@ use base qw(Koha::Object); =head1 NAME -Koha::MarcOrderAccount - Koha Marc Ordering Account Object class +Koha::Acquisition::MarcOrderAccount - Koha Marc Ordering Account Object class =head1 API diff --git a/Koha/MarcOrderAccounts.pm b/Koha/Acquisition/MarcOrderAccounts.pm similarity index 80% rename from Koha/MarcOrderAccounts.pm rename to Koha/Acquisition/MarcOrderAccounts.pm index f6ee407d658..9e55314f0b3 100644 --- a/Koha/MarcOrderAccounts.pm +++ b/Koha/Acquisition/MarcOrderAccounts.pm @@ -1,4 +1,4 @@ -package Koha::MarcOrderAccounts; +package Koha::Acquisition::MarcOrderAccounts; # This file is part of Koha. # @@ -18,13 +18,13 @@ package Koha::MarcOrderAccounts; use Modern::Perl; use Koha::Database; -use Koha::MarcOrderAccount; +use Koha::Acquisition::MarcOrderAccount; use base qw(Koha::Objects); =head1 NAME -Koha::MarcOrderAccount - Koha Marc Ordering Account Object class +Koha::Acquisition::MarcOrderAccount - Koha Marc Ordering Account Object class =head1 API @@ -45,7 +45,7 @@ sub _type { =cut sub object_class { - return 'Koha::MarcOrderAccount'; + return 'Koha::Acquisition::MarcOrderAccount'; } 1; diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl index 7f0f160bc2f..12cb7473f80 100755 --- a/acqui/addorderiso2709.pl +++ b/acqui/addorderiso2709.pl @@ -54,7 +54,7 @@ use Koha::Acquisition::Booksellers; use Koha::ImportBatches; use Koha::Import::Records; use Koha::Patrons; -use Koha::MarcOrder; +use Koha::Acquisition::MarcOrder; my $input = CGI->new; my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( @@ -95,7 +95,7 @@ if ( $op eq "" ) { $template->param( "basketno" => $cgiparams->{'basketno'} ); #display batches - my $batches_list = Koha::MarcOrder->import_batches_list(); + my $batches_list = Koha::Acquisition::MarcOrder->import_batches_list(); $template->param( batch_list => $batches_list->{list}, num_results => $batches_list->{num_results} @@ -116,7 +116,7 @@ if ( $op eq "" ) { bookseller => $bookseller, "allmatch" => $allmatch, ); - my $biblios_list = Koha::MarcOrder->import_biblios_list( $cgiparams->{'import_batch_id'} ); + my $biblios_list = Koha::Acquisition::MarcOrder->import_biblios_list( $cgiparams->{'import_batch_id'} ); my $overlay_action = $biblios_list->{overlay_action}; my $nomatch_action = $biblios_list->{nomatch_action}; my $item_action = $biblios_list->{item_action}; @@ -281,7 +281,7 @@ if ( $op eq "" ) { vendor => $bookseller, budget_id => $budget_id, }; - my $result = Koha::MarcOrder->import_record_and_create_order_lines($args); + my $result = Koha::Acquisition::MarcOrder->import_record_and_create_order_lines($args); $duplinbatch = $result->{duplicates_in_batch} if $result->{duplicates_in_batch}; next if $result->{skip}; # If a duplicate is found, or the import record wasn't selected it will be skipped diff --git a/admin/marc_order_accounts.pl b/admin/marc_order_accounts.pl index c95efe312dd..7f2bcae0640 100755 --- a/admin/marc_order_accounts.pl +++ b/admin/marc_order_accounts.pl @@ -32,10 +32,10 @@ use C4::Matcher; use Koha::UploadedFiles; use Koha::ImportBatchProfiles; -use Koha::MarcOrder; +use Koha::Acquisition::MarcOrder; use Koha::Acquisition::Booksellers; -use Koha::MarcOrderAccount; -use Koha::MarcOrderAccounts; +use Koha::Acquisition::MarcOrderAccount; +use Koha::Acquisition::MarcOrderAccounts; my $input = CGI->new; @@ -109,13 +109,15 @@ if ( $op eq 'acct_form' ) { } +$template->param( messages => \@messages, op => $op); + output_html_with_http_headers $input, $cookie, $template->output; sub show_account { my ( $input, $template ) = @_; my $acct_id = $input->param('id'); if ($acct_id) { - my $acct = Koha::MarcOrderAccounts->find($acct_id); + my $acct = Koha::Acquisition::MarcOrderAccounts->find($acct_id); if ($acct) { my $vendor = Koha::Acquisition::Booksellers->find( $acct->vendor_id ); $template->param( vendor => $vendor, account => $acct ); diff --git a/misc/cronjobs/marc_ordering_process.pl b/misc/cronjobs/marc_ordering_process.pl index 18084ad7245..d458218a49d 100755 --- a/misc/cronjobs/marc_ordering_process.pl +++ b/misc/cronjobs/marc_ordering_process.pl @@ -61,8 +61,8 @@ use Getopt::Long qw( GetOptions ); use File::Copy qw( copy move ); use Koha::Script -cron; -use Koha::MarcOrder; -use Koha::MarcOrderAccounts; +use Koha::Acquisition::MarcOrder; +use Koha::Acquisition::MarcOrderAccounts; use C4::Log qw( cronlogaction ); @@ -84,7 +84,7 @@ $verbose = 1 unless $verbose or $confirm; print "Test run only\n" unless $confirm; print "Fetching marc ordering accounts\n" if $verbose; -my @accounts = Koha::MarcOrderAccounts->search( +my @accounts = Koha::Acquisition::MarcOrderAccounts->search( {}, { join => [ 'vendor', 'budget' ] } )->as_list; @@ -116,13 +116,13 @@ foreach my $acct (@accounts) { agent => 'cron' }; if ( $acct->match_field && $acct->match_value ) { - my $file_match = Koha::MarcOrder->match_file_to_account($args); + my $file_match = Koha::Acquisition::MarcOrder->match_file_to_account($args); next if !$file_match; } if ($confirm) { say sprintf "Creating order lines from file %s", $filename if $verbose; - my $result = Koha::MarcOrder->create_order_lines_from_file($args); + my $result = Koha::Acquisition::MarcOrder->create_order_lines_from_file($args); if ( $result->{success} ) { $files_processed++; say sprintf "Successfully processed file: %s", $filename if $verbose; diff --git a/t/db_dependent/Koha/MarcOrder.t b/t/db_dependent/Koha/Acquisition/MarcOrder.t similarity index 56% rename from t/db_dependent/Koha/MarcOrder.t rename to t/db_dependent/Koha/Acquisition/MarcOrder.t index 8f2d01badb2..fe12dd7cec7 100755 --- a/t/db_dependent/Koha/MarcOrder.t +++ b/t/db_dependent/Koha/Acquisition/MarcOrder.t @@ -17,10 +17,10 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 9; -use Koha::MarcOrder; -use Koha::MarcOrderAccount; +use Koha::Acquisition::MarcOrder; +use Koha::Acquisition::MarcOrderAccount; use Koha::Acquisition::Baskets; use Koha::Acquisition::Bookseller; use MARC::Record; @@ -71,7 +71,7 @@ replacementprice: 975$v' ); my $marc_item_fields_to_order = @{ - Koha::MarcOrder::_get_syspref_mappings( + Koha::Acquisition::MarcOrder::_get_syspref_mappings( $record, 'MarcItemFieldsToOrder', ) }[0]; @@ -154,7 +154,7 @@ budget_code: 975$h' [ '975', ' ', ' ', p => 10, q => 1, h => 1 ], ); - my $marc_fields_to_order = Koha::MarcOrder::_get_syspref_mappings( + my $marc_fields_to_order = Koha::Acquisition::MarcOrder::_get_syspref_mappings( $record, 'MarcFieldsToOrder', ); @@ -206,7 +206,7 @@ subtest 'add_biblio_from_import_record()' => sub { my $import_record_id = C4::ImportBatch::AddBiblioToBatch( $import_batch_id, 0, $record, 'utf8', 0 ); my $import_record = Koha::Import::Records->find($import_record_id); - my $result = Koha::MarcOrder::add_biblio_from_import_record( + my $result = Koha::Acquisition::MarcOrder::add_biblio_from_import_record( { import_record => $import_record, matcher_id => $sample_import_batch->{matcher_id}, @@ -224,7 +224,7 @@ subtest 'add_biblio_from_import_record()' => sub { # Check that records are skipped if not selected when called from addorderiso2709.pl # Pass in an empty array and the record should be skipped my @import_record_id_selected = (); - my $result2 = Koha::MarcOrder::add_biblio_from_import_record( + my $result2 = Koha::Acquisition::MarcOrder::add_biblio_from_import_record( { import_record => $import_record, matcher_id => $sample_import_batch->{matcher_id}, @@ -254,7 +254,7 @@ subtest 'add_items_from_import_record() - addorderiso2709.pl' => sub { { budget_period_startdate => '2008-01-01', budget_period_enddate => '2008-12-31' , budget_period_active => 1 - , budget_period_description => "MAPERI" + # , budget_period_description => "MAPERI" } ); my $budgetid = C4::Budgets::AddBudget( @@ -304,7 +304,7 @@ subtest 'add_items_from_import_record() - addorderiso2709.pl' => sub { my $import_record = Koha::Import::Records->find($import_record_id); my @import_record_id_selected = ($import_record_id); - my $result = Koha::MarcOrder::add_biblio_from_import_record( + my $result = Koha::Acquisition::MarcOrder::add_biblio_from_import_record( { import_record => $import_record, matcher_id => $sample_import_batch->{matcher_id}, @@ -393,7 +393,7 @@ subtest 'add_items_from_import_record() - addorderiso2709.pl' => sub { 'enumchrons' => [] }; - my $itemnumbers = Koha::MarcOrder::add_items_from_import_record( + my $itemnumbers = Koha::Acquisition::MarcOrder::add_items_from_import_record( { record_result => $result->{record_result}, basket_id => $basket->basketno, @@ -456,7 +456,7 @@ subtest 'match_file_to_account' => sub { close $fh; - my $account1 = Koha::MarcOrderAccount->new( + my $account1 = Koha::Acquisition::MarcOrderAccount->new( { match_field => '975$p', match_value => '12345', @@ -465,7 +465,7 @@ subtest 'match_file_to_account' => sub { } )->store; - my $file_match = Koha::MarcOrder->match_file_to_account( + my $file_match = Koha::Acquisition::MarcOrder->match_file_to_account( { filename => $name, filepath => $name, @@ -475,7 +475,7 @@ subtest 'match_file_to_account' => sub { is( $file_match, 1, 'File matched correctly to the account' ); - my $account2 = Koha::MarcOrderAccount->new( + my $account2 = Koha::Acquisition::MarcOrderAccount->new( { match_field => '975$p', match_value => 'abcde', @@ -484,7 +484,7 @@ subtest 'match_file_to_account' => sub { } )->store; - my $file_match2 = Koha::MarcOrder->match_file_to_account( + my $file_match2 = Koha::Acquisition::MarcOrder->match_file_to_account( { filename => $name, filepath => $name, @@ -496,3 +496,294 @@ subtest 'match_file_to_account' => sub { $schema->storage->txn_rollback; }; + +subtest '_create_basket_for_file' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $bookseller = Koha::Acquisition::Bookseller->new( + { + name => "my vendor", + address1 => "bookseller's address", + phone => "0123456", + active => 1, + deliverytime => 5, + } + )->store; + + my $basket_count = Koha::Acquisition::Baskets->count; + + my $basketno = Koha::Acquisition::MarcOrder::_create_basket_for_file( + { + filename => 'test_marc_file.marcxml', + vendor_id => $bookseller->id, + } + ); + + my $new_basket_count = Koha::Acquisition::Baskets->count; + + is( $new_basket_count, $basket_count + 1, 'Basket was successfully created' ); + + my $basket = Koha::Acquisition::Baskets->find($basketno); + + is( $basket->basketname, 'test_marc_file.marcxml', 'Basket was successfully created' ); + + $schema->storage->txn_rollback; +}; + +subtest 'parse_input_into_order_line_fields' => sub { + plan tests => 8; + + $schema->storage->txn_begin; + + my $bpid = C4::Budgets::AddBudgetPeriod( + { + budget_period_startdate => '2008-01-01', budget_period_enddate => '2008-12-31' + , budget_period_active => 1 + } + ); + my $budgetid = C4::Budgets::AddBudget( + { + budget_code => "BC_1", + budget_name => "budget_name_test_1", + budget_period_id => $bpid, + } + ); + my $budgetid2 = C4::Budgets::AddBudget( + { + budget_code => "BC_2", + budget_name => "budget_name_test_2", + budget_period_id => $bpid, + } + ); + my $fund1 = C4::Budgets::GetBudget($budgetid); + my $fund2 = C4::Budgets::GetBudget($budgetid2); + my $budget_code_1 = $fund1->{budget_code}; + my $budget_code_2 = $fund2->{budget_code}; + + my $order_line_fields = Koha::Acquisition::MarcOrder::parse_input_into_order_line_fields( + { + agent => 'cron', + biblionumber => 1, + budget_id => 100, + basket_id => 101, + fields => { + quantity => 2, + c_quantity => 1, + homebranches => [ 1, 2 ], + holdingbranches => undef, + budget_codes => [ $budget_code_1, $budget_code_2 ], + } + } + ); + + is( + $order_line_fields->{loop_limit}, 2, + "Quantity has been read correctly" + ); + is( scalar( @{ $order_line_fields->{homebranch} } ), 2, "Home branches have been read correctly" ); + is( @{ $order_line_fields->{homebranch} }[0], 1, "Home branches has been read correctly" ); + is( scalar( @{ $order_line_fields->{holdingbranch} } ), 0, "Holding branches have been read correctly" ); + is( scalar( @{ $order_line_fields->{budget_code} } ), 2, "Budget codes have been read correctly" ); + is( @{ $order_line_fields->{budget_code} }[0], $budget_code_1, "Budget codes has been read correctly" ); + + # Test with no budget codes to check account budget_id assigned correctly + $order_line_fields = Koha::Acquisition::MarcOrder::parse_input_into_order_line_fields( + { + agent => 'cron', + biblionumber => 1, + budget_id => 100, + basket_id => 101, + fields => { + quantity => 2, + c_quantity => 1, + homebranches => [ 1, 2 ], + holdingbranches => undef, + budget_codes => [], + } + } + ); + + is( scalar( @{ $order_line_fields->{budget_code} } ), 2, "Budget codes have been read correctly" ); + is( @{ $order_line_fields->{budget_code} }[0], 100, "Budget codes has been read correctly" ); + + $schema->storage->txn_rollback; +}; + +subtest 'create_items_and_generate_order_hash' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + my $bpid = C4::Budgets::AddBudgetPeriod( + { + budget_period_startdate => '2008-01-01', budget_period_enddate => '2008-12-31' + , budget_period_active => 1 + } + ); + my $budgetid = C4::Budgets::AddBudget( + { + budget_code => "BC_1", + budget_name => "budget_name_test_1", + budget_period_id => $bpid, + } + ); + my $budgetid2 = C4::Budgets::AddBudget( + { + budget_code => "BC_2", + budget_name => "budget_name_test_2", + budget_period_id => $bpid, + } + ); + my $fund1 = C4::Budgets::GetBudget($budgetid); + my $fund2 = C4::Budgets::GetBudget($budgetid2); + my $budget_id_1 = $fund1->{budget_id}; + my $budget_id_2 = $fund2->{budget_id}; + + my $bookseller = Koha::Acquisition::Bookseller->new( + { + name => "my vendor", + address1 => "bookseller's address", + phone => "0123456", + active => 1, + deliverytime => 5, + } + )->store; + + my $basket = $builder->build_object( + { + class => "Koha::Acquisition::Baskets", + value => { + booksellerid => $bookseller->id, + create_items => 'ordering', + is_standing => 0, + closedate => undef + } + } + ); + + my $biblio = $builder->build_sample_biblio; + my $library = $builder->build( { source => 'Branch' } ); + my $branchcode = $library->{branchcode}; + + my $fields = { + loop_limit => 2, + budget_code => [ $budget_id_1, $budget_id_2 ], + budget_id => $budget_id_1, + basket_id => $basket->basketno, + biblionumber => $biblio->biblionumber, + homebranch => [ $branchcode, $branchcode ], + price => [ 10, 8 ], + }; + + t::lib::Mocks::mock_preference( + 'AcqCreateItem', 'ordering' + ); + + my $items_count = Koha::Items->count; + my $orders_count = Koha::Acquisition::Orders->count; + + my $order_line_details = Koha::Acquisition::MarcOrder::create_items_and_generate_order_hash( + { + agent => 'cron', + fields => $fields, + vendor => $bookseller, + active_currency => Koha::Acquisition::Currencies->get_active, + } + ); + my $new_items_count = Koha::Items->count; + my $new_orders_count = Koha::Acquisition::Orders->count; + + is( $new_items_count, $items_count + 2, 'Items were successfully created' ); + is( $new_orders_count, $orders_count + 2, 'Orders were successfully created' ); + + my @items = Koha::Items->search( { biblionumber => $biblio->biblionumber } )->as_list; + + foreach my $item (@items) { + is( $item->homebranch, $branchcode, 'Item was created in correct branch' ); + } + + my @orderlines = sort { $a->budget_id <=> $b->budget_id } Koha::Acquisition::Orders->search( { biblionumber => $biblio->biblionumber } )->as_list; + + for( my $i = 0; $i < (scalar(@orderlines)); $i++ ) { + is( $orderlines[$i]->budget_id, @{$fields->{budget_code}}[$i], 'Orderline was created with correct budget' ); + } + + $schema->storage->txn_rollback; +}; + +subtest '_create_item_fields_from_syspref' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( + 'MarcItemFieldsToOrder', 'homebranch: 975$a +holdingbranch: 975$b +itype: 975$y +nonpublic_note: 975$x +public_note: 975$z +loc: 975$c +ccode: 975$8 +notforloan: 975$7 +uri: 975$u +copyno: 975$n +quantity: 975$q +budget_code: 975$h +price: 975$p +replacementprice: 975$v' + ); + + t::lib::Mocks::mock_preference( + 'MarcFieldsToOrder', 'price: 949$p +quantity: 949$q +budget_code: 949$h' + ); + + my $record = MARC::Record->new(); + + $record->add_fields( + [ '001', '1234' ], + [ + '975', ' ', ' ', p => 10, q => 1, h => 1, a => 'CPL', b => 'CPL', y => 'Book', + x => 'Private note', z => 'Public note', c => 'Shelf', 8 => 'ccode', 7 => 0, + u => '12345abcde', n => '12345', v => 10 + ], + ); + $record->add_fields( + [ '001', '1234' ], + + [ '949', ' ', ' ', p => 10, q => 1, h => 1 ], + ); + + my $marc_item_fields_to_order = Koha::Acquisition::MarcOrder::_get_syspref_mappings( + $record, 'MarcItemFieldsToOrder', + ); + + my $marc_fields_to_order = Koha::Acquisition::MarcOrder::_get_syspref_mappings( + $record, 'MarcFieldsToOrder', + ); + + my $item_fields = Koha::Acquisition::MarcOrder::_create_item_fields_from_syspref( + { + marc_fields_to_order => $marc_fields_to_order, + marc_item_fields_to_order => $marc_item_fields_to_order + } + ); + + is( + @{$item_fields->{itemprices}}[0], 10, + "Price has been read correctly" + ); + is( + @{$item_fields->{itypes}}[0], "Book", + "Itypes has been read correctly" + ); + is($item_fields->{quantity}, 1, "Quantity has been read correctly"); + is($item_fields->{c_price}, 10, "MarcFieldsToOrder price has been read correctly"); + + $schema->storage->txn_rollback; + +} + -- 2.39.3 (Apple Git-146)