@@ -, +, @@ --- C4/Circulation.pm | 17 +++ C4/Reserves.pm | 109 ++++++++++++++---- Koha/Schema/Result/OldReserve.pm | 12 +- Koha/Schema/Result/Reserve.pm | 12 +- catalogue/moredetail.pl | 7 ++ catalogue/updateitem.pl | 23 +++- cataloguing/additem.pl | 34 +++++- ...20844-added-holdtype-field-to-reserves.sql | 3 + .../admin/preferences/circulation.pref | 6 + .../prog/en/modules/catalogue/moredetail.tt | 19 ++- .../prog/en/modules/cataloguing/additem.tt | 14 +++ t/db_dependent/Holds/RevertWaitingStatus.t | 64 +++++++++- 12 files changed, 286 insertions(+), 34 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_20844-added-holdtype-field-to-reserves.sql --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -3684,6 +3684,23 @@ sub LostItem{ DeleteTransfer($itemnumber); } + my ( $restype, $res ) = C4::Reserves::CheckReserves( $itemnumber ); + my $LostItemLevelHold; + if ( $restype eq "Waiting" || $restype eq "Reserved" ) { + if ( $res->{originalholdtype} eq "B" ) { + #originalholdtype equalling "B" means this is a bib level hold + C4::Reserves::RevertWaitingStatus({itemnumber => $itemnumber}); + + #If the LostBibLevelHoldsRevert syspref is enabled then revert the allocated reserve back to an unallocated bib-level hold + if ( C4::Context->preference('LostBibLevelHoldsRevert')) { + C4::Reserves::RevertItemLevelHoldToBibLevelHold({ reserveid => $res->{reserve_id} }); + } + } else { + #Otherwise originalholdtype equals "I" and is an item level hold + $LostItemLevelHold = 1; + } + } + return $LostItemLevelHold; } sub GetOfflineOperations { --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -130,6 +130,8 @@ BEGIN { &AlterPriority &ToggleLowestPriority + &RevertWaitingStatus + &RevertItemLevelHoldToBibLevelHold &ReserveSlip &ToggleSuspend &SuspendAll @@ -198,23 +200,47 @@ sub AddReserve { # Don't add itemtype limit if specific item is selected $itemtype = undef if $checkitem; - # updates take place here - my $hold = Koha::Hold->new( - { - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - reservedate => $resdate, - branchcode => $branch, - priority => $priority, - reservenotes => $notes, - itemnumber => $checkitem, - found => $found, - waitingdate => $waitingdate, - expirationdate => $expdate, - itemtype => $itemtype, - } - )->store(); - $hold->set_waiting() if $found eq 'W'; + # Create reserves with with originalholdtype="B" if biblio level hold and "I" if item level hold + my $hold; + if (!($checkitem)) { + # updates take place here + $hold = Koha::Hold->new( + { + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + reservedate => $resdate, + branchcode => $branch, + priority => $priority, + reservenotes => $notes, + itemnumber => $checkitem, + found => $found, + waitingdate => $waitingdate, + expirationdate => $expdate, + itemtype => $itemtype, + originalholdtype => 'B', + } + )->store(); + $hold->set_waiting() if $found eq 'W'; + } else { + # updates take place here + $hold = Koha::Hold->new( + { + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + reservedate => $resdate, + branchcode => $branch, + priority => $priority, + reservenotes => $notes, + itemnumber => $checkitem, + found => $found, + waitingdate => $waitingdate, + expirationdate => $expdate, + itemtype => $itemtype, + originalholdtype => 'I', + } + )->store(); + $hold->set_waiting() if $found eq 'W'; + } logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); @@ -1405,11 +1431,9 @@ sub _FixPriority { $hold = Koha::Holds->find( $reserve_id ); return unless $hold; } - unless ( $biblionumber ) { # FIXME This is a very weird API $biblionumber = $hold->biblionumber; } - if ( $rank eq "del" ) { # FIXME will crash if called without $hold $hold->cancel; } @@ -1521,7 +1545,8 @@ sub _Findgroupreserve { biblioitems.biblioitemnumber AS biblioitemnumber, reserves.itemnumber AS itemnumber, reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype + reserves.itemtype AS itemtype, + reserves.originalholdtype AS originalholdtype FROM reserves JOIN biblioitems USING (biblionumber) JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber) @@ -1556,7 +1581,8 @@ sub _Findgroupreserve { biblioitems.biblioitemnumber AS biblioitemnumber, reserves.itemnumber AS itemnumber, reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype + reserves.itemtype AS itemtype, + reserves.originalholdtype AS originalholdtype FROM reserves JOIN biblioitems USING (biblionumber) JOIN hold_fill_targets USING (biblionumber, borrowernumber) @@ -1590,7 +1616,8 @@ sub _Findgroupreserve { reserves.timestamp AS timestamp, reserves.itemnumber AS itemnumber, reserves.reserve_id AS reserve_id, - reserves.itemtype AS itemtype + reserves.itemtype AS itemtype, + reserves.originalholdtype AS originalholdtype FROM reserves WHERE reserves.biblionumber = ? AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?) @@ -1910,6 +1937,44 @@ sub RevertWaitingStatus { _FixPriority( { biblionumber => $reserve->{biblionumber} } ); } + +=head2 RevertItemLevelHoldToBibLevelHold + + RevertItemLevelHoldToBibLevelHold({ reserveid => $reserveid }); + + Reverts a item level hold (which was originally a bib-level hold, which is determined by the value of reserves.originalholdtype) back to a bib-level hold. + +=cut + +sub RevertItemLevelHoldToBibLevelHold { + my ( $params ) = @_; + my $reserve_id = $params->{'reserveid'}; + + return unless ( $reserve_id ); + my $dbh = C4::Context->dbh; + + #Get the waiting reserve we want to revert + my $query = " + SELECT * FROM reserves + WHERE reserve_id = ? + AND itemnumber IS NOT NULL + "; + my $sth = $dbh->prepare( $query ); + $sth->execute( $reserve_id ); + my $reserve = $sth->fetchrow_hashref(); + + ## Set the itemnumber to 'NULL' + $query = " + UPDATE reserves + SET + itemnumber = NULL + WHERE + reserve_id = ? + "; + $sth = $dbh->prepare( $query ); + $sth->execute( $reserve_id ); +} + =head2 ReserveSlip ReserveSlip( --- a/Koha/Schema/Result/OldReserve.pm +++ a/Koha/Schema/Result/OldReserve.pm @@ -137,6 +137,12 @@ __PACKAGE__->table("old_reserves"); is_nullable: 1 size: 10 +=head2 originalholdtype + + data_type: 'varchar' + is_nullable: 1 + size: 1 + =cut __PACKAGE__->add_columns( @@ -192,6 +198,8 @@ __PACKAGE__->add_columns( }, "itemtype", { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, + "originalholdtype", + { data_type => "varchar", is_nullable => 1, size => 1 }, ); =head1 PRIMARY KEY @@ -289,8 +297,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:54 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ynb6NtiTN8ZZur2YCvxlNg +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-09-02 09:48:38 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:cc4o9r7wLXapCJJ3WfDy4g # You can replace this text with custom content, and it will be preserved on regeneration --- a/Koha/Schema/Result/Reserve.pm +++ a/Koha/Schema/Result/Reserve.pm @@ -141,6 +141,12 @@ __PACKAGE__->table("reserves"); is_nullable: 1 size: 10 +=head2 originalholdtype + + data_type: 'varchar' + is_nullable: 1 + size: 1 + =cut __PACKAGE__->add_columns( @@ -206,6 +212,8 @@ __PACKAGE__->add_columns( }, "itemtype", { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, + "originalholdtype", + { data_type => "varchar", is_nullable => 1, size => 1 }, ); =head1 PRIMARY KEY @@ -313,8 +321,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:54 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:2GCET9quFpUvzuN7MUWZNw +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-08-25 12:05:26 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:YIZXhPQwRJXrzYkdmVQH7Q __PACKAGE__->belongs_to( "item", --- a/catalogue/moredetail.pl +++ a/catalogue/moredetail.pl @@ -71,6 +71,13 @@ my $itemnumber = $query->param('itemnumber'); my $data = &GetBiblioData($biblionumber); my $dewey = $data->{'dewey'}; my $showallitems = $query->param('showallitems'); +my $lostitemlevelhold = $query->param('LostItemLevelHold'); +my $lostitemnumber = $query->param('lostitemnumber'); +my $itemlost = $query->param('itemlost'); + +if ($lostitemlevelhold && $itemlost) { + $template->param(LostItemLevelHold => $lostitemlevelhold, itemlost => $itemlost, lostitemnumber => $lostitemnumber, biblionumber => $biblionumber); +} #coping with subscriptions my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); --- a/catalogue/updateitem.pl +++ a/catalogue/updateitem.pl @@ -26,6 +26,7 @@ use C4::Items; use C4::Output; use C4::Circulation; use C4::Reserves; +use Data::Dumper; my $cgi= new CGI; @@ -39,6 +40,8 @@ my $itemnotes=$cgi->param('itemnotes'); my $itemnotes_nonpublic=$cgi->param('itemnotes_nonpublic'); my $withdrawn=$cgi->param('withdrawn'); my $damaged=$cgi->param('damaged'); +my $cancelhold = $cgi->param('CancelHold'); +my $retainhold = $cgi->param('RetainHold'); my $confirm=$cgi->param('confirm'); my $dbh = C4::Context->dbh; @@ -53,8 +56,19 @@ for ($damaged,$itemlost,$withdrawn) { } } +if ($cancelhold) { + my ($restype, $res ) = C4::Reserves::CheckReserves( $itemnumber ); + my $hold = Koha::Holds->find($res->{reserve_id}); + $hold->cancel; +} + +#Assigning variables for use in cancelling item level holds when item marked as 'Lost' +my $LostItemLevelHold; +my $originalitemnumber = $itemnumber; + # modify MARC item if input differs from items table. my $item_changes = {}; + if (defined $itemnotes_nonpublic) { # i.e., itemnotes_nonpublic parameter passed from form checkauth($cgi, 0, {editcatalogue => 'edit_items'}, 'intranet'); if ((not defined $item_data_hashref->{'itemnotes_nonpublic'}) or $itemnotes_nonpublic ne $item_data_hashref->{'itemnotes_nonpublic'}) { @@ -79,7 +93,12 @@ elsif (defined $itemnotes) { # i.e., itemnotes parameter passed from form } ModItem($item_changes, $biblionumber, $itemnumber); +$LostItemLevelHold = LostItem($itemnumber, 'moredetail') if $itemlost; + +if ( $LostItemLevelHold) { + print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&LostItemLevelHold=$LostItemLevelHold&itemlost=$itemlost&lostitemnumber=$itemnumber#item$itemnumber"); +} else { + print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); +} -LostItem($itemnumber, 'moredetail') if $itemlost; -print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); --- a/cataloguing/additem.pl +++ a/cataloguing/additem.pl @@ -40,7 +40,7 @@ use C4::Search; use Storable qw(thaw freeze); use URI::Escape; use C4::Members; - +use Data::Dumper; use MARC::File::XML; use URI::Escape; @@ -385,9 +385,18 @@ my $fa_barcode = $input->param('barcode'); my $fa_branch = $input->param('branch'); my $fa_stickyduedate = $input->param('stickyduedate'); my $fa_duedatespec = $input->param('duedatespec'); - +my $cancelhold = $input->param('CancelHold'); +my $retainhold = $input->param('RetainHold'); my $frameworkcode = &GetFrameworkCode($biblionumber); +if ($cancelhold || $retainhold) { + $op =''; +} + +#Assigning variables for use in cancelling item level holds when item marked as 'Lost' +my $LostItemLevelHold; +my $originalitemnumber = $itemnumber; + # Defining which userflag is needing according to the framework currently used my $userflags; if (defined $input->param('frameworkcode')) { @@ -692,6 +701,7 @@ if ($op eq "additem") { # check that the barcode don't exist already my $addedolditem = TransformMarcToKoha($itemtosave); my $exist_itemnumber = get_item_from_barcode($addedolditem->{'barcode'}); + my $lostitemnumber; if ($exist_itemnumber && $exist_itemnumber != $itemnumber) { push @errors,"barcode_not_unique"; } else { @@ -700,9 +710,14 @@ if ($op eq "additem") { $itemnumber = q{}; my $olditemlost = $item->{itemlost}; my $newitemlost = $newitem->{itemlost}; - LostItem( $item->{itemnumber}, 'additem' ) + $LostItemLevelHold = LostItem( $item->{itemnumber}, 'additem' ) if $newitemlost && $newitemlost ge '1' && !$olditemlost; } + + if ($LostItemLevelHold) { + $lostitemnumber = $originalitemnumber; + } + $template->param( LostItemLevelHold => $LostItemLevelHold, lostitemnumber => $lostitemnumber ); $nextop="additem"; } elsif ($op eq "delinkitem"){ my $analyticfield = '773'; @@ -720,6 +735,12 @@ if ($op eq "additem") { my $modbibresult = ModBiblio($record, $biblionumber,''); } +if ($cancelhold) { + my ( $restype, $res ) = C4::Reserves::CheckReserves( $originalitemnumber ); + my $hold = Koha::Holds->find($res->{reserve_id}); + $hold->cancel; +} + # #------------------------------------------------------------------------------- # build screen with existing items. and "new" one @@ -905,6 +926,13 @@ foreach my $tag ( keys %{$tagslib}){ my $item = Koha::Items->find($itemnumber); # We certainly want to fetch it earlier +#If you are trying to mark an item as lost which has an item level hold on it return $originalitemnumber +if ($LostItemLevelHold) { + $template->param( itemnumber => $originalitemnumber); +} else { + $template->param( itemnumber => $itemnumber ); +} + # what's the next op ? it's what we are not in : an add if we're editing, otherwise, and edit. $template->param( biblionumber => $biblionumber, --- a/installer/data/mysql/atomicupdate/bug_20844-added-holdtype-field-to-reserves.sql +++ a/installer/data/mysql/atomicupdate/bug_20844-added-holdtype-field-to-reserves.sql @@ -0,0 +1,3 @@ +ALTER TABLE reserves ADD originalholdtype VARCHAR(1); +ALTER TABLE old_reserves ADD originalholdtype VARCHAR(1); +INSERT INTO systempreferences (variable, value, options, explanation, type) VALUES ('LostBibLevelHoldsRevert', 1, 'Enable|Disable', 'If the item which has an allocated hold on it which was originally a bib-level hold then when the item is marked as lost revert the hold to a bib-level hold', 'YesNo'); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -595,6 +595,12 @@ Circulation: - pref: ReservesMaxPickUpDelay class: integer - days. + - + - If the item which has an allocated hold on it which was originally a bib-level hold is marked as lost then revert the hold to a bib-level hold + - pref: LostBibLevelHoldsRevert + choices: + yes: "Enable" + no: "Disable" - - pref: LostItemCancelOutstandingTransfers choices: --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -42,14 +42,29 @@ Show all items[% END %] +
[% IF ( ONLY_ONE ) %]
You are only viewing one item. View All
[% END %] + [% FOREACH ITEM_DAT IN ITEM_DATA %]
-

Barcode [% ITEM_DAT.barcode | html %] [% IF ( ITEM_DAT.notforloantext ) %][% ITEM_DAT.notforloantext | html %] [% END %]

-

Item information [% IF ( CAN_user_editcatalogue_edit_catalogue ) %][% UNLESS ( ITEM_DAT.nomod ) %] +

Barcode [% ITEM_DAT.barcode | html %] [% IF ( ITEM_DAT.notforloantext ) %][% ITEM_DAT.notforloantext | html %] [% END %]

+ [% IF ((ITEM_DAT.itemnumber == lostitemnumber) && (LostItemLevelHold)) %] +
+
+ + + + This item has an item-level hold on it: + + +
+
+ [% END %] + +

Item information [% IF ( CAN_user_editcatalogue_edit_catalogue ) %][% UNLESS ( ITEM_DAT.nomod ) %] [% IF ( CAN_user_editcatalogue_edit_items ) %] [Edit Item] [% END %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt @@ -258,6 +258,20 @@ function confirm_deletion() { [% ELSE %]

Edit Item #[% itemnumber | html %][% IF ( barcode ) %] / Barcode [% barcode | html %][% END %]

[% END %] + + + [% IF (LostItemLevelHold) %] +
+
+ + This item has an item-level hold on it: + + +
+
+ [% END %] + +
    [% FOREACH ite IN item %] --- a/t/db_dependent/Holds/RevertWaitingStatus.t +++ a/t/db_dependent/Holds/RevertWaitingStatus.t @@ -16,7 +16,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 12; use MARC::Record; use Koha::Libraries; @@ -109,6 +109,68 @@ ok( scalar @$priorities == 2, 'Only 2 holds remain in the reserves table' ); ok( $priorities->[0]->[0] == 1, 'First hold has a priority of 1' ); ok( $priorities->[1]->[0] == 2, 'Second hold has a priority of 2' ); +#Bug 20844 +#Test reverting the waiting status of holds when the reserved items are set to lost + +#Create bib-level hold and test what happens when the item an allocated bib-level hold is allocated on is lost when the LostBibLevelHoldsRevert syspref is enabled +my $hold_id = C4::Reserves::AddReserve( + $branchcode, + 4, + $biblionumber, + my $bibitems = q{}, + my $priority, + my $resdate, + my $expdate, + my $notes = q{}, + $title, + my $checkitem, + my $found, +); + +my $hold = Koha::Holds->find($hold_id); +is( $hold->originalholdtype, 'B', 'Holds originalholdtype is "B" which is correct' ); +ModReserveAffect($itemnumber, 4, 0); #Confirm the hold - now it's an allocated bib-level hold on $itemnumber and has status is 'W' +$hold = Koha::Holds->find($hold_id); +is( $hold->found, 'W', 'Hold status is "W" (waiting) which is correct' ); +is( $hold->itemnumber, $itemnumber, 'Bib-level hold is allocated and has an itemnumber' ); +t::lib::Mocks::mock_preference( 'LostBibLevelHoldsRevert', 1 ); + +if (C4::Context->preference('LostBibLevelHoldsRevert') ) { + #When item is set to lost revert hold to bib-level (i.e. remove the itemnumber in hold record) and change reserves.found to '' + RevertWaitingStatus({ itemnumber => $itemnumber }); + RevertItemLevelHoldToBibLevelHold({ reserveid => $hold_id }); +} +$hold = Koha::Holds->find($hold_id); +is( $hold->found, undef, 'reserve.found reverted to undef now item is lost' ); +is( $hold->itemnumber, undef, 'Hold reverted back to unallocated bib level hold' ); + +#Create item-level hold and test what happens when the reserved item is set to lost +$hold_id = C4::Reserves::AddReserve( + $branchcode, + 4, + $biblionumber, + $bibitems = q{}, + $priority, + $resdate, + $expdate, + $notes = q{}, + $title, + $itemnumber, + $found, +); + +$hold = Koha::Holds->find($hold_id); +is( $hold->itemnumber, $itemnumber, 'This is an item-level hold' ); + +is( $hold->originalholdtype, 'I', 'Holds originalholdtype is correct' ); +ModReserveAffect($itemnumber, 4, 0); #Confirm the hold - now it's a waiting item-level hold with a reserves.found value of 'W' +$hold = Koha::Holds->find($hold_id); +is( $hold->found, 'W', 'Hold status is waiting' ); + +RevertWaitingStatus({ itemnumber => $itemnumber }); +$hold = Koha::Holds->find($hold_id); +is( $hold->found, undef, 'Reserves found value is reverted to undef as the reserved item has been lost' ); + $schema->storage->txn_rollback; # Helper method to set up a Biblio. --