From 251a85f8c908f43acaca7073cc10fffd18a4b469 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 5 Nov 2012 14:35:23 -0500 Subject: [PATCH] [SIGNED OFF] Bug 14945 - Add the ability to store the last patron to return an item 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 --- C4/Circulation.pm | 8 ++ Koha/Item.pm | 33 +++++ Koha/Schema/Result/ItemsLastBorrower.pm | 133 +++++++++++++++++++++ catalogue/moredetail.pl | 2 + installer/data/mysql/atomicupdate/bug_14945.sql | 13 ++ installer/data/mysql/kohastructure.sql | 16 +++ installer/data/mysql/sysprefs.sql | 1 + .../prog/en/modules/admin/preferences/opac.pref | 8 +- .../prog/en/modules/catalogue/moredetail.tt | 4 + .../opac-tmpl/bootstrap/en/modules/opac-privacy.tt | 1 + t/db_dependent/Circulation/AnonymiseIssueHistory.t | 83 ++++++++++++- 11 files changed, 300 insertions(+), 2 deletions(-) create mode 100644 Koha/Schema/Result/ItemsLastBorrower.pm create mode 100644 installer/data/mysql/atomicupdate/bug_14945.sql diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e5f95f3..6033763 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -48,6 +48,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; @@ -2127,6 +2129,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 diff --git a/Koha/Item.pm b/Koha/Item.pm index 2dcccfb..e8543e1 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -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 index 0000000..1a97cd7 --- /dev/null +++ b/Koha/Schema/Result/ItemsLastBorrower.pm @@ -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 + +=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 + +=back + +=cut + +__PACKAGE__->set_primary_key("id"); + +=head1 UNIQUE CONSTRAINTS + +=head2 C + +=over 4 + +=item * L + +=back + +=cut + +__PACKAGE__->add_unique_constraint("itemnumber", ["itemnumber"]); + +=head1 RELATIONS + +=head2 borrowernumber + +Type: belongs_to + +Related object: L + +=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 + +=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; diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl index 3736c13..2d64385 100755 --- a/catalogue/moredetail.pl +++ b/catalogue/moredetail.pl @@ -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 index 0000000..497e98f --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_14945.sql @@ -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; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index dab7538..28afe93 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1270,6 +1270,22 @@ CREATE TABLE `items` ( -- holdings/item information ) 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` -- diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 88a1f5f..836280a 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -412,6 +412,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'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref index 17d0b1a..e874d09 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ -563,6 +563,13 @@ OPAC: 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: yes: Allow @@ -616,7 +623,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 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt index 1ddc952..d76b70f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -1,3 +1,4 @@ +[% USE Koha %] [% INCLUDE 'doc-head-open.inc' %] Koha › Catalog › Item details for [% title %] [% FOREACH subtitl IN subtitle %] [% subtitl.subfield %][% END %] [% INCLUDE 'doc-head-close.inc' %] @@ -216,6 +217,9 @@
  • Last seen:[% IF ( ITEM_DAT.datelastseen ) %][% ITEM_DAT.datelastseen | $KohaDates %] [%END %] 
  • Last borrowed:[% IF (ITEM_DAT.datelastborrowed ) %][% ITEM_DAT.datelastborrowed | $KohaDates %][% END %] 
    • + [% IF Koha.Preference('StoreLastBorrower') && ITEM_DAT.object.last_returned_by %] +
  • Last returned by: [% ITEM_DAT.object.last_returned_by.cardnumber %] 
    • + [% END %] [% IF ( ITEM_DAT.card0 ) %]
  • Last borrower: [% ITEM_DAT.card0 %] 
    • [% END %] [% IF ( ITEM_DAT.card1 ) %]
  • Previous borrower: [% ITEM_DAT.card1 %] 
    • [% END %] [% IF ( ITEM_DAT.card2 ) %]
  • Previous borrower: [% ITEM_DAT.card2 %] 
    • [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt index 3b265ee..76ffe2f 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt @@ -77,6 +77,7 @@

    Whatever your privacy rule you choose, you can delete all your reading history immediately by clicking here. BE CAREFUL. Once you've confirmed the deletion, no one can retrieve the list!

    + [% IF Koha.Preference('StoreLastBorrower') %]

    Please note, the last person to return an item is tracked for the management of items returned damaged.

    [% END %] [% END # / IF Ask_data %] diff --git a/t/db_dependent/Circulation/AnonymiseIssueHistory.t b/t/db_dependent/Circulation/AnonymiseIssueHistory.t index 213be0b..4013d39 100644 --- a/t/db_dependent/Circulation/AnonymiseIssueHistory.t +++ b/t/db_dependent/Circulation/AnonymiseIssueHistory.t @@ -1,8 +1,9 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 4; use C4::Context; use C4::Circulation; +use Koha::Items; use t::lib::Mocks; use t::lib::TestBuilder; @@ -136,3 +137,83 @@ subtest 'AnonymousPatron is not defined' => sub { |, undef, $item->{itemnumber}); 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' ); +}; -- 1.9.1