From f50009eb7fd6ff2ccdae2942e456e21df803296e Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 18 Feb 2014 10:06:33 +0100 Subject: [PATCH] [PASSED QA] Bug 10859: Alert if a borrower already has an issue for the same biblio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The librarian can force the checkout anyway. It doesn't alert the librarian if the biblio is a subscription Test plan: 1. Create a biblio with at least 2 items 2. Checkout the first item for a borrower 3. Set syspref AllowMultipleIssuesOnABiblio to OFF. 4. Try to checkout the second item with the same borrower. A message should appear telling you that this borrower already borrowed an item from this biblio. If you have the permission 'force_checkout' You should also see two buttons to confirm (or not) the checkout 5. Click on 'No'. The checkout is not done 6. Repeat step 4 and click 'Yes', the checkout is done. 7. Return the second item. 8. Set syspref AllowMultipleIssuesOnABiblio to ON 9. Try to checkout the second item with the same borrower. This time the checkout is done without warnings. Followed test plan. Works as expected. Signed-off-by: Marc Véron Signed-off-by: Katrin Fischer All tests and QA script pass, works well. Tested: * Permission to override * check out a second item from a record with subscriptions works * check out a second item from a 'normal' record is warned about, but can be done * No permission to override * subscription item: can be checked out * normal item: can't be checked out * Feature turned off * Check out never warns/blocks --- C4/Circulation.pm | 86 ++++++++++++++++++ installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 11 +++ .../en/modules/admin/preferences/circulation.pref | 6 ++ .../prog/en/modules/circ/circulation.tt | 9 ++ t/db_dependent/Circulation/GetIssues.t | 101 +++++++++++++++++++++ 6 files changed, 214 insertions(+) create mode 100644 t/db_dependent/Circulation/GetIssues.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f8b204a..3f8b129 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1043,6 +1043,25 @@ sub CanBookBeIssued { } } + if (not C4::Context->preference('AllowMultipleIssuesOnABiblio')) { + # Check if borrower has already issued an item from the same biblio + # Only if it's not a subscription + my $biblionumber = $item->{biblionumber}; + require C4::Serials; + my $is_a_subscription = C4::Serials::CountSubscriptionFromBiblionumber($biblionumber); + unless ($is_a_subscription) { + my $issues = GetIssues( { + borrowernumber => $borrower->{borrowernumber}, + biblionumber => $biblionumber, + } ); + my @issues = $issues ? @$issues : (); + # If there is at least one issue on another item than the item we want to checkout + if (scalar @issues > 0 and $issues[0]->{itemnumber} != $item->{itemnumber}) { + $needsconfirmation{BIBLIO_ALREADY_ISSUED} = 1; + } + } + } + return ( \%issuingimpossible, \%needsconfirmation, \%alerts ); } @@ -2324,6 +2343,73 @@ sub GetOpenIssue { } +=head2 GetIssues + + $issues = GetIssues({}); # return all issues! + $issues = GetIssues({ borrowernumber => $borrowernumber, biblionumber => $biblionumber }); + +Returns all pending issues that match given criteria. +Returns a arrayref or undef if an error occurs. + +Allowed criteria are: + +=over 2 + +=item * borrowernumber + +=item * biblionumber + +=item * itemnumber + +=back + +=cut + +sub GetIssues { + my ($criteria) = @_; + + # Build filters + my @filters; + my @allowed = qw(borrowernumber biblionumber itemnumber); + foreach (@allowed) { + if (defined $criteria->{$_}) { + push @filters, { + field => $_, + value => $criteria->{$_}, + }; + } + } + + # Do we need to join other tables ? + my %join; + if (defined $criteria->{biblionumber}) { + $join{items} = 1; + } + + # Build SQL query + my $where = ''; + if (@filters) { + $where = "WHERE " . join(' AND ', map { "$_->{field} = ?" } @filters); + } + my $query = q{ + SELECT issues.* + FROM issues + }; + if (defined $join{items}) { + $query .= q{ + LEFT JOIN items ON (issues.itemnumber = items.itemnumber) + }; + } + $query .= $where; + + # Execute SQL query + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare($query); + my $rv = $sth->execute(map { $_->{value} } @filters); + + return $rv ? $sth->fetchall_arrayref({}) : undef; +} + =head2 GetItemIssues $issues = &GetItemIssues($itemnumber, $history); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 661910b..847c5ed 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -17,6 +17,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AllowHoldsOnDamagedItems','1','','Allow hold requests to be placed on damaged items','YesNo'), ('AllowHoldsOnPatronsPossessions','1',NULL,'Allow holds on records that patron have items of it','YesNo'), ('AllowItemsOnHoldCheckout','0','','Do not generate RESERVE_WAITING and RESERVED warning when checking out items reserved to someone else. This allows self checkouts for those items.','YesNo'), +('AllowMultipleIssuesOnABiblio',1,'Allow/Don\'t allow patrons to check out multiple items from one biblio','','YesNo'), ('AllowMultipleCovers','0','1','Allow multiple cover images to be attached to each bibliographic record.','YesNo'), ('AllowNotForLoanOverride','0','','If ON, Koha will allow the librarian to loan a not for loan item.','YesNo'), ('AllowOfflineCirculation','0','','If on, enables HTML5 offline circulation functionality.','YesNo'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 78ba6ca..f9f8f1a 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8065,6 +8065,17 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.15.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q{ + INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) + VALUES('AllowMultipleIssuesOnABiblio',1,'Allow/Don\'t allow patrons to check out multiple items from one biblio','','YesNo') + }); + + print "Upgrade to $DBversion done (Bug 10859 - Add system preference AllowMultipleIssuesOnABiblio)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 70ac352..12154c1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -337,6 +337,12 @@ Circulation: alert: "display a message" nothing : "do nothing" - . + - + - pref: AllowMultipleIssuesOnABiblio + choices: + yes: Allow + no: "Don't allow" + - patrons to check out multiple items from the same biblio. Checkin Policy: - - pref: BlockReturnOfWithdrawnItems diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index e667bc3..cb37005 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -270,6 +270,15 @@ var MSG_EXPORT_SELECT_CHECKOUTS = _("You must select checkout(s) to export"); [% IF HIGHHOLDS %]
  • High demand item. Loan period shortened to [% HIGHHOLDS.duration %] days (due [% HIGHHOLDS.returndate %]). Check out anyway?
    • [% END %] + +[% IF BIBLIO_ALREADY_ISSUED %] +
  • + Borrower has already an issue on another item from this biblio. + [% IF CAN_user_circulate_force_checkout %] + Check out anyway? + [% END %] +
    • +[% END %] [% IF HIGHHOLDS %] diff --git a/t/db_dependent/Circulation/GetIssues.t b/t/db_dependent/Circulation/GetIssues.t new file mode 100644 index 0000000..36607d4 --- /dev/null +++ b/t/db_dependent/Circulation/GetIssues.t @@ -0,0 +1,101 @@ +#!/usr/bin/perl + +use Modern::Perl; + +use Test::More; +use Test::MockModule; +use C4::Biblio; +use C4::Items; +use C4::Members; +use C4::Branch; +use C4::Category; +use C4::Circulation; +use MARC::Record; + +my $branchcode; +my $branch_created; +my @branches = keys %{ GetBranches() }; +if (@branches) { + $branchcode = $branches[0]; +} else { + $branchcode = 'B'; + ModBranch({ add => 1, branchcode => $branchcode, branchname => 'Branch' }); + $branch_created = 1; +} + +my %item_branch_infos = ( + homebranch => $branchcode, + holdingbranch => $branchcode, +); + +my ($biblionumber1) = AddBiblio(MARC::Record->new, ''); +my $itemnumber1 = AddItem({ barcode => '0101', %item_branch_infos }, $biblionumber1); +my $itemnumber2 = AddItem({ barcode => '0102', %item_branch_infos }, $biblionumber1); + +my ($biblionumber2) = AddBiblio(MARC::Record->new, ''); +my $itemnumber3 = AddItem({ barcode => '0203', %item_branch_infos }, $biblionumber2); + +my $categorycode; +my $category_created; +my @categories = C4::Category->all; +if (@categories) { + $categorycode = $categories[0]->{categorycode} +} else { + $categorycode = 'C'; + C4::Context->dbh->do( + "INSERT INTO categories(categorycode) VALUES(?)", undef, $categorycode); + $category_created = 1; +} + +my $borrowernumber = AddMember(categorycode => $categorycode, branchcode => $branchcode); +my $borrower = GetMember(borrowernumber => $borrowernumber); + +# Need to mock userenv for AddIssue +my $module = new Test::MockModule('C4::Context'); +$module->mock('userenv', sub { { branch => $branchcode } }); +AddIssue($borrower, '0101'); +AddIssue($borrower, '0203'); + +# Begin tests... +my $issues; +$issues = GetIssues({biblionumber => $biblionumber1}); +is(scalar @$issues, 1, "Biblio $biblionumber1 has 1 item issued"); +is($issues->[0]->{itemnumber}, $itemnumber1, "First item of biblio $biblionumber1 is issued"); + +$issues = GetIssues({biblionumber => $biblionumber2}); +is(scalar @$issues, 1, "Biblio $biblionumber2 has 1 item issued"); +is($issues->[0]->{itemnumber}, $itemnumber3, "First item of biblio $biblionumber2 is issued"); + +$issues = GetIssues({borrowernumber => $borrowernumber}); +is(scalar @$issues, 2, "Borrower $borrowernumber checked out 2 items"); + +$issues = GetIssues({borrowernumber => $borrowernumber, biblionumber => $biblionumber1}); +is(scalar @$issues, 1, "One of those is an item from biblio $biblionumber1"); + +$issues = GetIssues({borrowernumber => $borrowernumber, biblionumber => $biblionumber2}); +is(scalar @$issues, 1, "The other is an item from biblio $biblionumber2"); + +$issues = GetIssues({itemnumber => $itemnumber2}); +is(scalar @$issues, 0, "No one has issued the second item of biblio $biblionumber2"); + +END { + AddReturn('0101', $branchcode); + AddReturn('0203', $branchcode); + DelMember($borrowernumber); + if ($category_created) { + C4::Context->dbh->do( + "DELETE FROM categories WHERE categorycode = ?", undef, $categorycode); + } + my $dbh = C4::Context->dbh; + C4::Items::DelItem($dbh, $biblionumber1, $itemnumber1); + C4::Items::DelItem($dbh, $biblionumber1, $itemnumber2); + C4::Items::DelItem($dbh, $biblionumber2, $itemnumber3); + C4::Biblio::DelBiblio($biblionumber1); + C4::Biblio::DelBiblio($biblionumber2); + + if ($branch_created) { + DelBranch($branchcode); + } +}; + +done_testing; -- 1.8.3.2