@@ -, +, @@ --- C4/Circulation.pm | 4 +- C4/Overdues.pm | 2 +- C4/XSLT.pm | 2 +- Koha/Item.pm | 4 +- Koha/Recall.pm | 70 ++++---- Koha/Recalls.pm | 24 ++- Koha/Schema/Result/Borrower.pm | 4 +- Koha/Schema/Result/Branchtransfer.pm | 13 +- Koha/Schema/Result/Recall.pm | 31 +++- .../bug_19532_make_status_enum.pl | 16 ++ installer/data/mysql/kohastructure.sql | 4 +- misc/devel/update_dbix_class_files.pl | 2 +- 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 - 21 files changed, 336 insertions(+), 293 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl --- a/C4/Circulation.pm +++ a/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') ) { --- a/C4/Overdues.pm +++ a/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 ) { --- a/C4/XSLT.pm +++ a/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 --- a/Koha/Item.pm +++ a/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. --- a/Koha/Recall.pm +++ a/Koha/Recall.pm @@ -144,8 +144,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 @@ -160,8 +159,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 @@ -176,8 +174,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 @@ -192,8 +189,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 @@ -208,8 +204,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 @@ -224,24 +219,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 @@ -294,10 +287,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' }); @@ -317,9 +310,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; @@ -327,10 +320,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. @@ -343,11 +337,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 @@ -380,9 +374,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; } @@ -416,7 +410,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; } @@ -432,7 +426,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; } @@ -447,22 +441,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; } --- a/Koha/Recalls.pm +++ a/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 --- a/Koha/Schema/Result/Borrower.pm +++ a/Koha/Schema/Result/Borrower.pm @@ -1930,8 +1930,8 @@ Composing rels: L -> permission __PACKAGE__->many_to_many("permissions", "user_permissions", "permission"); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-02-25 00:33:23 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:qPGKczsaKaLxxiNzZS5zdQ +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-03-11 15:09:09 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3iAhB3L4DomQBZKohTMSIg __PACKAGE__->has_many( "extended_attributes", --- a/Koha/Schema/Result/Branchtransfer.pm +++ a/Koha/Schema/Result/Branchtransfer.pm @@ -103,13 +103,15 @@ any comments related to the transfer =head2 reason data_type: 'enum' - extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","Recall","CancelRecall"]} + extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","TransferCancellation","Recall","CancelRecall"]} is_nullable: 1 +what triggered the transfer + =head2 cancellation_reason data_type: 'enum' - extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","ItemLost","WrongTransfer"]} + extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","ItemLost","WrongTransfer","Recall","CancelRecall"]} is_nullable: 1 what triggered the transfer cancellation @@ -183,6 +185,7 @@ __PACKAGE__->add_columns( "Reserve", "LostReserve", "CancelReserve", + "TransferCancellation", "Recall", "CancelRecall", ], @@ -205,6 +208,8 @@ __PACKAGE__->add_columns( "CancelReserve", "ItemLost", "WrongTransfer", + "Recall", + "CancelRecall", ], }, is_nullable => 1, @@ -271,8 +276,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-10-14 15:07:03 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:APOPxPO4uRpPHgSEhXbnTw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-03-11 15:09:09 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:fBpQLcKUzN3siNGU6EYFZA sub koha_object_class { 'Koha::Item::Transfer'; --- a/Koha/Schema/Result/Recall.pm +++ a/Koha/Schema/Result/Recall.pm @@ -74,9 +74,12 @@ __PACKAGE__->table("recalls"); =head2 status - data_type: 'varchar' + data_type: 'enum' + default_value: 'requested' + extra: {list => ["requested","overdue","waiting","in_transit","cancelled","expired","fulfilled"]} is_nullable: 1 - size: 1 + +Request status =head2 timestamp @@ -106,6 +109,7 @@ __PACKAGE__->table("recalls"); =head2 old data_type: 'tinyint' + default_value: 0 is_nullable: 1 =head2 item_level_recall @@ -152,7 +156,22 @@ __PACKAGE__->add_columns( "priority", { data_type => "smallint", is_nullable => 1 }, "status", - { data_type => "varchar", is_nullable => 1, size => 1 }, + { + data_type => "enum", + default_value => "requested", + extra => { + list => [ + "requested", + "overdue", + "waiting", + "in_transit", + "cancelled", + "expired", + "fulfilled", + ], + }, + is_nullable => 1, + }, "timestamp", { data_type => "timestamp", @@ -175,7 +194,7 @@ __PACKAGE__->add_columns( is_nullable => 1, }, "old", - { data_type => "tinyint", is_nullable => 1 }, + { data_type => "tinyint", default_value => 0, is_nullable => 1 }, "item_level_recall", { data_type => "tinyint", default_value => 0, is_nullable => 0 }, ); @@ -265,8 +284,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-10-14 15:07:03 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3OJBkRJzqxZpuRp0GYGixw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-03-11 15:09:09 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:0O1XsinX9DtgTZz+lDNd7g __PACKAGE__->add_columns( '+old' => { is_boolean => 1 }, --- a/installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl +++ a/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" + }); + }, +}; --- a/installer/data/mysql/kohastructure.sql +++ a/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), --- a/misc/devel/update_dbix_class_files.pl +++ a/misc/devel/update_dbix_class_files.pl @@ -38,7 +38,7 @@ my $db_name; my $db_user; my $db_passwd; my $koha_conf; -my $force; +my $force = 1; my $help; GetOptions( --- a/t/db_dependent/Circulation.t +++ a/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" ); --- a/t/db_dependent/Circulation/CalcFine.t +++ a/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' ); --- a/t/db_dependent/Circulation/transferbook.t +++ a/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"); --- a/t/db_dependent/Koha/Biblio.t +++ a/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' ); --- a/t/db_dependent/Koha/Item.t +++ a/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; --- a/t/db_dependent/Koha/Patron.t +++ a/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" ); --- a/t/db_dependent/Koha/Recall.t +++ a/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, --- a/t/db_dependent/Koha/Recalls.t +++ a/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(); --- a/t/db_dependent/XSLT.t +++ a/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,[]); --