From ecd89d20302118202ebaa9fbe0a46aea624fe8a8 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 5 Nov 2012 14:35:23 -0500 Subject: [PATCH] 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 Signed-off-by: Jen DeMuth Signed-off-by: Tom Misilo Signed-off-by: Jonathan Druart Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com --- C4/Circulation.pm | 8 ++ Koha/Item.pm | 33 +++++ Koha/Schema/Result/ItemsLastBorrower.pm | 133 ++++++++++++++++++ catalogue/moredetail.pl | 2 + .../data/mysql/atomicupdate/bug_14945.sql | 13 ++ installer/data/mysql/kohastructure.sql | 16 +++ installer/data/mysql/sysprefs.sql | 1 + .../en/modules/admin/preferences/opac.pref | 8 +- .../prog/en/modules/catalogue/moredetail.tt | 4 + .../bootstrap/en/modules/opac-privacy.tt | 1 + .../Circulation/AnonymiseIssueHistory.t | 84 ++++++++++- 11 files changed, 301 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 35651f2293..ae53366c7e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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 diff --git a/Koha/Item.pm b/Koha/Item.pm index 2dcccfb192..e8543e1df1 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 0000000000..1a97cd7673 --- /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 70378d0268..1b33f75e86 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 0000000000..497e98f84f --- /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 95a8241cda..02725fd1df 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -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` -- diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 999f5c5b8b..10e45c2ac5 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -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'), 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 b3c0a2b9e8..09b850c52a 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 @@ -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 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 ac6ab111b3..bde296d451 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 db378ad46a..38ca4fa15f 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt @@ -96,6 +96,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 f4611ebfb5..87049ad7f5 100644 --- a/t/db_dependent/Circulation/AnonymiseIssueHistory.t +++ b/t/db_dependent/Circulation/AnonymiseIssueHistory.t @@ -18,11 +18,13 @@ 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; -- 2.39.5