Bug 14945 - Add the ability to store the last patron to return an item
authorKyle M Hall <kyle@bywatersolutions.com>
Mon, 5 Nov 2012 19:35:23 +0000 (14:35 -0500)
committerBrendan Gallagher <brendan@bywatersolutions.com>
Thu, 31 Dec 2015 19:32:20 +0000 (19:32 +0000)
Currently if the AnonymousPatron system preference is in use, all patron
data is anonymized. Some libraries would like to be able to see the last
patron who returned out an item ( in case of damage ) but still keep all
other patrons anonymized.

* Add the table items_last_borrower ( id, itemnumber, borrowernumber )
* Add new system preference StoreLastBorrower
* If StoreLastBorrower is enabled, upon checkin have Koha insert into
  this table the patron who last returned this item. Replace existing
  row based on itemnumber if exists.
* If table has a row for a given item, link to the patron from the item
  details page.

Test plan:
1) Apply patch
2) Run updatedatabase.pl
3) Enable StoreLastBorrower
4) Issue an item to a patron and return said item
5) Issue the same item to a second patron, do not return it.
6) View moredetail.pl for the given bib, find the given item. There
   should be a new field in the history list 'Last returned by' with a link
   to the last patron to return the item.

Optionally, you can also verify this works even if patron issuing
history has been set to anonymize issues upon return.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Jen DeMuth <JDeMuth@roseville.ca.us>
Signed-off-by: Tom Misilo <misilot@fit.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
C4/Circulation.pm
Koha/Item.pm
Koha/Schema/Result/ItemsLastBorrower.pm [new file with mode: 0644]
catalogue/moredetail.pl
installer/data/mysql/atomicupdate/bug_14945.sql [new file with mode: 0644]
installer/data/mysql/kohastructure.sql
installer/data/mysql/sysprefs.sql
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt
t/db_dependent/Circulation/AnonymiseIssueHistory.t

index 35651f2..ae53366 100644 (file)
@@ -47,6 +47,8 @@ use Algorithm::CheckDigits;
 use Data::Dumper;
 use Koha::DateUtils;
 use Koha::Calendar;
+use Koha::Items;
+use Koha::Borrowers;
 use Koha::Borrower::Debarments;
 use Koha::Database;
 use Carp;
@@ -2178,6 +2180,12 @@ sub MarkIssueReturned {
     $sth_del->execute($borrowernumber, $itemnumber);
 
     ModItem( { 'onloan' => undef }, undef, $itemnumber );
+
+    if ( C4::Context->preference('StoreLastBorrower') ) {
+        my $item = Koha::Items->find( $itemnumber );
+        my $patron = Koha::Borrowers->find( $borrowernumber );
+        $item->last_returned_by( $patron );
+    }
 }
 
 =head2 _debar_user_on_return
index 2dcccfb..e8543e1 100644 (file)
@@ -24,6 +24,7 @@ use Carp;
 use Koha::Database;
 
 use Koha::Branches;
+use Koha::Borrowers;
 
 use base qw(Koha::Object);
 
@@ -73,6 +74,38 @@ sub holding_branch {
     return $self->{_holding_branch};
 }
 
+=head3 last_returned_by
+
+Gets and sets the last borrower to return an item.
+
+Accepts and returns Koha::Borrower objects
+
+$item->last_returned_by( $borrowernumber );
+
+$last_returned_by = $item->last_returned_by();
+
+=cut
+
+sub last_returned_by {
+    my ( $self, $borrower ) = @_;
+
+    my $items_last_returned_by_rs = Koha::Database->new()->schema()->resultset('ItemsLastBorrower');
+
+    if ($borrower) {
+        return $items_last_returned_by_rs->update_or_create(
+            { borrowernumber => $borrower->borrowernumber, itemnumber => $self->id } );
+    }
+    else {
+        unless ( $self->{_last_returned_by} ) {
+            my $result = $items_last_returned_by_rs->single( { itemnumber => $self->id } );
+            if ($result) {
+                $self->{_last_returned_by} = Koha::Borrowers->find( $result->get_column('borrowernumber') );
+            }
+        }
+
+        return $self->{_last_returned_by};
+    }
+}
 
 =head3 type
 
diff --git a/Koha/Schema/Result/ItemsLastBorrower.pm b/Koha/Schema/Result/ItemsLastBorrower.pm
new file mode 100644 (file)
index 0000000..1a97cd7
--- /dev/null
@@ -0,0 +1,133 @@
+use utf8;
+package Koha::Schema::Result::ItemsLastBorrower;
+
+# Created by DBIx::Class::Schema::Loader
+# DO NOT MODIFY THE FIRST PART OF THIS FILE
+
+=head1 NAME
+
+Koha::Schema::Result::ItemsLastBorrower
+
+=cut
+
+use strict;
+use warnings;
+
+use base 'DBIx::Class::Core';
+
+=head1 TABLE: C<items_last_borrower>
+
+=cut
+
+__PACKAGE__->table("items_last_borrower");
+
+=head1 ACCESSORS
+
+=head2 id
+
+  data_type: 'integer'
+  is_auto_increment: 1
+  is_nullable: 0
+
+=head2 itemnumber
+
+  data_type: 'integer'
+  is_foreign_key: 1
+  is_nullable: 0
+
+=head2 borrowernumber
+
+  data_type: 'integer'
+  is_foreign_key: 1
+  is_nullable: 0
+
+=head2 created_on
+
+  data_type: 'timestamp'
+  datetime_undef_if_invalid: 1
+  default_value: current_timestamp
+  is_nullable: 0
+
+=cut
+
+__PACKAGE__->add_columns(
+  "id",
+  { data_type => "integer", is_auto_increment => 1, is_nullable => 0 },
+  "itemnumber",
+  { data_type => "integer", is_foreign_key => 1, is_nullable => 0 },
+  "borrowernumber",
+  { data_type => "integer", is_foreign_key => 1, is_nullable => 0 },
+  "created_on",
+  {
+    data_type => "timestamp",
+    datetime_undef_if_invalid => 1,
+    default_value => \"current_timestamp",
+    is_nullable => 0,
+  },
+);
+
+=head1 PRIMARY KEY
+
+=over 4
+
+=item * L</id>
+
+=back
+
+=cut
+
+__PACKAGE__->set_primary_key("id");
+
+=head1 UNIQUE CONSTRAINTS
+
+=head2 C<itemnumber>
+
+=over 4
+
+=item * L</itemnumber>
+
+=back
+
+=cut
+
+__PACKAGE__->add_unique_constraint("itemnumber", ["itemnumber"]);
+
+=head1 RELATIONS
+
+=head2 borrowernumber
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::Borrower>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "borrowernumber",
+  "Koha::Schema::Result::Borrower",
+  { borrowernumber => "borrowernumber" },
+  { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" },
+);
+
+=head2 itemnumber
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::Item>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "itemnumber",
+  "Koha::Schema::Result::Item",
+  { itemnumber => "itemnumber" },
+  { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" },
+);
+
+
+# Created by DBIx::Class::Schema::Loader v0.07040 @ 2015-10-02 12:33:33
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ByHKNZCEz4a1AqTnOJgUWA
+
+
+# You can replace this text with custom code or comments, and it will be preserved on regeneration
+1;
index 70378d0..1b33f75 100755 (executable)
@@ -38,6 +38,7 @@ use C4::Reserves qw(GetReservesFromBiblionumber);
 
 use Koha::Acquisition::Bookseller;
 use Koha::DateUtils;
+use Koha::Items;
 
 my $query=new CGI;
 
@@ -135,6 +136,7 @@ foreach ( keys %{$data} ) {
 
 ($itemnumber) and @items = (grep {$_->{'itemnumber'} == $itemnumber} @items);
 foreach my $item (@items){
+    $item->{object} = Koha::Items->find( $item->{itemnumber} );
     $item->{itemlostloop}= GetAuthorisedValues(GetAuthValCode('items.itemlost',$fw),$item->{itemlost}) if GetAuthValCode('items.itemlost',$fw);
     $item->{itemdamagedloop}= GetAuthorisedValues(GetAuthValCode('items.damaged',$fw),$item->{damaged}) if GetAuthValCode('items.damaged',$fw);
     $item->{'collection'}              = $ccodes->{ $item->{ccode} } if ($ccodes);
diff --git a/installer/data/mysql/atomicupdate/bug_14945.sql b/installer/data/mysql/atomicupdate/bug_14945.sql
new file mode 100644 (file)
index 0000000..497e98f
--- /dev/null
@@ -0,0 +1,13 @@
+INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES ('StoreLastBorrower','0','','If ON, the last borrower to return an item will be stored in items.last_returned_by','YesNo');
+
+CREATE TABLE IF NOT EXISTS `items_last_borrower` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `itemnumber` int(11) NOT NULL,
+  `borrowernumber` int(11) NOT NULL,
+  `created_on` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+  PRIMARY KEY (`id`),
+  UNIQUE KEY `itemnumber` (`itemnumber`),
+  KEY `borrowernumber` (`borrowernumber`),
+  CONSTRAINT `items_last_borrower_ibfk_2` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
+  CONSTRAINT `items_last_borrower_ibfk_1` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
index 95a8241..02725fd 100644 (file)
@@ -1264,6 +1264,22 @@ CREATE TABLE `items` ( -- holdings/item information
   CONSTRAINT `items_ibfk_3` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
+--
+-- Table structure for table `items_last_borrower`
+--
+
+CREATE TABLE IF NOT EXISTS `items_last_borrower` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `itemnumber` int(11) NOT NULL,
+  `borrowernumber` int(11) NOT NULL,
+  `created_on` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+  PRIMARY KEY (`id`),
+  UNIQUE KEY `itemnumber` (`itemnumber`),
+  KEY `borrowernumber` (`borrowernumber`),
+  CONSTRAINT `items_last_borrower_ibfk_2` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
+  CONSTRAINT `items_last_borrower_ibfk_1` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+
 --
 -- Table structure for table `itemtypes`
 --
index 999f5c5..10e45c2 100644 (file)
@@ -437,6 +437,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('StaffSerialIssueDisplayCount','3','','Number of serial issues to display per subscription in the Staff client','Integer'),
 ('StaticHoldsQueueWeight','0',NULL,'Specify a list of library location codes separated by commas -- the list of codes will be traversed and weighted with first values given higher weight for holds fulfillment -- alternatively, if RandomizeHoldsQueueWeight is set, the list will be randomly selective','Integer'),
 ('StatisticsFields','location|itype|ccode', NULL, 'Define Fields (from the items table) used for statistics members','Free'),
+('StoreLastBorrower','0','','If ON, the last borrower to return an item will be stored in items.last_returned_by','YesNo'),
 ('SubfieldsToAllowForRestrictedBatchmod','','Define a list of subfields for which edition is authorized when items_batchmod_restricted permission is enabled, separated by spaces. Example: 995\$f 995\$h 995\$j',NULL,'Free'),
 ('SubfieldsToAllowForRestrictedEditing','','Define a list of subfields for which edition is authorized when edit_items_restricted permission is enabled, separated by spaces. Example: 995\$f 995\$h 995\$j',NULL,'Free'),
 ('SubfieldsToUseWhenPrefill','','','Define a list of subfields to use when prefilling items (separated by space)','Free'),
index b3c0a2b..09b850c 100644 (file)
@@ -562,6 +562,13 @@ OPAC:
             - expired patrons from OPAC actions such as placing a hold or renewing.  Note that the setting for a patron category takes priority over this system preference.
 
     Privacy:
+        -
+            - pref: StoreLastBorrower
+              default: 0
+              choices:
+                  yes: Store
+                  no: "Don't store"
+            - the last patron to return an item. This setting is independent of opacreadinghistory/AnonymousPatron.
         -
             - pref: AnonSuggestions
               choices:
@@ -623,7 +630,6 @@ OPAC:
             - pref: RestrictedPageTitle
               class: long
             - "as title of your restricted page (appears in the breadcrumb and on the top of the restricted page)"
-
     Shelf Browser:
         -
             - pref: OPACShelfBrowser
index ac6ab11..bde296d 100644 (file)
@@ -1,3 +1,4 @@
+[% USE Koha %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Catalog &rsaquo; Item details for [% title %] [% FOREACH subtitl IN subtitle %] [% subtitl.subfield %][% END %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
 
                 <li><span class="label">Last seen:</span>[% IF ( ITEM_DAT.datelastseen ) %][% ITEM_DAT.datelastseen | $KohaDates %] [%END %]&nbsp;</li>
                 <li><span class="label">Last borrowed:</span>[% IF (ITEM_DAT.datelastborrowed ) %][% ITEM_DAT.datelastborrowed | $KohaDates %][% END %]&nbsp;</li>
+                [% IF Koha.Preference('StoreLastBorrower') && ITEM_DAT.object.last_returned_by %]
+                    <li><span class="label">Last returned by:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.object.last_returned_by.borrowernumber %]">[% ITEM_DAT.object.last_returned_by.cardnumber %]</a>&nbsp;</li>
+                [% END %]
                 [% IF ( ITEM_DAT.card0 ) %]<li><span class="label">Last borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower0 %]">[% ITEM_DAT.card0 %]</a>&nbsp;</li>[% END %]
                 [% IF ( ITEM_DAT.card1 ) %]<li><span class="label">Previous borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower1 %]">[% ITEM_DAT.card1 %]</a>&nbsp;</li>[% END %]
                 [% IF ( ITEM_DAT.card2 ) %]<li><span class="label">Previous borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower2 %]">[% ITEM_DAT.card2 %]</a>&nbsp;</li>[% END %]
index db378ad..38ca4fa 100644 (file)
@@ -96,6 +96,7 @@
                             <p>Whatever your privacy rule you choose, you can delete all your reading history immediately by clicking here. <b>BE CAREFUL</b>. Once you've confirmed the deletion, no one can retrieve the list!</p>
                             <input type="submit" value="Immediate deletion" class="btn btn-danger" onclick="return confirmDelete(MSG_CONFIRM_AGAIN);" />
                         </form>
+                        [% IF Koha.Preference('StoreLastBorrower') %]<p id="store-last-borrower-msg">Please note, the last person to return an item is tracked for the management of items returned damaged.</p>[% END %]
                     [% END # / IF Ask_data %]
                 </div> <!-- / .userprivacy -->
             </div> <!-- / .span10 -->
index f4611eb..87049ad 100644 (file)
 
 use Modern::Perl;
 
-use Test::More tests => 3;
+use Test::More tests => 4;
 
 use C4::Context;
 use C4::Circulation;
+
 use Koha::Database;
+use Koha::Items;
 
 use t::lib::Mocks;
 use t::lib::TestBuilder;
@@ -161,6 +163,86 @@ subtest 'AnonymousPatron is not defined' => sub {
     is( $borrowernumber_used_to_anonymised, undef, 'With AnonymousPatron is not defined, the issue should have been anonymised anyway' );
 };
 
+subtest 'Test StoreLastBorrower' => sub {
+    plan tests => 4;
+
+    t::lib::Mocks::mock_preference( 'StoreLastBorrower', '1' );
+
+    my $patron = $builder->build(
+        {
+            source => 'Borrower',
+            value  => { privacy => 1, }
+        }
+    );
+
+    my $item = $builder->build(
+        {
+            source => 'Item',
+            value  => {
+                itemlost  => 0,
+                withdrawn => 0,
+            },
+        }
+    );
+
+    my $issue = $builder->build(
+        {
+            source => 'Issue',
+            value  => {
+                borrowernumber => $patron->{borrowernumber},
+                itemnumber     => $item->{itemnumber},
+            },
+        }
+    );
+
+    my $item_object   = Koha::Items->find( $item->{itemnumber} );
+    my $patron_object = $item_object->last_returned_by();
+    is( $patron_object, undef, 'Koha::Item::last_returned_by returned undef' );
+
+    my ( $returned, undef, undef ) = C4::Circulation::AddReturn( $item->{barcode}, undef, undef, undef, '2010-10-10' );
+
+    $item_object   = Koha::Items->find( $item->{itemnumber} );
+    $patron_object = $item_object->last_returned_by();
+    is( ref($patron_object), 'Koha::Borrower', 'Koha::Item::last_returned_by returned Koha::Borrower' );
+
+    $patron = $builder->build(
+        {
+            source => 'Borrower',
+            value  => { privacy => 1, }
+        }
+    );
+
+    $issue = $builder->build(
+        {
+            source => 'Issue',
+            value  => {
+                borrowernumber => $patron->{borrowernumber},
+                itemnumber     => $item->{itemnumber},
+            },
+        }
+    );
+
+    ( $returned, undef, undef ) = C4::Circulation::AddReturn( $item->{barcode}, undef, undef, undef, '2010-10-10' );
+
+    $item_object   = Koha::Items->find( $item->{itemnumber} );
+    $patron_object = $item_object->last_returned_by();
+    is( $patron_object->id, $patron->{borrowernumber}, 'Second patron to return item replaces the first' );
+
+    $patron = $builder->build(
+        {
+            source => 'Borrower',
+            value  => { privacy => 1, }
+        }
+    );
+    $patron_object = Koha::Borrowers->find( $patron->{borrowernumber} );
+
+    $item_object->last_returned_by($patron_object);
+    $item_object = Koha::Items->find( $item->{itemnumber} );
+    my $patron_object2 = $item_object->last_returned_by();
+    is( $patron_object->id, $patron_object2->id,
+        'Calling last_returned_by with Borrower object sets last_returned_by to that borrower' );
+};
+
 $schema->storage->txn_rollback;
 
 1;