Bugzilla – Attachment 22330 Details for
Bug 8854
invoice filters don't all work
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
bug 8854: fix some invoice search filters
bug-8854-fix-some-invoice-search-filters.patch (text/plain), 9.73 KB, created by
Galen Charlton
on 2013-10-23 19:35:45 UTC
(
hide
)
Description:
bug 8854: fix some invoice search filters
Filename:
MIME Type:
Creator:
Galen Charlton
Created:
2013-10-23 19:35:45 UTC
Size:
9.73 KB
patch
obsolete
>From 48c34c8a16c07c33ebcbee47036516127967cbfe Mon Sep 17 00:00:00 2001 >From: Galen Charlton <gmc@esilibrary.com> >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 <gmc@esilibrary.com> >--- > C4/Acquisition.pm | 21 ++++++++++++--------- > t/Acquisition/Invoice.t | 8 ++++---- > t/db_dependent/Acquisition/Invoices.t | 32 +++++++++++++++++++++++++++++--- > 3 files changed, 45 insertions(+), 16 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..08a4439 100755 >--- a/t/Acquisition/Invoice.t >+++ b/t/Acquisition/Invoice.t >@@ -6,8 +6,6 @@ use C4::Context; > use Test::More tests => 49; > 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,8 +42,8 @@ my @invoices = C4::Acquisition::GetInvoices( > ); > my $history = $dbh->{mock_all_history}; > >-is(scalar(@$history), 1); >-my @bound_params = @{ $history->[0]->{bound_params} }; >+ok(scalar(@$history) > 0); >+my @bound_params = @{ $history->[-1]->{bound_params} }; > is(scalar(@bound_params), 15); > is($bound_params[0], 'supplierid'); > is($bound_params[1], '%invoicenumber%'); >diff --git a/t/db_dependent/Acquisition/Invoices.t b/t/db_dependent/Acquisition/Invoices.t >index 6b3e080..caf75c2 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8854
:
14923
|
15055
|
15056
|
22330
|
22442
|
22504
|
22976