Attachment 43077 Details for Bug 14945 – [SIGNED OFF] Bug 14945 - Add the ability to store the last patron to return an item

[patch] [SIGNED OFF] Bug 14945 - Add the ability to store the last patron to return an item

SIGNED-OFF-Bug-14945---Add-the-ability-to-store-th.patch (text/plain), 18.75 KB, created by Nick Clemens (kidclamp) on 2015-10-02 18:46:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2015-10-02 18:46:44 UTC

Size: 18.75 KB

patch

obsolete

>From 251a85f8c908f43acaca7073cc10fffd18a4b469 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <nick@quecheelibrary.org>
>---
> 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<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;
>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' %]
> <title>Koha &rsaquo; Catalog &rsaquo; Item details for [% title %] [% FOREACH subtitl IN subtitle %] [% subtitl.subfield %][% END %]</title>
> [% INCLUDE 'doc-head-close.inc' %]
>@@ -216,6 +217,9 @@
> 
>                 <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 %]
>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 @@
>                             <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> 
>             </div> 
>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

Actions: View | Diff | Splinter Review

Attachments on bug 14945: 43073 | 43074 | 43075 | 43077 | 44075 | 45147 | 45466 | 45510 | 45511 | 45522 | 45523