From 319defb87abb608b2e826df370d9cc79391cb5e7 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 29 Jan 2013 09:42:12 -0800 Subject: [PATCH] bug 8854: fix some invoice search filters Fix the supplier, shipment date, and library filters on the invoice search. An invoice's library is (in parallel with order search) defined as the library of the staff member that approved the basket. Before this patch, the code was referring to an aqorders.branchcode column that doesn't exist. This patch also improves the author, title, ISBN/EAN/ISSN, publisher, and publication year filters to no longer require exact matches; substring matches now suffice. Finally, this patch considers biblio.copyrightdate in addition to biblioitems.publicationyear for publication date searches, as the MARC21 frameworks use the former column but not the latter. This patch also fixes the current test cases for invoices so that they pass and adds regression tests. Test plan: [1] Create two invoices for different vendors. [2] Do an invoice search and filter on shipment date. Verify that the expected invoice(s) are returned. [3] Do an invoice search and filter on branch (of the staff member that approved the basket). Verify that the expected invoice(s) are returned. [4] Do an invoice search and filter on supplier. Verify that the expected invoice(s) are returned. [5] Do invoice searches on author, title, ISBN/EAN/ISSN, publisher, and publication year and verify that the results are as expected. Signed-off-by: Galen Charlton --- C4/Acquisition.pm | 21 ++++++++++++--------- t/Acquisition/Invoice.t | 15 ++++++++------- t/db_dependent/Acquisition/Invoices.t | 32 +++++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 8c40dcd..c79d7b8 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -2278,6 +2278,7 @@ sub AddClaim { my @invoices = GetInvoices( invoicenumber => $invoicenumber, + supplierid => $supplierid, suppliername => $suppliername, shipmentdatefrom => $shipmentdatefrom, # ISO format shipmentdateto => $shipmentdateto, # ISO format @@ -2322,6 +2323,8 @@ sub GetInvoices { FROM aqinvoices LEFT JOIN aqbooksellers ON aqbooksellers.id = aqinvoices.booksellerid LEFT JOIN aqorders ON aqorders.invoiceid = aqinvoices.invoiceid + LEFT JOIN aqbasket ON aqbasket.basketno=aqorders.basketno + LEFT JOIN borrowers ON aqbasket.authorisedby=borrowers.borrowernumber LEFT JOIN biblio ON aqorders.biblionumber = biblio.biblionumber LEFT JOIN biblioitems ON biblio.biblionumber = biblioitems.biblionumber LEFT JOIN subscription ON biblio.biblionumber = subscription.biblionumber @@ -2342,11 +2345,11 @@ sub GetInvoices { push @bind_args, "%$args{suppliername}%"; } if($args{shipmentdatefrom}) { - push @bind_strs, " aqinvoices.shipementdate >= ? "; + push @bind_strs, " aqinvoices.shipmentdate >= ? "; push @bind_args, $args{shipmentdatefrom}; } if($args{shipmentdateto}) { - push @bind_strs, " aqinvoices.shipementdate <= ? "; + push @bind_strs, " aqinvoices.shipmentdate <= ? "; push @bind_args, $args{shipmentdateto}; } if($args{billingdatefrom}) { @@ -2358,27 +2361,27 @@ sub GetInvoices { push @bind_args, $args{billingdateto}; } if($args{isbneanissn}) { - push @bind_strs, " (biblioitems.isbn LIKE ? OR biblioitems.ean LIKE ? OR biblioitems.issn LIKE ? ) "; + push @bind_strs, " (biblioitems.isbn LIKE CONCAT('%', ?, '%') OR biblioitems.ean LIKE CONCAT('%', ?, '%') OR biblioitems.issn LIKE CONCAT('%', ?, '%') ) "; push @bind_args, $args{isbneanissn}, $args{isbneanissn}, $args{isbneanissn}; } if($args{title}) { - push @bind_strs, " biblio.title LIKE ? "; + push @bind_strs, " biblio.title LIKE CONCAT('%', ?, '%') "; push @bind_args, $args{title}; } if($args{author}) { - push @bind_strs, " biblio.author LIKE ? "; + push @bind_strs, " biblio.author LIKE CONCAT('%', ?, '%') "; push @bind_args, $args{author}; } if($args{publisher}) { - push @bind_strs, " biblioitems.publishercode LIKE ? "; + push @bind_strs, " biblioitems.publishercode LIKE CONCAT('%', ?, '%') "; push @bind_args, $args{publisher}; } if($args{publicationyear}) { - push @bind_strs, " biblioitems.publicationyear = ? "; - push @bind_args, $args{publicationyear}; + push @bind_strs, " ((biblioitems.publicationyear LIKE CONCAT('%', ?, '%')) OR (biblio.copyrightdate LIKE CONCAT('%', ?, '%'))) "; + push @bind_args, $args{publicationyear}, $args{publicationyear}; } if($args{branchcode}) { - push @bind_strs, " aqorders.branchcode = ? "; + push @bind_strs, " borrowers.branchcode = ? "; push @bind_args, $args{branchcode}; } diff --git a/t/Acquisition/Invoice.t b/t/Acquisition/Invoice.t index 0fd5b91..aa1172d 100755 --- a/t/Acquisition/Invoice.t +++ b/t/Acquisition/Invoice.t @@ -3,11 +3,9 @@ use Modern::Perl; use C4::Context; -use Test::More tests => 49; +use Test::More tests => 50; use Test::MockModule; -use_ok('C4::Acquisition'); - my $module = new Test::MockModule('C4::Context'); $module->mock('_new_dbh', sub { my $dbh = DBI->connect( 'DBI:Mock:', '', '' ) @@ -17,6 +15,8 @@ $module->mock('_new_dbh', sub { my $dbh = C4::Context->dbh; +use_ok('C4::Acquisition'); + # We need to add a resultset to avoid DBI fail # ("DBI bind_columns: invalid number of arguments...") my $rs = [ @@ -42,9 +42,9 @@ my @invoices = C4::Acquisition::GetInvoices( ); my $history = $dbh->{mock_all_history}; -is(scalar(@$history), 1); -my @bound_params = @{ $history->[0]->{bound_params} }; -is(scalar(@bound_params), 15); +ok(scalar(@$history) > 0); +my @bound_params = @{ $history->[-1]->{bound_params} }; +is(scalar(@bound_params), 16); is($bound_params[0], 'supplierid'); is($bound_params[1], '%invoicenumber%'); is($bound_params[2], '%suppliername%'); @@ -59,7 +59,8 @@ is($bound_params[10], 'title'); is($bound_params[11], 'author'); is($bound_params[12], 'publisher'); is($bound_params[13], 'publicationyear'); -is($bound_params[14], 'branchcode'); +is($bound_params[14], 'publicationyear'); +is($bound_params[15], 'branchcode'); $dbh->{mock_clear_history} = 1; $dbh->{mock_add_resultset} = $rs; diff --git a/t/db_dependent/Acquisition/Invoices.t b/t/db_dependent/Acquisition/Invoices.t index 6b3e080..78b7cfb 100644 --- a/t/db_dependent/Acquisition/Invoices.t +++ b/t/db_dependent/Acquisition/Invoices.t @@ -9,7 +9,7 @@ use warnings; use C4::Bookseller qw( GetBookSellerFromId ); use C4::Biblio qw( AddBiblio ); -use Test::More tests => 14; +use Test::More tests => 21; BEGIN { use_ok('C4::Acquisition'); @@ -19,6 +19,8 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; +$dbh->do(q{DELETE FROM aqinvoices}); + my $booksellerid = C4::Bookseller::AddBookseller( { name => "my vendor", @@ -41,7 +43,14 @@ my $budgetid = C4::Budgets::AddBudget( my $budget = C4::Budgets::GetBudget( $budgetid ); my ($ordernumber1, $ordernumber2, $ordernumber3); -my ($biblionumber1, $biblioitemnumber1) = AddBiblio(MARC::Record->new, ''); +my $bibrec1 = MARC::Record->new(); +$bibrec1->append_fields( + MARC::Field->new('020', '', '', 'a' => '1234567890'), + MARC::Field->new('100', '', '', 'a' => 'Shakespeare, Billy'), + MARC::Field->new('245', '', '', 'a' => 'Bug 8854'), + MARC::Field->new('260', '', '', 'b' => 'Scholastic Publishing', c => 'c2012'), +); +my ($biblionumber1, $biblioitemnumber1) = AddBiblio($bibrec1, ''); my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); ( undef, $ordernumber1 ) = C4::Acquisition::NewOrder( @@ -74,7 +83,9 @@ my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); ); my $invoiceid1 = AddInvoice(invoicenumber => 'invoice1', booksellerid => $booksellerid, unknown => "unknown"); -my $invoiceid2 = AddInvoice(invoicenumber => 'invoice2', booksellerid => $booksellerid, unknown => "unknown"); +my $invoiceid2 = AddInvoice(invoicenumber => 'invoice2', booksellerid => $booksellerid, unknown => "unknown", + shipmentdate => '2012-12-24', + ); my ($datereceived, $new_ordernumber) = ModReceiveOrder( $biblionumber1, @@ -122,6 +133,21 @@ cmp_ok(scalar @invoices, '>=', 2, 'GetInvoices returns at least two invoices'); @invoices = GetInvoices(invoicenumber => 'invoice2'); cmp_ok(scalar @invoices, '>=', 1, 'GetInvoices returns at least one invoice when a specific invoice is requested'); +@invoices = GetInvoices(shipmentdateto => '2012-12-24', shipmentdatefrom => '2012-12-24'); +is($invoices[0]->{invoicenumber}, 'invoice2', 'GetInvoices() to search by shipmentdate works (bug 8854)'); +@invoices = GetInvoices(title => 'Bug'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by title works (bug 8854)'); +@invoices = GetInvoices(author => 'Billy'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by author works (bug 8854)'); +@invoices = GetInvoices(publisher => 'Scholastic'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by publisher works (bug 8854)'); +@invoices = GetInvoices(publicationyear => '2012'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by publication/copyright year works (bug 8854)'); +@invoices = GetInvoices(isbneanissn => '1234567890'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by ISBN works (bug 8854)'); +@invoices = GetInvoices(isbneanissn => '123456789'); +is($invoices[0]->{invoicenumber}, 'invoice1', 'GetInvoices() to search by partial ISBN works (bug 8854)'); + my $invoicesummary1 = GetInvoice($invoiceid1); is($invoicesummary1->{'invoicenumber'}, 'invoice1', 'GetInvoice retrieves correct invoice'); is($invoicesummary1->{'invoicenumber'}, $invoice1->{'invoicenumber'}, 'GetInvoice and GetInvoiceDetails retrieve same information'); -- 1.7.10.4