View | Details | Raw Unified | Return to bug 22049
Collapse All | Expand All

(-)a/C4/Circulation.pm (-39 / +23 lines)
Lines 1910-1917 sub AddReturn { Link Here
1910
1910
1911
        if ($patron) {
1911
        if ($patron) {
1912
            eval {
1912
            eval {
1913
                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1913
                if ( $dropbox ) {
1914
                    $circControlBranch, $return_date, $patron->privacy );
1914
                    MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1915
                        $dropboxdate, $patron->privacy );
1916
                }
1917
                else {
1918
                    MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1919
                        $return_date, $patron->privacy );
1920
                }
1915
            };
1921
            };
1916
            unless ( $@ ) {
1922
            unless ( $@ ) {
1917
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
1923
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
Lines 2088-2117 sub AddReturn { Link Here
2088
2094
2089
=head2 MarkIssueReturned
2095
=head2 MarkIssueReturned
2090
2096
2091
  MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy);
2097
  MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy);
2092
2098
2093
Unconditionally marks an issue as being returned by
2099
Unconditionally marks an issue as being returned by
2094
moving the C<issues> row to C<old_issues> and
2100
moving the C<issues> row to C<old_issues> and
2095
setting C<returndate> to the current date, or
2101
setting C<returndate> to the current date.
2096
the last non-holiday date of the branccode specified in
2097
C<dropbox_branch> .  Assumes you've already checked that 
2098
it's safe to do this, i.e. last non-holiday > issuedate.
2099
2102
2100
if C<$returndate> is specified (in iso format), it is used as the date
2103
if C<$returndate> is specified (in iso format), it is used as the date
2101
of the return. It is ignored when a dropbox_branch is passed in.
2104
of the return.
2102
2105
2103
C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
2106
C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
2104
the old_issue is immediately anonymised
2107
the old_issue is immediately anonymised
2105
2108
2106
Ideally, this function would be internal to C<C4::Circulation>,
2109
Ideally, this function would be internal to C<C4::Circulation>,
2107
not exported, but it is currently needed by one 
2110
not exported, but it is currently used in misc/cronjobs/longoverdue.pl.
2108
routine in C<C4::Accounts>.
2109
2111
2110
=cut
2112
=cut
2111
2113
2112
sub MarkIssueReturned {
2114
sub MarkIssueReturned {
2113
    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;
2115
    my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_;
2114
2115
2116
2116
    # Retrieve the issue
2117
    # Retrieve the issue
2117
    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
2118
    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
Lines 2127-2167 sub MarkIssueReturned { Link Here
2127
        die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
2128
        die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
2128
            unless Koha::Patrons->find( $anonymouspatron );
2129
            unless Koha::Patrons->find( $anonymouspatron );
2129
    }
2130
    }
2130
    my $database = Koha::Database->new();
2131
    my $schema   = $database->schema;
2132
    my $dbh   = C4::Context->dbh;
2133
2131
2134
    my $query = 'UPDATE issues SET returndate=';
2132
    my $schema = Koha::Database->schema;
2135
    my @bind;
2136
    if ($dropbox_branch) {
2137
        my $calendar = Koha::Calendar->new( branchcode => $dropbox_branch );
2138
        my $dropboxdate = $calendar->addDate( DateTime->now( time_zone => C4::Context->tz), -1 );
2139
        $query .= ' ? ';
2140
        push @bind, $dropboxdate->strftime('%Y-%m-%d %H:%M');
2141
    } elsif ($returndate) {
2142
        $query .= ' ? ';
2143
        push @bind, $returndate;
2144
    } else {
2145
        $query .= ' now() ';
2146
    }
2147
    $query .= ' WHERE issue_id = ?';
2148
    push @bind, $issue_id;
2149
2133
2150
    # FIXME Improve the return value and handle it from callers
2134
    # FIXME Improve the return value and handle it from callers
2151
    $schema->txn_do(sub {
2135
    $schema->txn_do(sub {
2152
2136
2153
        # Update the returndate
2137
        # Update the returndate value
2154
        $dbh->do( $query, undef, @bind );
2138
        if ( $returndate ) {
2155
2139
            $issue->returndate( $returndate )->store->discard_changes; # update and refetch
2156
        # We just updated the returndate, so we need to refetch $issue
2140
        }
2157
        $issue->discard_changes;
2141
        else {
2142
            $issue->returndate( \'NOW()' )->store->discard_changes; # update and refetch
2143
        }
2158
2144
2159
        # Create the old_issues entry
2145
        # Create the old_issues entry
2160
        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
2146
        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
2161
2147
2162
        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
2148
        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
2163
        if ( $privacy == 2) {
2149
        if ( $privacy == 2) {
2164
            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $old_checkout->issue_id);
2150
            $old_checkout->borrowernumber($anonymouspatron)->store;
2165
        }
2151
        }
2166
2152
2167
        # And finally delete the issue
2153
        # And finally delete the issue
Lines 3714-3720 sub LostItem{ Link Here
3714
            #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
3700
            #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
3715
        }
3701
        }
3716
3702
3717
        MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;
3703
        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
3718
    }
3704
    }
3719
3705
3720
    #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
3706
    #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
Lines 3785-3791 sub ProcessOfflineReturn { Link Here
3785
            MarkIssueReturned(
3771
            MarkIssueReturned(
3786
                $issue->{borrowernumber},
3772
                $issue->{borrowernumber},
3787
                $itemnumber,
3773
                $itemnumber,
3788
                undef,
3789
                $operation->{timestamp},
3774
                $operation->{timestamp},
3790
            );
3775
            );
3791
            ModItem(
3776
            ModItem(
Lines 3820-3826 sub ProcessOfflineIssue { Link Here
3820
            MarkIssueReturned(
3805
            MarkIssueReturned(
3821
                $issue->{borrowernumber},
3806
                $issue->{borrowernumber},
3822
                $itemnumber,
3807
                $itemnumber,
3823
                undef,
3824
                $operation->{timestamp},
3808
                $operation->{timestamp},
3825
            );
3809
            );
3826
        }
3810
        }
(-)a/misc/cronjobs/longoverdue.pl (-1 / +1 lines)
Lines 313-319 foreach my $startrange (sort keys %$lost) { Link Here
313
                    LostItem( $row->{'itemnumber'}, 'cronjob', $mark_returned );
313
                    LostItem( $row->{'itemnumber'}, 'cronjob', $mark_returned );
314
                } elsif ( $mark_returned ) {
314
                } elsif ( $mark_returned ) {
315
                    my $patron = Koha::Patrons->find( $row->{borrowernumber} );
315
                    my $patron = Koha::Patrons->find( $row->{borrowernumber} );
316
                    MarkIssueReturned($row->{borrowernumber},$row->{itemnumber},undef,undef,$patron->privacy)
316
                    MarkIssueReturned($row->{borrowernumber},$row->{itemnumber},undef,$patron->privacy)
317
                }
317
                }
318
            }
318
            }
319
            $count++;
319
            $count++;
(-)a/t/db_dependent/Circulation/MarkIssueReturned.t (-6 / +5 lines)
Lines 61-88 subtest 'anonymous patron' => sub { Link Here
61
    # The next call will raise an error, because data are not correctly set
61
    # The next call will raise an error, because data are not correctly set
62
    t::lib::Mocks::mock_preference('AnonymousPatron', '');
62
    t::lib::Mocks::mock_preference('AnonymousPatron', '');
63
    my $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
63
    my $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
64
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, 'dropbox_branch', 'returndate', 2 ) };
64
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
65
    like ( $@, qr<Fatal error: the patron \(\d+\) .* AnonymousPatron>, 'AnonymousPatron is not set - Fatal error on anonymization' );
65
    like ( $@, qr<Fatal error: the patron \(\d+\) .* AnonymousPatron>, 'AnonymousPatron is not set - Fatal error on anonymization' );
66
    Koha::Checkouts->find( $issue->issue_id )->delete;
66
    Koha::Checkouts->find( $issue->issue_id )->delete;
67
67
68
    my $anonymous_borrowernumber = Koha::Patron->new({categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode} })->store->borrowernumber;
68
    my $anonymous_borrowernumber = Koha::Patron->new({categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode} })->store->borrowernumber;
69
    t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber);
69
    t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber);
70
    $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
70
    $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
71
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, 'dropbox_branch', 'returndate', 2 ) };
71
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
72
    is ( $@, q||, 'AnonymousPatron is set correctly - no error expected');
72
    is ( $@, q||, 'AnonymousPatron is set correctly - no error expected');
73
};
73
};
74
74
75
my ( $issue_id, $issue );
75
my ( $issue_id, $issue );
76
# The next call will return undef for invalid item number
76
# The next call will return undef for invalid item number
77
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, 'invalid_itemnumber', 'dropbox_branch', 'returndate', 0 ) };
77
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, 'invalid_itemnumber', undef, 0 ) };
78
is( $@, '', 'No die triggered by invalid itemnumber' );
78
is( $@, '', 'No die triggered by invalid itemnumber' );
79
is( $issue_id, undef, 'No issue_id returned' );
79
is( $issue_id, undef, 'No issue_id returned' );
80
80
81
# In the next call we return the item and try it another time
81
# In the next call we return the item and try it another time
82
$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
82
$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
83
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, undef, 0 ) };
83
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
84
is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" );
84
is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" );
85
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, undef, 0 ) };
85
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
86
is( $@, '', 'No crash on returning item twice' );
86
is( $@, '', 'No crash on returning item twice' );
87
is( $issue_id, undef, 'Cannot return an item twice' );
87
is( $issue_id, undef, 'Cannot return an item twice' );
88
88
89
- 

Return to bug 22049