From 975f52a4cfe14ed1b198531827fb6eb2f2c23b2f Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Mon, 14 Mar 2022 23:54:15 -1000 Subject: [PATCH] Bug 19532: (RM follow-up) Fix recalls.old is default 0 Signed-off-by: Fridolin Somers --- Koha/Item.pm | 8 +- Koha/Recalls.pm | 2 +- Koha/Template/Plugin/Biblio.pm | 2 +- catalogue/detail.pl | 2 +- circ/circulation.pl | 2 +- installer/data/mysql/db_revs/211200018.pl | 4 +- installer/data/mysql/kohastructure.sql | 4 +- misc/cronjobs/recalls/expire_recalls.pl | 2 +- opac/opac-user.pl | 4 +- recalls/recalls_queue.pl | 2 +- recalls/request.pl | 4 +- t/db_dependent/Koha/Item.t | 114 +++++++++++----------- 12 files changed, 79 insertions(+), 71 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 80dcdf0d31..5760ebc05c 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1461,7 +1461,13 @@ Return the relevant recall for this item sub recall { my ( $self ) = @_; - my @recalls = Koha::Recalls->search({ biblionumber => $self->biblionumber, old => undef }, { order_by => { -asc => 'recalldate' } })->as_list; + my @recalls = Koha::Recalls->search( + { + biblionumber => $self->biblionumber, + old => 0, + }, + { order_by => { -asc => 'recalldate' } } + )->as_list; foreach my $recall (@recalls) { if ( $recall->item_level_recall and $recall->itemnumber == $self->itemnumber ){ return $recall; diff --git a/Koha/Recalls.pm b/Koha/Recalls.pm index 204680ca1b..e7e360c2ec 100644 --- a/Koha/Recalls.pm +++ b/Koha/Recalls.pm @@ -233,7 +233,7 @@ sub move_recall { borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, itemnumber => [ $item->itemnumber, undef ], - old => undef + old => 0, }, { order_by => { -asc => 'recalldate' } } )->next; diff --git a/Koha/Template/Plugin/Biblio.pm b/Koha/Template/Plugin/Biblio.pm index 59030574c1..19e10163fc 100644 --- a/Koha/Template/Plugin/Biblio.pm +++ b/Koha/Template/Plugin/Biblio.pm @@ -60,7 +60,7 @@ sub CanArticleRequest { sub RecallsCount { my ( $self, $biblionumber ) = @_; - my $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => undef }); + my $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => 0 }); return $recalls->count; } diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 40e54b5394..e11e8d1567 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -403,7 +403,7 @@ foreach my $item (@items) { } if ( C4::Context->preference('UseRecalls') ) { - my $recall = Koha::Recalls->find({ itemnumber => $item->{itemnumber}, old => undef }); + my $recall = Koha::Recalls->find({ itemnumber => $item->{itemnumber}, old => 0 }); if ( defined $recall ) { $item->{recalled} = 1; $item->{recall} = $recall; diff --git a/circ/circulation.pl b/circ/circulation.pl index 358a34cb18..ac83bd2796 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -400,7 +400,7 @@ if (@$barcodes) { biblionumber => $item->biblionumber, itemnumber => [ undef, $item->itemnumber ], status => [ 'requested', 'waiting' ], - old => undef, + old => 0, borrowernumber => $patron->borrowernumber, } ); diff --git a/installer/data/mysql/db_revs/211200018.pl b/installer/data/mysql/db_revs/211200018.pl index 5b57350cdd..fac13045eb 100755 --- a/installer/data/mysql/db_revs/211200018.pl +++ b/installer/data/mysql/db_revs/211200018.pl @@ -20,12 +20,12 @@ return { cancellationdate datetime DEFAULT NULL, recallnotes mediumtext, priority smallint(6) DEFAULT NULL, - status status ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested', + status ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested', timestamp timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, itemnumber int(11) DEFAULT NULL, waitingdate datetime DEFAULT NULL, expirationdate datetime DEFAULT NULL, - old TINYINT(1) DEFAULT NULL, + old TINYINT(1) NOT NULL DEFAULT 0, item_level_recall TINYINT(1) NOT NULL DEFAULT 0, PRIMARY KEY (recall_id), KEY borrowernumber (borrowernumber), diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index b6b6f369ac..69b3613963 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4291,12 +4291,12 @@ CREATE TABLE recalls ( -- information related to recalls in Koha cancellationdate datetime 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 ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested' COMMENT "Request status", + status ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested', -- request status 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 datetime DEFAULT NULL, -- the date the item was marked as waiting for the patron at the library expirationdate datetime DEFAULT NULL, -- the date the recall expires - old TINYINT(1) DEFAULT 0, -- flag if the recall is old and no longer active, i.e. expired, cancelled or completed + old TINYINT(1) NOT NULL DEFAULT 0, -- flag if the recall is old and no longer active, i.e. expired, cancelled or completed item_level_recall TINYINT(1) NOT NULL DEFAULT 0, -- flag if item-level recall PRIMARY KEY (recall_id), KEY borrowernumber (borrowernumber), diff --git a/misc/cronjobs/recalls/expire_recalls.pl b/misc/cronjobs/recalls/expire_recalls.pl index 7da4fdcd41..2f6b4d2bf6 100755 --- a/misc/cronjobs/recalls/expire_recalls.pl +++ b/misc/cronjobs/recalls/expire_recalls.pl @@ -37,7 +37,7 @@ use C4::Log; cronlogaction(); -my $recalls = Koha::Recalls->search({ old => undef }); +my $recalls = Koha::Recalls->search({ old => 0 }); while( my $recall = $recalls->next ) { if ( ( $recall->requested or $recall->overdue ) and $recall->expirationdate and dt_from_string( $recall->expirationdate ) < dt_from_string() ){ # recall is requested or overdue and has surpassed the specified expiration date diff --git a/opac/opac-user.pl b/opac/opac-user.pl index ec18afabc9..5bc84c5b3f 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -309,7 +309,7 @@ if ( $pending_checkouts->count ) { # Useless test } if ( C4::Context->preference('UseRecalls') ) { - my $maybe_recalls = Koha::Recalls->search({ biblionumber => $issue->{biblionumber}, itemnumber => [ undef, $issue->{itemnumber} ], old => undef }); + my $maybe_recalls = Koha::Recalls->search({ biblionumber => $issue->{biblionumber}, itemnumber => [ undef, $issue->{itemnumber} ], old => 0 }); while( my $recall = $maybe_recalls->next ) { if ( $recall->checkout and $recall->checkout->issue_id == $issue->{issue_id} ) { $issue->{recall} = 1; @@ -345,7 +345,7 @@ $template->param( ); if ( C4::Context->preference('UseRecalls') ) { - my $recalls = Koha::Recalls->search( { borrowernumber => $borrowernumber, old => undef } ); + my $recalls = Koha::Recalls->search( { borrowernumber => $borrowernumber, old => 0 } ); $template->param( RECALLS => $recalls ); } diff --git a/recalls/recalls_queue.pl b/recalls/recalls_queue.pl index 197c02f27a..9479adaf46 100755 --- a/recalls/recalls_queue.pl +++ b/recalls/recalls_queue.pl @@ -42,7 +42,7 @@ if ( $op eq 'cancel_multiple_recalls' ) { $op = 'list' } elsif ( $op eq 'list' ) { - my $recalls = Koha::Recalls->search({ old => undef }); + my $recalls = Koha::Recalls->search({ old => 0 }); $template->param( recalls => $recalls, checkboxes => 1, diff --git a/recalls/request.pl b/recalls/request.pl index b086616ef8..da8ae1e88c 100755 --- a/recalls/request.pl +++ b/recalls/request.pl @@ -38,7 +38,7 @@ my ($template, $loggedinuser, $cookie)= get_template_and_user( my $op = $input->param('op') || 'list'; my @recall_ids = $input->multi_param('recall_ids'); my $biblionumber = $input->param('biblionumber'); -my $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => undef }); +my $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => 0 }); my $biblio = Koha::Biblios->find( $biblionumber ); if ( $op eq 'cancel_multiple_recalls' ) { @@ -49,7 +49,7 @@ if ( $op eq 'cancel_multiple_recalls' ) { } if ( $op eq 'list' ) { - $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => undef }); + $recalls = Koha::Recalls->search({ biblionumber => $biblionumber, old => 0 }); $biblio = Koha::Biblios->find( $biblionumber ); } diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 244ce5b50a..38d3f2234d 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -1165,6 +1165,64 @@ subtest 'columns_to_str' => sub { $cache->clear_from_cache("MarcSubfieldStructure-"); $schema->storage->txn_rollback; + +}; + +subtest 'store() tests' => sub { + + plan tests => 1; + + subtest '_set_found_trigger() tests' => sub { + + plan tests => 6; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $item = $builder->build_sample_item({ itemlost => 1, itemlost_on => dt_from_string() }); + + # Add a lost item debit + my $debit = $patron->account->add_debit( + { + amount => 10, + type => 'LOST', + item_id => $item->id, + interface => 'intranet', + } + ); + + my $lostreturn_policy = 'charge'; + + my $mocked_circ_rules = Test::MockModule->new('Koha::CirculationRules'); + $mocked_circ_rules->mock( 'get_lostreturn_policy', sub { return $lostreturn_policy; } ); + + # simulate it was found + $item->set( { itemlost => 0 } )->store; + + my $messages = $item->object_messages; + + my $message_1 = $messages->[0]; + + is( $message_1->type, 'info', 'type is correct' ); + is( $message_1->message, 'lost_refunded', 'message is correct' ); + + # Find the refund credit + my $credit = $debit->credits->next; + + is_deeply( + $message_1->payload, + { credit_id => $credit->id }, + 'type is correct' + ); + + my $message_2 = $messages->[1]; + + is( $message_2->type, 'info', 'type is correct' ); + is( $message_2->message, 'lost_charge', 'message is correct' ); + is( $message_2->payload, undef, 'no payload' ); + + $schema->storage->txn_rollback; + }; }; subtest 'Recalls tests' => sub { @@ -1362,59 +1420,3 @@ subtest 'Recalls tests' => sub { $schema->storage->txn_rollback; }; -subtest 'store() tests' => sub { - - plan tests => 1; - - subtest '_set_found_trigger() tests' => sub { - - plan tests => 6; - - $schema->storage->txn_begin; - - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - my $item = $builder->build_sample_item({ itemlost => 1, itemlost_on => dt_from_string() }); - - # Add a lost item debit - my $debit = $patron->account->add_debit( - { - amount => 10, - type => 'LOST', - item_id => $item->id, - interface => 'intranet', - } - ); - - my $lostreturn_policy = 'charge'; - - my $mocked_circ_rules = Test::MockModule->new('Koha::CirculationRules'); - $mocked_circ_rules->mock( 'get_lostreturn_policy', sub { return $lostreturn_policy; } ); - - # simulate it was found - $item->set( { itemlost => 0 } )->store; - - my $messages = $item->object_messages; - - my $message_1 = $messages->[0]; - - is( $message_1->type, 'info', 'type is correct' ); - is( $message_1->message, 'lost_refunded', 'message is correct' ); - - # Find the refund credit - my $credit = $debit->credits->next; - - is_deeply( - $message_1->payload, - { credit_id => $credit->id }, - 'type is correct' - ); - - my $message_2 = $messages->[1]; - - is( $message_2->type, 'info', 'type is correct' ); - is( $message_2->message, 'lost_charge', 'message is correct' ); - is( $message_2->payload, undef, 'no payload' ); - - $schema->storage->txn_rollback; - }; -}; -- 2.39.5