From ea6eb301a6aa89b1749f71063ed21cc8c503a5e7 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 11 Mar 2022 12:20:18 -0300 Subject: [PATCH] Bug 19532: Make recalls.status an ENUM This patch makes the status attribute an ENUM, setting the default value as 'requested' as well. The chosen names are easier to read than single letters. Also, renamed F into fulfilled (this impacts methods names as well). This is because 'finished' or 'completed' is more a synonym for old => 1... Signed-off-by: Tomas Cohen Arazi Signed-off-by: Fridolin Somers --- C4/Circulation.pm | 4 +- C4/Overdues.pm | 2 +- C4/XSLT.pm | 2 +- Koha/Item.pm | 4 +- Koha/Recall.pm | 70 ++++---- Koha/Recalls.pm | 24 ++- .../bug_19532_make_status_enum.pl | 16 ++ installer/data/mysql/kohastructure.sql | 4 +- t/db_dependent/Circulation.t | 152 +++++++++--------- t/db_dependent/Circulation/CalcFine.t | 3 +- t/db_dependent/Circulation/transferbook.t | 32 ++-- t/db_dependent/Koha/Biblio.t | 61 +++---- t/db_dependent/Koha/Item.t | 111 +++++++------ t/db_dependent/Koha/Patron.t | 72 ++++----- t/db_dependent/Koha/Recall.t | 19 +-- t/db_dependent/Koha/Recalls.t | 2 +- t/db_dependent/XSLT.t | 1 - 17 files changed, 299 insertions(+), 280 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 7d0317fee5..03b5580d69 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -381,7 +381,7 @@ sub transferbook { } # find recall - my $recall = Koha::Recalls->find({ itemnumber => $itemnumber, status => 'T' }); + my $recall = Koha::Recalls->find({ itemnumber => $itemnumber, status => 'in_transit' }); if ( defined $recall and C4::Context->preference('UseRecalls') ) { # do a transfer if the recall branch is different to the item holding branch if ( $recall->branchcode eq $fbr ) { @@ -2354,7 +2354,7 @@ sub AddReturn { $request->status('RET') if $request; } - my $transfer_recall = Koha::Recalls->find({ itemnumber => $item->itemnumber, status => 'T' }); # all recalls that have triggered a transfer will have an allocated itemnumber + my $transfer_recall = Koha::Recalls->find({ itemnumber => $item->itemnumber, status => 'in_transit' }); # all recalls that have triggered a transfer will have an allocated itemnumber if ( $transfer_recall and $transfer_recall->branchcode eq $branch and C4::Context->preference('UseRecalls') ) { diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 05a6cf08e3..7d8877b303 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -260,7 +260,7 @@ sub CalcFine { # check if item has been recalled. recall should have been marked Overdue by cronjob, so only look at overdue recalls # only charge using recall_overdue_fine if there is an item-level recall for this particular item, OR a biblio-level recall - my @recalls = Koha::Recalls->search({ biblionumber => $item->{biblionumber}, old => undef, status => 'O' })->as_list; + my @recalls = Koha::Recalls->search({ biblionumber => $item->{biblionumber}, old => undef, status => 'overdue' })->as_list; my $bib_level_recall = 0; $bib_level_recall = 1 if scalar @recalls > 0; foreach my $recall ( @recalls ) { diff --git a/C4/XSLT.pm b/C4/XSLT.pm index 7a8e4a9112..747416ea7b 100644 --- a/C4/XSLT.pm +++ b/C4/XSLT.pm @@ -354,7 +354,7 @@ sub buildKohaItemsNamespace { my $status; my $substatus = ''; - my $recalls = Koha::Recalls->search({ itemnumber => $item->itemnumber, status => 'W' }); + my $recalls = Koha::Recalls->search({ itemnumber => $item->itemnumber, status => 'waiting' }); if ( $recalls->count ) { # recalls take priority over holds diff --git a/Koha/Item.pm b/Koha/Item.pm index 81b2c73bdd..2b5ab5c044 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1608,7 +1608,9 @@ Get the most relevant recall for this item. sub check_recalls { my ( $self ) = @_; - my @recalls = Koha::Recalls->search({ biblionumber => $self->biblionumber, itemnumber => [ $self->itemnumber, undef ], status => [ 'R','O','W','T' ] }, { order_by => { -asc => 'recalldate' } })->as_list; + my @recalls = + Koha::Recalls->search( { biblionumber => $self->biblionumber, itemnumber => [ $self->itemnumber, undef ], status => [ 'requested', 'overdue', 'waiting', 'in_transit' ] }, + { order_by => { -asc => 'recalldate' } } )->as_list; my $recall; # iterate through relevant recalls to find the best one. diff --git a/Koha/Recall.pm b/Koha/Recall.pm index 0ea8c1c36d..394e74848d 100644 --- a/Koha/Recall.pm +++ b/Koha/Recall.pm @@ -142,8 +142,7 @@ Return true if recall status is requested. sub requested { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'R'; + return $self->status eq 'requested'; } =head3 waiting @@ -158,8 +157,7 @@ Return true if recall is awaiting pickup. sub waiting { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'W'; + return $self->status eq 'waiting'; } =head3 overdue @@ -174,8 +172,7 @@ Return true if recall is overdue to be returned. sub overdue { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'O'; + return $self->status eq 'overdue'; } =head3 in_transit @@ -190,8 +187,7 @@ Return true if recall is in transit. sub in_transit { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'T'; + return $self->status eq 'in_transit'; } =head3 expired @@ -206,8 +202,7 @@ Return true if recall has expired. sub expired { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'E'; + return $self->status eq 'expired'; } =head3 cancelled @@ -222,24 +217,22 @@ Return true if recall has been cancelled. sub cancelled { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'C'; + return $self->status eq 'cancelled'; } -=head3 finished +=head3 fulfilled - if ( $recall->finished ) + if ( $recall->fulfilled ) - [% IF recall.finished %] + [% IF recall.fulfilled %] -Return true if recall is finished and has been fulfilled. +Return true if the recall has been fulfilled. =cut -sub finished { +sub fulfilled { my ( $self ) = @_; - my $status = $self->status; - return $status && $status eq 'F'; + return $self->status eq 'fulfilled'; } =head3 calc_expirationdate @@ -292,10 +285,10 @@ sub start_transfer { if ( $self->item_level_recall ) { # already has an itemnumber - $self->update({ status => 'T' }); + $self->update({ status => 'in_transit' }); } else { my $itemnumber = $params->{item}->itemnumber; - $self->update({ status => 'T', itemnumber => $itemnumber }); + $self->update({ status => 'in_transit', itemnumber => $itemnumber }); } my ( $dotransfer, $messages ) = C4::Circulation::transferbook({ to_branch => $self->branchcode, from_branch => $self->item->holdingbranch, barcode => $self->item->barcode, trigger => 'Recall' }); @@ -315,9 +308,9 @@ sub revert_transfer { my ( $self ) = @_; if ( $self->item_level_recall ) { - $self->update({ status => 'R' }); + $self->update({ status => 'requested' }); } else { - $self->update({ status => 'R', itemnumber => undef }); + $self->update({ status => 'requested', itemnumber => undef }); } return $self; @@ -325,10 +318,11 @@ sub revert_transfer { =head3 set_waiting - $recall->set_waiting({ - expirationdate => $expirationdate, - item => $item_object - }); + $recall->set_waiting( + { expirationdate => $expirationdate, + item => $item_object + } + ); Set the recall as waiting and update expiration date. Notify the recall requester. @@ -341,11 +335,11 @@ sub set_waiting { my $itemnumber; if ( $self->item_level_recall ) { $itemnumber = $self->itemnumber; - $self->update({ status => 'W', waitingdate => dt_from_string, expirationdate => $params->{expirationdate} }); + $self->update({ status => 'waiting', waitingdate => dt_from_string, expirationdate => $params->{expirationdate} }); } else { # biblio-level recall with no itemnumber. need to set itemnumber $itemnumber = $params->{item}->itemnumber; - $self->update({ status => 'W', waitingdate => dt_from_string, expirationdate => $params->{expirationdate}, itemnumber => $itemnumber }); + $self->update({ status => 'waiting', waitingdate => dt_from_string, expirationdate => $params->{expirationdate}, itemnumber => $itemnumber }); } # send notice to recaller to pick up item @@ -378,9 +372,9 @@ Revert recall waiting status. sub revert_waiting { my ( $self ) = @_; if ( $self->item_level_recall ){ - $self->update({ status => 'R', waitingdate => undef }); + $self->update({ status => 'requested', waitingdate => undef }); } else { - $self->update({ status => 'R', waitingdate => undef, itemnumber => undef }); + $self->update({ status => 'requested', waitingdate => undef, itemnumber => undef }); } return $self; } @@ -414,7 +408,7 @@ Set a recall as overdue when the recall has been requested and the borrower who sub set_overdue { my ( $self, $params ) = @_; my $interface = $params->{interface} || 'COMMANDLINE'; - $self->update({ status => 'O' }); + $self->update({ status => 'overdue' }); C4::Log::logaction( 'RECALLS', 'OVERDUE', $self->recall_id, "Recall status set to overdue", $interface ) if ( C4::Context->preference('RecallsLog') ); return $self; } @@ -430,7 +424,7 @@ Set a recall as expired. This may be done manually or by a cronjob, either when sub set_expired { my ( $self, $params ) = @_; my $interface = $params->{interface} || 'COMMANDLINE'; - $self->update({ status => 'E', old => 1, expirationdate => dt_from_string }); + $self->update({ status => 'expired', old => 1, expirationdate => dt_from_string }); C4::Log::logaction( 'RECALLS', 'EXPIRE', $self->recall_id, "Recall expired", $interface ) if ( C4::Context->preference('RecallsLog') ); return $self; } @@ -445,22 +439,22 @@ Set a recall as cancelled. This may be done manually, either by the borrower tha sub set_cancelled { my ( $self ) = @_; - $self->update({ status => 'C', old => 1, cancellationdate => dt_from_string }); + $self->update({ status => 'cancelled', old => 1, cancellationdate => dt_from_string }); C4::Log::logaction( 'RECALLS', 'CANCEL', $self->recall_id, "Recall cancelled", 'INTRANET' ) if ( C4::Context->preference('RecallsLog') ); return $self; } -=head3 set_finished +=head3 set_fulfilled - $recall->set_finished; + $recall->set_fulfilled; Set a recall as finished. This should only be called when the item allocated to a recall is checked out to the borrower who requested the recall. =cut -sub set_finished { +sub set_fulfilled { my ( $self ) = @_; - $self->update({ status => 'F', old => 1 }); + $self->update({ status => 'fulfilled', old => 1 }); C4::Log::logaction( 'RECALLS', 'FULFILL', $self->recall_id, "Recall fulfilled", 'INTRANET' ) if ( C4::Context->preference('RecallsLog') ); return $self; } diff --git a/Koha/Recalls.pm b/Koha/Recalls.pm index be9f88e11e..b0777c3901 100644 --- a/Koha/Recalls.pm +++ b/Koha/Recalls.pm @@ -33,9 +33,7 @@ Koha::Recalls - Koha Recalls Object set class =head1 API -=head2 Internal methods - -=cut +=head2 Class methods =head3 add_recall @@ -79,7 +77,7 @@ sub add_recall { recalldate => dt_from_string(), biblionumber => $biblio->biblionumber, branchcode => $branchcode, - status => 'R', + status => 'requested', itemnumber => defined $itemnumber ? $itemnumber : undef, expirationdate => $expirationdate, item_level_recall => defined $itemnumber ? 1 : 0, @@ -149,7 +147,9 @@ sub add_recall { borrowernumber => $borrowernumber, }); -A patron is attempting to check out an item that has been recalled by another patron. If the recall is requested/overdue, they have the option of cancelling the recall. If the recall is waiting, they also have the option of reverting the waiting status. +A patron is attempting to check out an item that has been recalled by another patron. +If the recall is requested/overdue, they have the option of cancelling the recall. +If the recall is waiting, they also have the option of reverting the waiting status. We can also fulfill the recall here if the recall is placed by this borrower. @@ -183,9 +183,17 @@ sub move_recall { if ( $message eq 'no action provided' and $item and $item->biblionumber and $borrowernumber ) { # move_recall was not called to revert or cancel, but was called to fulfill - my $recall = Koha::Recalls->search({ borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, itemnumber => [ $item->itemnumber, undef ], old => undef }, { order_by => { -asc => 'recalldate' } })->next; + my $recall = Koha::Recalls->search( + { + borrowernumber => $borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => [ $item->itemnumber, undef ], + old => undef + }, + { order_by => { -asc => 'recalldate' } } + )->next; if ( $recall ) { - $recall->set_finished; + $recall->set_fulfilled; $message = 'fulfilled'; } } @@ -193,6 +201,8 @@ sub move_recall { return $message; } +=head2 Internal methods + =head3 _type =cut diff --git a/installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl b/installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl new file mode 100755 index 0000000000..9935c9cbf2 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl @@ -0,0 +1,16 @@ +use Modern::Perl; + +return { + bug_number => "19532", + description => "Make recalls.status an enum", + up => sub { + my ($args) = @_; + my ($dbh) = @$args{qw(dbh)}; + + $dbh->do(q{ + ALTER TABLE recalls + MODIFY COLUMN + status ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested' COMMENT "Request status" + }); + }, +}; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 6d2af4f324..b6b6f369ac 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 varchar(1) DEFAULT NULL, -- a one letter code defining the status of the recall. R=requested, O=overdue, W=awaiting pickup, T=in transit, E=expired, C=cancelled, F=finished/completed + status ENUM('requested','overdue','waiting','in_transit','cancelled','expired','fulfilled') DEFAULT 'requested' COMMENT "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 NULL, -- flag if the recall is old and no longer active, i.e. expired, cancelled or completed + old TINYINT(1) 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/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 5f2a567729..7cd2c0d74f 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1455,7 +1455,6 @@ subtest "CanBookBeRenewed tests" => sub { borrowernumber => $recall_borrower->borrowernumber, branchcode => $recall_borrower->branchcode, item_level_recall => 1, - status => 'R', })->store; ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); is( $error, 'recalled', 'Cannot renew item that has been recalled' ); @@ -1468,7 +1467,6 @@ subtest "CanBookBeRenewed tests" => sub { borrowernumber => $recall_borrower->borrowernumber, branchcode => $recall_borrower->branchcode, item_level_recall => 0, - status => 'R', })->store; ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); is( $error, 'recalled', 'Cannot renew item if biblio is recalled and has no item allocated' ); @@ -1481,7 +1479,6 @@ subtest "CanBookBeRenewed tests" => sub { borrowernumber => $recall_borrower->borrowernumber, branchcode => $recall_borrower->branchcode, item_level_recall => 1, - status => 'R', })->store; ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); is( $renewokay, 1, 'Can renew item if item-level recall on biblio is not on this item' ); @@ -1494,7 +1491,6 @@ subtest "CanBookBeRenewed tests" => sub { borrowernumber => $recall_borrower->borrowernumber, branchcode => $recall_borrower->branchcode, item_level_recall => 0, - status => 'R', })->store; $recall->set_waiting({ item => $recall_item1 }); ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); @@ -1998,42 +1994,42 @@ subtest 'AddIssue | recalls' => sub { }); # checking out item that they have recalled - my $recall1 = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - item_level_recall => 1, - branchcode => $patron1->branchcode, - status => 'R', - })->store; + my $recall1 = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + item_level_recall => 1, + branchcode => $patron1->branchcode, + } + )->store; AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall1->recall_id } ); $recall1 = Koha::Recalls->find( $recall1->recall_id ); - is( $recall1->finished, 1, 'Recall was fulfilled when patron checked out item' ); + is( $recall1->fulfilled, 1, 'Recall was fulfilled when patron checked out item' ); AddReturn( $item->barcode, $item->homebranch ); # this item is has a recall request. cancel recall - my $recall2 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - item_level_recall => 1, - branchcode => $patron2->branchcode, - status => 'R', - })->store; + my $recall2 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + item_level_recall => 1, + branchcode => $patron2->branchcode, + } + )->store; AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall2->recall_id, cancel_recall => 'cancel' } ); $recall2 = Koha::Recalls->find( $recall2->recall_id ); is( $recall2->cancelled, 1, 'Recall was cancelled when patron checked out item' ); AddReturn( $item->barcode, $item->homebranch ); # this item is waiting to fulfill a recall. revert recall - my $recall3 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - item_level_recall => 1, - branchcode => $patron2->branchcode, - status => 'R', - })->store; + my $recall3 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + item_level_recall => 1, + branchcode => $patron2->branchcode, + } + )->store; $recall3->set_waiting; AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall3->recall_id, cancel_recall => 'revert' } ); $recall3 = Koha::Recalls->find( $recall3->recall_id ); @@ -3978,14 +3974,14 @@ subtest 'CanBookBeIssued | recalls' => sub { }); # item-level recall - my $recall = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - item_level_recall => 1, - branchcode => $patron1->branchcode, - status => 'R', - })->store; + my $recall = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + item_level_recall => 1, + branchcode => $patron1->branchcode, + } + )->store; my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron2, $item->barcode, undef, undef, undef, undef ); is( $needsconfirmation->{RECALLED}->recall_id, $recall->recall_id, "Another patron has placed an item-level recall on this item" ); @@ -3993,14 +3989,14 @@ subtest 'CanBookBeIssued | recalls' => sub { $recall->set_cancelled; # biblio-level recall - $recall = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => undef, - item_level_recall => 0, - branchcode => $patron1->branchcode, - status => 'R', - })->store; + $recall = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => undef, + item_level_recall => 0, + branchcode => $patron1->branchcode, + } + )->store; ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron2, $item->barcode, undef, undef, undef, undef ); is( $needsconfirmation->{RECALLED}->recall_id, $recall->recall_id, "Another patron has placed a biblio-level recall and this item is eligible to fill it" ); @@ -4008,15 +4004,15 @@ subtest 'CanBookBeIssued | recalls' => sub { $recall->set_cancelled; # biblio-level recall - $recall = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - biblionumber => $item->biblionumber, - itemnumber => undef, - item_level_recall => 0, - branchcode => $patron1->branchcode, - status => 'R', - })->store; - $recall->set_waiting({ item => $item, expirationdate => dt_from_string() }); + $recall = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => undef, + item_level_recall => 0, + branchcode => $patron1->branchcode, + } + )->store; + $recall->set_waiting( { item => $item, expirationdate => dt_from_string() } ); my ( undef, undef, undef, $messages ) = CanBookBeIssued( $patron1, $item->barcode, undef, undef, undef, undef ); is( $messages->{RECALLED}, $recall->recall_id, "This book can be issued by this patron and they have placed a recall" ); @@ -4058,42 +4054,42 @@ subtest 'AddReturn | recalls' => sub { # this item can fill a recall with pickup at this branch AddIssue( $patron1->unblessed, $item1->barcode ); - my $recall1 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - biblionumber => $item1->biblionumber, - itemnumber => $item1->itemnumber, - item_level_recall => 1, - branchcode => $item1->homebranch, - status => 'R', - })->store; + my $recall1 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + biblionumber => $item1->biblionumber, + itemnumber => $item1->itemnumber, + item_level_recall => 1, + branchcode => $item1->homebranch, + } + )->store; my ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item1->barcode, $item1->homebranch ); is( $messages->{RecallFound}->recall_id, $recall1->recall_id, "Recall found" ); $recall1->set_cancelled; # this item can fill a recall but needs transfer AddIssue( $patron1->unblessed, $item1->barcode ); - $recall1 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - biblionumber => $item1->biblionumber, - itemnumber => $item1->itemnumber, - item_level_recall => 1, - branchcode => $patron2->branchcode, - status => 'R', - })->store; + $recall1 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + biblionumber => $item1->biblionumber, + itemnumber => $item1->itemnumber, + item_level_recall => 1, + branchcode => $patron2->branchcode, + } + )->store; ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item1->barcode, $item1->homebranch ); is( $messages->{RecallNeedsTransfer}, $item1->homebranch, "Recall requiring transfer found" ); $recall1->set_cancelled; # this item is already in transit, do not ask to transfer AddIssue( $patron1->unblessed, $item1->barcode ); - $recall1 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - biblionumber => $item1->biblionumber, - itemnumber => $item1->itemnumber, - item_level_recall => 1, - branchcode => $patron2->branchcode, - status => 'R', - })->store; + $recall1 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + biblionumber => $item1->biblionumber, + itemnumber => $item1->itemnumber, + item_level_recall => 1, + branchcode => $patron2->branchcode, + } + )->store; $recall1->start_transfer; ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item1->barcode, $patron2->branchcode ); is( $messages->{TransferredRecall}->recall_id, $recall1->recall_id, "In transit recall found" ); diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index bed49a46dc..52e994155a 100755 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -232,7 +232,6 @@ subtest 'Recall overdue fines' => sub { recalldate => dt_from_string, biblionumber => $item->biblionumber, branchcode => $branch->{branchcode}, - status => 'R', itemnumber => $item->itemnumber, expirationdate => undef, item_level_recall => 1 @@ -242,7 +241,7 @@ subtest 'Recall overdue fines' => sub { my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); is( int($amount), 25, 'Use recall fine amount specified in circulation rules' ); - $recall->set_finished; + $recall->set_fulfilled; ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); is( int($amount), 5, 'With no recall, use normal fine amount' ); diff --git a/t/db_dependent/Circulation/transferbook.t b/t/db_dependent/Circulation/transferbook.t index c4de38e1ca..946a3024f7 100755 --- a/t/db_dependent/Circulation/transferbook.t +++ b/t/db_dependent/Circulation/transferbook.t @@ -157,27 +157,27 @@ subtest 'transfer already at destination' => sub { # recalls t::lib::Mocks::mock_preference('UseRecalls', 1); - my $recall = Koha::Recall->new({ - biblionumber => $item->biblionumber, - itemnumber => $item->itemnumber, - item_level_recall => 1, - borrowernumber => $patron->borrowernumber, - branchcode => $library->branchcode, - status => 'R', - })->store; + my $recall = Koha::Recall->new( + { biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + item_level_recall => 1, + borrowernumber => $patron->borrowernumber, + branchcode => $library->branchcode, + } + )->store; ( $recall, $dotransfer, $messages ) = $recall->start_transfer; is( $dotransfer, 0, 'Do not transfer recalled item, it has already arrived' ); is( $messages->{RecallPlacedAtHoldingBranch}, 1, "We found the recall"); my $item2 = $builder->build_object({ class => 'Koha::Items' }); # this item will have a different holding branch to the pickup branch - $recall = Koha::Recall->new({ - biblionumber => $item2->biblionumber, - itemnumber => $item2->itemnumber, - item_level_recall => 1, - borrowernumber => $patron->borrowernumber, - branchcode => $library->branchcode, - status => 'R', - })->store; + $recall = Koha::Recall->new( + { biblionumber => $item2->biblionumber, + itemnumber => $item2->itemnumber, + item_level_recall => 1, + borrowernumber => $patron->borrowernumber, + branchcode => $library->branchcode, + } + )->store; ( $recall, $dotransfer, $messages ) = $recall->start_transfer; is( $dotransfer, 1, 'Transfer of recalled item succeeded' ); is( $messages->{RecallFound}->recall_id, $recall->recall_id, "We found the recall"); diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index 658f292c51..3751fdca3e 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -888,6 +888,7 @@ subtest 'Recalls tests' => sub { plan tests => 12; $schema->storage->txn_begin; + my $item1 = $builder->build_sample_item; my $biblio = $item1->biblio; my $branchcode = $item1->holdingbranch; @@ -897,36 +898,36 @@ subtest 'Recalls tests' => sub { my $item2 = $builder->build_object({ class => 'Koha::Items', value => { holdingbranch => $branchcode, homebranch => $branchcode, biblionumber => $biblio->biblionumber, itype => $item1->effective_itemtype } }); t::lib::Mocks::mock_userenv({ patron => $patron1 }); - my $recall1 = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => $item1->itemnumber, - expirationdate => undef, - item_level_recall => 1 - })->store; - my $recall2 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => undef, - expirationdate => undef, - item_level_recall => 0 - })->store; - my $recall3 = Koha::Recall->new({ - borrowernumber => $patron3->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => $item1->itemnumber, - expirationdate => undef, - item_level_recall => 1 - })->store; + my $recall1 = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => $item1->itemnumber, + expirationdate => undef, + item_level_recall => 1 + } + )->store; + my $recall2 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => undef, + expirationdate => undef, + item_level_recall => 0 + } + )->store; + my $recall3 = Koha::Recall->new( + { borrowernumber => $patron3->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => $item1->itemnumber, + expirationdate => undef, + item_level_recall => 1 + } + )->store; my $recalls_count = $biblio->recalls->count; is( $recalls_count, 3, 'Correctly get number of active recalls for biblio' ); diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 6b645b5b52..9db7de79e0 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -1179,29 +1179,34 @@ subtest 'Recalls tests' => sub { my $patron1 = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $branchcode } }); my $patron2 = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $branchcode } }); my $patron3 = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $branchcode } }); - my $item2 = $builder->build_object({ class => 'Koha::Items', value => { holdingbranch => $branchcode, homebranch => $branchcode, biblionumber => $biblio->biblionumber, itype => $item1->effective_itemtype } }); - t::lib::Mocks::mock_userenv({ patron => $patron1 }); + my $item2 = $builder->build_object( + { class => 'Koha::Items', + value => { holdingbranch => $branchcode, homebranch => $branchcode, biblionumber => $biblio->biblionumber, itype => $item1->effective_itemtype } + } + ); - my $recall1 = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => $item1->itemnumber, - expirationdate => undef, - item_level_recall => 1 - })->store; - my $recall2 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => $item1->itemnumber, - expirationdate => undef, - item_level_recall =>1 - })->store; + t::lib::Mocks::mock_userenv( { patron => $patron1 } ); + + my $recall1 = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => $item1->itemnumber, + expirationdate => undef, + item_level_recall => 1 + } + )->store; + my $recall2 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => $item1->itemnumber, + expirationdate => undef, + item_level_recall => 1 + } + )->store; is( $item1->recall->borrowernumber, $patron1->borrowernumber, 'Correctly returns most relevant recall' ); @@ -1274,16 +1279,16 @@ subtest 'Recalls tests' => sub { C4::Circulation::AddIssue( $patron2->unblessed, $item1->barcode ); is( $item1->can_be_recalled({ patron => $patron1 }), 1, "Can recall item" ); - $recall1 = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => undef, - expirationdate => undef, - item_level_recall => 0 - })->store; + $recall1 = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => undef, + expirationdate => undef, + item_level_recall => 0 + } + )->store; # Patron2 has Item1 checked out. Patron1 has placed a biblio-level recall on Biblio1, so check if Item1 can fulfill Patron1's recall. @@ -1313,27 +1318,27 @@ subtest 'Recalls tests' => sub { # check_recalls tests - $recall1 = Koha::Recall->new({ - borrowernumber => $patron2->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => $item1->itemnumber, - expirationdate => undef, - item_level_recall => 1 - })->store; - $recall2 = Koha::Recall->new({ - borrowernumber => $patron1->borrowernumber, - recalldate => Koha::DateUtils::dt_from_string, - biblionumber => $biblio->biblionumber, - branchcode => $branchcode, - status => 'R', - itemnumber => undef, - expirationdate => undef, - item_level_recall => 0 - })->store; - $recall2->set_waiting({ item => $item1 }); + $recall1 = Koha::Recall->new( + { borrowernumber => $patron2->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => $item1->itemnumber, + expirationdate => undef, + item_level_recall => 1 + } + )->store; + $recall2 = Koha::Recall->new( + { borrowernumber => $patron1->borrowernumber, + recalldate => \'NOW()', + biblionumber => $biblio->biblionumber, + branchcode => $branchcode, + itemnumber => undef, + expirationdate => undef, + item_level_recall => 0 + } + )->store; + $recall2->set_waiting( { item => $item1 } ); # return a waiting recall my $check_recall = $item1->check_recalls; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index b95057812e..a5f56167cc 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -1060,42 +1060,42 @@ subtest 'recalls() tests' => sub { my $biblio2 = $builder->build_object({ class => 'Koha::Biblios' }); my $item2 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $biblio2->biblionumber } }); - Koha::Recall->new({ - biblionumber => $biblio1->biblionumber, - borrowernumber => $patron->borrowernumber, - itemnumber => $item1->itemnumber, - branchcode => $patron->branchcode, - recalldate => dt_from_string, - status => 'R', - item_level_recall => 1, - })->store; - Koha::Recall->new({ - biblionumber => $biblio2->biblionumber, - borrowernumber => $patron->borrowernumber, - itemnumber => $item2->itemnumber, - branchcode => $patron->branchcode, - recalldate => dt_from_string, - status => 'R', - item_level_recall => 1, - })->store; - Koha::Recall->new({ - biblionumber => $biblio1->biblionumber, - borrowernumber => $patron->borrowernumber, - itemnumber => undef, - branchcode => $patron->branchcode, - recalldate => dt_from_string, - status => 'R', - item_level_recall => 0, - })->store; - my $recall = Koha::Recall->new({ - biblionumber => $biblio1->biblionumber, - borrowernumber => $patron->borrowernumber, - itemnumber => undef, - branchcode => $patron->branchcode, - recalldate => dt_from_string, - status => 'R', - item_level_recall => 0, - })->store; + Koha::Recall->new( + { biblionumber => $biblio1->biblionumber, + borrowernumber => $patron->borrowernumber, + itemnumber => $item1->itemnumber, + branchcode => $patron->branchcode, + recalldate => \'NOW()', + item_level_recall => 1, + } + )->store; + Koha::Recall->new( + { biblionumber => $biblio2->biblionumber, + borrowernumber => $patron->borrowernumber, + itemnumber => $item2->itemnumber, + branchcode => $patron->branchcode, + recalldate => \'NOW()', + item_level_recall => 1, + } + )->store; + Koha::Recall->new( + { biblionumber => $biblio1->biblionumber, + borrowernumber => $patron->borrowernumber, + itemnumber => undef, + branchcode => $patron->branchcode, + recalldate => \'NOW()', + item_level_recall => 0, + } + )->store; + my $recall = Koha::Recall->new( + { biblionumber => $biblio1->biblionumber, + borrowernumber => $patron->borrowernumber, + itemnumber => undef, + branchcode => $patron->branchcode, + recalldate => \'NOW()', + item_level_recall => 0, + } + )->store; $recall->set_cancelled; is( $patron->recalls->count, 3, "Correctly gets this patron's active recalls" ); diff --git a/t/db_dependent/Koha/Recall.t b/t/db_dependent/Koha/Recall.t index d6e43f8d2e..0805b4af79 100755 --- a/t/db_dependent/Koha/Recall.t +++ b/t/db_dependent/Koha/Recall.t @@ -71,7 +71,7 @@ my $recall1 = Koha::Recall->new({ recalldate => dt_from_string, biblionumber => $biblio1->biblionumber, branchcode => $branch1, - status => 'R', + status => 'requested', itemnumber => $item1->itemnumber, expirationdate => undef, item_level_recall => 1 @@ -82,21 +82,20 @@ is( $recall1->item->homebranch, $item1->homebranch, "Recall item relationship co is( $recall1->patron->categorycode, $category1, "Recall patron relationship correctly linked" ); is( $recall1->library->branchname, Koha::Libraries->find( $branch1 )->branchname, "Recall library relationship correctly linked" ); is( $recall1->checkout->itemnumber, $item1->itemnumber, "Recall checkout relationship correctly linked" ); -is( $recall1->requested, 1, "Recall has been requested" ); +ok( $recall1->requested, "Recall has been requested" ); is( $recall1->should_be_overdue, 1, "Correctly calculated that recall should be marked overdue" ); $recall1->set_overdue({ interface => 'COMMANDLINE' }); -is( $recall1->overdue, 1, "Recall is overdue" ); +ok( $recall1->overdue, "Recall is overdue" ); $recall1->set_cancelled; -is( $recall1->cancelled, 1, "Recall is cancelled" ); +ok( $recall1->cancelled, "Recall is cancelled" ); my $recall2 = Koha::Recall->new({ borrowernumber => $patron1->borrowernumber, recalldate => dt_from_string, biblionumber => $biblio1->biblionumber, branchcode => $branch1, - status => 'R', itemnumber => $item1->itemnumber, expirationdate => undef, item_level_recall => 1 @@ -143,7 +142,6 @@ my $recall3 = Koha::Recall->new({ recalldate => dt_from_string, biblionumber => $biblio1->biblionumber, branchcode => $branch1, - status => 'R', itemnumber => $item1->itemnumber, expirationdate => undef, item_level_recall => 1 @@ -151,15 +149,15 @@ my $recall3 = Koha::Recall->new({ # test that recall gets T status $recall3->start_transfer; -is( $recall3->in_transit, 1, "Recall is in transit" ); +ok( $recall3->in_transit, "Recall is in transit" ); $recall3->revert_transfer; -is( $recall3->requested, 1, "Recall transfer has been cancelled and the status reverted" ); +ok( $recall3->requested, "Recall transfer has been cancelled and the status reverted" ); is( $recall3->itemnumber, $item1->itemnumber, "Item persists for item-level recall" ); # for testing purposes, pretend the item gets checked out -$recall3->set_finished; -is( $recall3->finished, 1, "Recall has been fulfilled" ); +$recall3->set_fulfilled; +ok( $recall3->fulfilled, "Recall has been fulfilled" ); C4::Circulation::AddIssue( $patron2->unblessed, $item1->barcode ); my $recall4 = Koha::Recall->new({ @@ -167,7 +165,6 @@ my $recall4 = Koha::Recall->new({ recalldate => dt_from_string, biblionumber => $biblio1->biblionumber, branchcode => $branch1, - status => 'R', itemnumber => undef, expirationdate => undef, item_level_recall => 0, diff --git a/t/db_dependent/Koha/Recalls.t b/t/db_dependent/Koha/Recalls.t index 02fd40b12f..4b0ac8f048 100755 --- a/t/db_dependent/Koha/Recalls.t +++ b/t/db_dependent/Koha/Recalls.t @@ -170,6 +170,6 @@ ok( $recall->cancelled, "Recall cancelled with move_recall" ); }); $message = Koha::Recalls->move_recall({ recall_id => $recall->recall_id, item => $item2, borrowernumber => $patron1->borrowernumber }); $recall = Koha::Recalls->find( $recall->recall_id ); -ok( $recall->finished, "Recall fulfilled with move_recall" ); +ok( $recall->fulfilled, "Recall fulfilled with move_recall" ); $schema->storage->txn_rollback(); diff --git a/t/db_dependent/XSLT.t b/t/db_dependent/XSLT.t index c37f95ee57..5de621f7f7 100755 --- a/t/db_dependent/XSLT.t +++ b/t/db_dependent/XSLT.t @@ -147,7 +147,6 @@ subtest 'buildKohaItemsNamespace status tests' => sub { biblionumber => $item->biblionumber, itemnumber => $item->itemnumber, branchcode => $item->holdingbranch, - status => 'R', }}); $recall->set_waiting; $xml = C4::XSLT::buildKohaItemsNamespace( $item->biblionumber,[]); -- 2.20.1