Attachment 25366 Details for Bug 10859 – [Signed-off] Bug 10859: Alert if a borrower already has an issue for the same biblio

[patch] [Signed-off] Bug 10859: Alert if a borrower already has an issue for the same biblio

Signed-off-Bug-10859-Alert-if-a-borrower-already-h.patch (text/plain), 11.54 KB, created by Marc Véron on 2014-02-18 10:29:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marc Véron

Created: 2014-02-18 10:29:08 UTC

Size: 11.54 KB

patch

obsolete

>From ae9dd344247fbd93751d92119c1d7f381e383b23 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Tue, 18 Feb 2014 10:06:33 +0100
>Subject: [PATCH] [Signed-off] 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 <veron@veron.ch>
>---
> 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 1668d37..cc9bb56 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 f043d0b..eb49846 100755
>--- a/installer/data/mysql/updatedatabase.pl
>+++ b/installer/data/mysql/updatedatabase.pl
>@@ -7953,6 +7953,17 @@ if ( C4::Context->preference("Version") < TransformToNum($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 33ae4c9..36c9820 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 5a6a82d..d877b47 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>@@ -269,6 +269,15 @@ var MSG_EXPORT_SELECT_CHECKOUTS = _("You must select checkout(s) to export");
> [% IF  HIGHHOLDS %]
>     <li>High demand item. Loan period shortened to [% HIGHHOLDS.duration %] days (due [% HIGHHOLDS.returndate %]). Check out anyway?</li>
> [% END %]
>+
>+[% IF BIBLIO_ALREADY_ISSUED %]
>+  <li>
>+    Borrower has already an issue on another item from this biblio.
>+    [% IF CAN_user_circulate_force_checkout %]
>+      Check out anyway?
>+    [% END %]
>+  </li>
>+[% END %]
> </ul>
> 
> [% 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.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 10859: 20954 | 25362 | 25366 | 26263 | 26264 | 26267 | 26268 | 26269 | 26270