Bug 19532: Make recalls.status an ENUM
authorTomas Cohen Arazi <tomascohen@theke.io>
Fri, 11 Mar 2022 15:20:18 +0000 (12:20 -0300)
committerFridolin Somers <fridolin.somers@biblibre.com>
Tue, 15 Mar 2022 08:45:52 +0000 (22:45 -1000)
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 <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
17 files changed:
C4/Circulation.pm
C4/Overdues.pm
C4/XSLT.pm
Koha/Item.pm
Koha/Recall.pm
Koha/Recalls.pm
installer/data/mysql/atomicupdate/bug_19532_make_status_enum.pl [new file with mode: 0755]
installer/data/mysql/kohastructure.sql
t/db_dependent/Circulation.t
t/db_dependent/Circulation/CalcFine.t
t/db_dependent/Circulation/transferbook.t
t/db_dependent/Koha/Biblio.t
t/db_dependent/Koha/Item.t
t/db_dependent/Koha/Patron.t
t/db_dependent/Koha/Recall.t
t/db_dependent/Koha/Recalls.t
t/db_dependent/XSLT.t

index 7d0317f..03b5580 100644 (file)
@@ -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') ) {
index 05a6cf0..7d8877b 100644 (file)
@@ -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 ) {
index 7a8e4a9..747416e 100644 (file)
@@ -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
index 81b2c73..2b5ab5c 100644 (file)
@@ -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.
index 0ea8c1c..394e748 100644 (file)
@@ -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;
 }
index be9f88e..b0777c3 100644 (file)
@@ -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 (executable)
index 0000000..9935c9c
--- /dev/null
@@ -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"
+        });
+    },
+};
index 6d2af4f..b6b6f36 100644 (file)
@@ -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),
index 5f2a567..7cd2c0d 100755 (executable)
@@ -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" );
index bed49a4..52e9941 100755 (executable)
@@ -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' );
 
index c4de38e..946a302 100755 (executable)
@@ -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");
index 658f292..3751fdc 100755 (executable)
@@ -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' );
index 6b645b5..9db7de7 100755 (executable)
@@ -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;
index b950578..a5f5616 100755 (executable)
@@ -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" );
index d6e43f8..0805b4a 100755 (executable)
@@ -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,
index 02fd40b..4b0ac8f 100755 (executable)
@@ -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();
index c37f95e..5de621f 100755 (executable)
@@ -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,[]);