From 2f1bd4afcee6c3d30af8969f272d202c59f5598e Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 19 Feb 2018 02:04:24 +0000 Subject: [PATCH] Bug 19532: Extra fixes from Comment 68 This patch fixes the following: The db uppdate tries to insert after 'notes' in issuing rules, but this doesn't exist When placing recall I always get a warning:

Warning: Your library does not allow recalls for x item types.

Cancelling recall from 'Recall history tab' doesn't work If patron has no recalls there is 'Patron has no current recalls' message at bottom of tabs on checkout and details tabs for patron When confirming a recall if the item is from another branch I get a transfer generated as well as confirming hold for recall Feature now checks if the recall branch is the same as the item holding branch when checking in item to determine if transfer is required On recalls awaiting pickup: need to update recall.borrower to recall.patron - causing internal server error When placing a recall - if I hit 'back' after confirming and confirm again I get a second recall on the same item Recalls queue should have waiting date information Recalls queue should have a way to hide cancelled, I would expect closed recalls to be hidden by default Recalls queue now hides old (cancelled/expired/finished) recalls by default and has a checkbox to toggle this When viewing a patrons account in the staff side we should see that a checkout has an active recall Checking out the recall doesn't seem to close the recall Added an 'F' status for finished/closed recalls Signed-off-by: Nick Clemens Conflicts: Koha/Schema/Result/Borrower.pm --- C4/Circulation.pm | 19 +++++++++++ Koha/Recall.pm | 12 +++++++ Koha/Schema/Result/Biblio.pm | 19 +++++++++-- Koha/Schema/Result/Borrower.pm | 16 ++++++++++ Koha/Schema/Result/Branch.pm | 15 +++++++++ .../atomicupdate/bug_19532_-_add_recalls_table.sql | 4 +-- .../bug_19532_-_adding_recalls_circ_rules.perl | 2 +- installer/data/mysql/kohastructure.sql | 37 ++-------------------- .../prog/en/includes/recalls-reports.inc | 13 +++++++- .../intranet-tmpl/prog/en/includes/recalls.inc | 2 ++ .../prog/en/modules/circ/recalls_overdue.tt | 2 +- .../prog/en/modules/circ/recalls_queue.tt | 19 ++++++++--- .../prog/en/modules/circ/recalls_waiting.tt | 6 ++-- .../prog/en/modules/members/moremember.tt | 2 +- .../prog/en/modules/members/recallshistory.tt | 2 +- koha-tmpl/intranet-tmpl/prog/js/checkouts.js | 4 +++ koha-tmpl/intranet-tmpl/prog/js/recalls.js | 11 +++++-- .../opac-tmpl/bootstrap/en/modules/opac-recall.tt | 1 - opac/opac-recall.pl | 17 ++++++---- svc/checkouts | 7 ++++ t/db_dependent/Koha/Recalls.t | 7 ++-- 21 files changed, 154 insertions(+), 63 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index c42bcd3..6ffd714 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -357,6 +357,17 @@ sub transferbook { $dotransfer = 1; } + # check if item has been recalled. do a transfer if the recall branch is different to the item holding branch, otherwise don't do a transfer + my $recall = Koha::Recalls->find({ itemnumber => $item->itemnumber, status => 'R' }); + if (defined $recall){ + if ($recall->branchcode eq $fbr){ + $dotransfer = 0; + $messages->{'RecallPlacedAtHoldingBranch'} = 1; + } else { + $dotransfer = 1; + } + } + #actually do the transfer.... if ($dotransfer) { ModItemTransfer( $itemnumber, $fbr, $tbr ); @@ -1367,6 +1378,14 @@ sub AddIssue { } ); + # Check if a recall + if ( C4::Context->preference('UseRecalls') ) { + my $recall = Koha::Recalls->find({ itemnumber => $item->{'itemnumber'}, borrowernumber => $borrower->{'borrowernumber'} }); + if (defined $recall) { + $recall->update({ status => "F", old => 1 }); + } + } + if ( C4::Context->preference('ReturnToShelvingCart') ) { # ReturnToShelvingCart is on, anything issued should be taken off the cart. CartToShelf( $item->{'itemnumber'} ); diff --git a/Koha/Recall.pm b/Koha/Recall.pm index ad51e36..89cae47 100644 --- a/Koha/Recall.pm +++ b/Koha/Recall.pm @@ -178,6 +178,18 @@ sub is_cancelled { return $status && $status eq 'C'; } +=head3 is_finished + +Returns true if recall is closed + +=cut + +sub is_finished { + my ($self) = @_; + my $status = $self->status; + return $status && $status eq 'F'; +} + =head1 AUTHOR Aleisha Amohia diff --git a/Koha/Schema/Result/Biblio.pm b/Koha/Schema/Result/Biblio.pm index b118011..827d825 100644 --- a/Koha/Schema/Result/Biblio.pm +++ b/Koha/Schema/Result/Biblio.pm @@ -272,6 +272,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 recalls + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "recalls", +"Koha::Schema::Result::Recall", +{ "foreign.biblionumber" => "self.biblionumber" }, +{ cascade_copy => 0, cascade_delete => 0 }, +); + =head2 reserves Type: has_many @@ -348,7 +363,7 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:53 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:bUv00JjY09Hj2Zj4klqyxA +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-25 21:48:11 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:IReqh9sf7I012bhIlwXHgw 1; diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index 8e4580f..5ce5b43 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -1235,6 +1235,22 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 recalls + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "recalls", + +"Koha::Schema::Result::Recall", +{ "foreign.borrowernumber" => "self.borrowernumber" }, +{ cascade_copy => 0, cascade_delete => 0 }, +); + =head2 reserves Type: has_many diff --git a/Koha/Schema/Result/Branch.pm b/Koha/Schema/Result/Branch.pm index 13fe183..c2bd31d 100644 --- a/Koha/Schema/Result/Branch.pm +++ b/Koha/Schema/Result/Branch.pm @@ -601,6 +601,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 recalls + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "recalls", +"Koha::Schema::Result::Recall", +{ "foreign.branchcode" => "self.branchcode" }, +{ cascade_copy => 0, cascade_delete => 0 }, +); + =head2 reserves Type: has_many diff --git a/installer/data/mysql/atomicupdate/bug_19532_-_add_recalls_table.sql b/installer/data/mysql/atomicupdate/bug_19532_-_add_recalls_table.sql index 56ad5cf..7f79a93 100644 --- a/installer/data/mysql/atomicupdate/bug_19532_-_add_recalls_table.sql +++ b/installer/data/mysql/atomicupdate/bug_19532_-_add_recalls_table.sql @@ -9,13 +9,13 @@ CREATE TABLE `recalls` ( -- information related to recalls in Koha `cancellationdate` date default NULL, -- the date this recall was cancelled `recallnotes` mediumtext, -- notes related to this recall `priority` smallint(6) default NULL, -- where in the queue the patron sits - `status` varchar(1) default NULL, -- a one letter code defining what the status is of the recall is after it has been requested. R=requested, O=overdue, W=awaiting pickup, E=expired, C=cancelled + `status` varchar(1) default NULL, -- a one letter code defining what the status is of the recall is after it has been requested. R=requested, O=overdue, W=awaiting pickup, E=expired, C=cancelled, F=finished/completed `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP, -- the date and time this recall was last updated `itemnumber` int(11) default NULL, -- foreign key from the items table defining the specific item the recall request was placed on `waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library `expirationdate` DATE DEFAULT NULL, -- the date the recall expires `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- the optional itemtype of the item the patron is requesting - `old` int(1) default null, -- flag if the recall is old and no longer active, i.e. expired or cancelled + `old` int(1) default null, -- flag if the recall is old and no longer active, i.e. expired, cancelled or completed PRIMARY KEY (`recall_id`), KEY `prioritystatusidx` (priority,status), KEY `borrowernumber` (`borrowernumber`), diff --git a/installer/data/mysql/atomicupdate/bug_19532_-_adding_recalls_circ_rules.perl b/installer/data/mysql/atomicupdate/bug_19532_-_adding_recalls_circ_rules.perl index 3cf6a20..f0d6088 100644 --- a/installer/data/mysql/atomicupdate/bug_19532_-_adding_recalls_circ_rules.perl +++ b/installer/data/mysql/atomicupdate/bug_19532_-_adding_recalls_circ_rules.perl @@ -1,7 +1,7 @@ $DBversion = 'XXX'; if( CheckVersion( $DBversion ) ) { unless( column_exists( 'issuingrules', 'recall_due_date_interval' ) ) { - $dbh->do(q|ALTER TABLE issuingrules ADD recall_due_date_interval int(11) default NULL AFTER note|); + $dbh->do(q|ALTER TABLE issuingrules ADD recall_due_date_interval int(11) default NULL AFTER article_requests|); } unless( column_exists( 'issuingrules', 'recall_overdue_fine' ) ) { $dbh->do(q|ALTER TABLE issuingrules ADD recall_overdue_fine decimal(28,6) default NULL AFTER recall_due_date_interval|); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index f566cbc..6a2abb5 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1883,14 +1883,15 @@ CREATE TABLE `recalls` ( -- information related to recalls in Koha `cancellationdate` date default NULL, -- the date this recall was cancelled `recallnotes` mediumtext, -- notes related to this recall `priority` smallint(6) default NULL, -- where in the queue the patron sits - `found` varchar(1) default NULL, -- a one letter code defining what the status is of the recall is after it has been requested. R=requested, O=overdue, W=awaiting pickup, E=expired + `found` varchar(1) default NULL, -- a one letter code defining what the status is of the recall is after it has been requested. R=requested, O=overdue, W=awaiting pickup, E=expired, F=finished/completed `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP, -- the date and time this recall was last updated `itemnumber` int(11) default NULL, -- foreign key from the items table defining the specific item the recall request was placed on `waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library `expirationdate` DATE DEFAULT NULL, -- the date the recall expires `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- the optional itemtype of the item the patron is requesting + `old` int(1) default NULL, -- flag if the recall is old and no longer active, i.e. expired, cancelled or completed PRIMARY KEY (`recall_id`), - KEY `priorityfoundidx` (priority,found), + KEY `prioritystatusidx` (priority,status), KEY `borrowernumber` (`borrowernumber`), KEY `biblionumber` (`biblionumber`), KEY `itemnumber` (`itemnumber`), @@ -1904,38 +1905,6 @@ CREATE TABLE `recalls` ( -- information related to recalls in Koha ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; -- --- Table structure for table `old_recalls` --- - -DROP TABLE IF EXISTS `old_recalls`; -CREATE TABLE `old_recalls` ( -- information related to recalls in Koha - `recall_id` int(11) NOT NULL auto_increment, -- primary key - `borrowernumber` int(11) NULL default 0, -- foreign key from the borrowers table defining which patron requested a recall - `recalldate` date default NULL, -- the date the recall request was placed - `biblionumber` int(11) NULL default 0, -- foreign key from the biblio table defining which bib record this request is for - `branchcode` varchar(10) default NULL, -- foreign key from the branches table defining which branch the patron wishes to pick up their recall from - `cancellationdate` date default NULL, -- the date this recall was cancelled - `recallnotes` mediumtext, -- notes related to this recall - `priority` smallint(6) default NULL, -- where in the queue the patron sits - `found` varchar(1) default NULL, -- a one letter code defining what the status is of the recall is after it has been requested. R=requested, O=overdue, W=awaiting pickup, E=expired - `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP, -- the date and time this recall was last updated - `itemnumber` int(11) default NULL, -- foreign key from the items table defining the specific item the recall request was placed on - `waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library - `expirationdate` DATE DEFAULT NULL, -- the date the recall expires - `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- the optional itemtype of the item the patron is requesting - PRIMARY KEY (`recall_id`), - KEY `old_recalls_borrowernumber` (`borrowernumber`), - KEY `old_recalls_biblionumber` (`biblionumber`), - KEY `old_recalls_itemnumber` (`itemnumber`), - KEY `old_recalls_branchcode` (`branchcode`), - KEY `old_recalls_itemtype` (`itemtype`), - CONSTRAINT `old_recalls_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL, - CONSTRAINT `old_recalls_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE SET NULL ON UPDATE SET NULL, - CONSTRAINT `old_recalls_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE SET NULL ON UPDATE SET NULL, - CONSTRAINT `old_recalls_ibfk_4` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE SET NULL ON UPDATE SET NULL -) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; - --- -- Table structure for table `reserves` -- diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/recalls-reports.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/recalls-reports.inc index 0097a27..fd683d0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/recalls-reports.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/recalls-reports.inc @@ -8,6 +8,7 @@ Title Barcode Placed on + Waiting since Expires on Pickup location Status @@ -18,7 +19,11 @@ [% FOREACH recall IN recalls %] - + [% IF recall.is_cancelled || recall.has_expired || recall.is_finished %] + + [% ELSE %] + + [% END %] [% IF recall.is_requested || recall.is_waiting || recall.is_overdue %] @@ -52,6 +57,10 @@ [% recall.recalldate | $KohaDates %] + + [% IF recall.waitingdate %][% recall.waitingdate | $KohaDates %][% ELSE %]Not waiting[% END %] + + [% IF ( recall.is_waiting ) %] [% IF ( recall.expirationdate ) %] @@ -81,6 +90,8 @@ Cancelled [% ELSIF ( recall.is_overdue ) %] Overdue to be returned + [% ELSIF ( recall.is_finished ) %] + Closed [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/recalls.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/recalls.inc index 14d0436..3215544 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/recalls.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/recalls.inc @@ -59,6 +59,8 @@ Cancelled [% ELSIF ( recall.is_overdue ) %] Overdue to be returned + [% ELSIF ( recall.is_finished ) %] + Closed [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_overdue.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_overdue.tt index 5133938..4137632 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_overdue.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_overdue.tt @@ -5,7 +5,7 @@ [% INCLUDE 'doc-head-close.inc' %] - + [% INCLUDE 'datatables.inc' %] [% INCLUDE 'columns_settings.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_queue.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_queue.tt index b037bbf..c3d6b37 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_queue.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_queue.tt @@ -1,13 +1,11 @@ [% USE Koha %] [% USE KohaDates %] +[% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Koha › Circulation › Recalls queue [% INCLUDE 'doc-head-close.inc' %] - -[% INCLUDE 'datatables.inc' %] -[% INCLUDE 'columns_settings.inc' %] [% INCLUDE 'header.inc' %] @@ -31,7 +29,8 @@ [% IF recalls.count %]
- Select all + Select all | + Show old recalls [% INCLUDE 'recalls-reports.inc' %]
@@ -56,4 +55,16 @@ [% END %] +[% MACRO jsinclude BLOCK %] + [% INCLUDE 'datatables.inc' %] + [% INCLUDE 'columns_settings.inc' %] + + +[% END %] + + [% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_waiting.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_waiting.tt index b09acaf..8c700d6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_waiting.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/recalls_waiting.tt @@ -5,7 +5,7 @@ [% INCLUDE 'doc-head-close.inc' %] - + [% INCLUDE 'datatables.inc' %] + [% INCLUDE 'header.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js index c4f1f35..fcd4a35 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js +++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js @@ -274,6 +274,10 @@ $(document).ready(function() { onsite_checkout += " (" + INHOUSE_USE + ")"; } + if ( oObj.recalled == 1 ) { + title += " - This item has been recalled and the due date updated."; + } + title += " " + "You will be notified when your item is waiting to be picked up at the library.

[% ELSIF not error %]

All borrowable material is subject to recall if checked out and needed by someone else. We will ask the person who has checked out this item to return it so you may use it.

-

Warning: Your library does not allow recalls for x item types.


[% IF loggedinusername %]
diff --git a/opac/opac-recall.pl b/opac/opac-recall.pl index 6efe79f..ff0a191 100755 --- a/opac/opac-recall.pl +++ b/opac/opac-recall.pl @@ -23,6 +23,7 @@ use DateTime; use C4::Auth; use C4::Output; use C4::Context; +use C4::Letters; use Koha::Items; use Koha::Recall; use Koha::Recalls; @@ -48,17 +49,19 @@ my $biblio = Koha::Biblios->find($item->biblionumber); my $recalled = Koha::Recalls->find({ itemnumber => $itemnumber }); my $error; +if ( defined $recalled && $recalled->borrowernumber == $borrowernumber && ($recalled->is_requested || $recalled->is_waiting || $recalled->is_overdue) ){ + # can't place a recall on an item that this borrower has already recalled + # if recall is expired or cancelled, user may recall again + $error = 'duplicate'; +} + if ($op eq 'request'){ - if (!defined($borrowernumber)){ + if (!defined $borrowernumber){ # can't place recall if not logged in $error = 'notloggedin'; print $query->redirect('/cgi-bin/koha/opac-detail.pl?biblionumber='.$item->biblionumber); - } elsif ( defined $recalled && $recalled->borrowernumber == $borrowernumber && ($recalled->is_requested || $recalled->is_waiting || $recalled->is_overdue) ){ - # can't place a recall on an item that this borrower has already recalled - # if recall is expired or cancelled, user may recall again - $error = 'duplicate'; - } else { - # can recall + } elsif ( !defined $error ){ + # can recall my $borrower = Koha::Patrons->find($borrowernumber); my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule({ categorycode => $borrower->categorycode, itemtype => $item->itype, branchcode => $item->holdingbranch }); my $recall_request = Koha::Recall->new({ diff --git a/svc/checkouts b/svc/checkouts index 9a1b3a5..133fbed 100755 --- a/svc/checkouts +++ b/svc/checkouts @@ -171,6 +171,12 @@ while ( my $c = $sth->fetchrow_hashref() ) { my $av = Koha::AuthorisedValues->search({ category => 'DAMAGED', authorised_value => $c->{damaged} }); $damaged = $av->count ? $av->next->lib : ''; } + my $recall = Koha::Recalls->find({ itemnumber => $c->{itemnumber}, biblionumber => $c->{biblionumber}, status => { '=', ['R','O'] } }); + my $recalled = 0; + if ( defined $recall ){ + $recalled = 1; + } + my $checkout = { DT_RowId => $c->{itemnumber} . '-' . $c->{borrowernumber}, title => $c->{title}, @@ -228,6 +234,7 @@ while ( my $c = $sth->fetchrow_hashref() ) { cardnumber => $c->{cardnumber}, }, issued_today => !$c->{not_issued_today}, + recalled => $recalled, }; if ( $c->{not_issued_today} ) { diff --git a/t/db_dependent/Koha/Recalls.t b/t/db_dependent/Koha/Recalls.t index 76ad08f..1b421ff 100644 --- a/t/db_dependent/Koha/Recalls.t +++ b/t/db_dependent/Koha/Recalls.t @@ -62,9 +62,12 @@ is( $recall->is_waiting, 1, 'Recall is waiting for pickup' ); $recall->update({ status => 'C', cancellationdate => dt_from_string(), old => 1 }); is( $recall->is_cancelled, 1, 'Recall has been cancelled' ); -$recall->update({ status => 'E', expirationdate => dt_from_string() }); +$recall->update({ status => 'E', expirationdate => dt_from_string(), old => 1 }); is( $recall->has_expired, 1, 'Recall has expired' ); +$recall->update({ status => 'F', old => 1 }); +is ($recall->is_finished, 1, 'Recall is closed'); + is( $biblio->{biblionumber}, $recall->biblio->biblionumber, 'Can access biblio from recall' ); is( $patron->{borrowernumber}, $recall->patron->borrowernumber, 'Can access borrower from recall' ); @@ -73,6 +76,4 @@ is( $item->{itemnumber}, $recall->item->itemnumber, 'Can access item from recall is( $library->{branchcode}, $recall->library->branchcode, 'Can access branch from recall' ); -is( $checkout->{issue_id}, $recall->checkout->issue_id, 'Can access checkout from recall' ); - $schema->storage->txn_rollback; -- 2.1.4