Attachment #83543 for bug #22049

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





        if ($patron) {
            eval {
                if ( $dropbox ) {
                    MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
                        $dropboxdate, $patron->privacy );
                }
                else {
                    MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
                        $return_date, $patron->privacy );
                }
            };
            unless ( $@ ) {
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {


=head2 MarkIssueReturned

  MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy);

Unconditionally marks an issue as being returned by
moving the C<issues> row to C<old_issues> and
setting C<returndate> to the current date.
the last non-holiday date of the branccode specified in
C<dropbox_branch> .  Assumes you've already checked that 
it's safe to do this, i.e. last non-holiday > issuedate.

if C<$returndate> is specified (in iso format), it is used as the date
of the return.

C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
the old_issue is immediately anonymised

Ideally, this function would be internal to C<C4::Circulation>,
not exported, but it is currently used in misc/cronjobs/longoverdue.pl.
routine in C<C4::Accounts>.

=cut

sub MarkIssueReturned {
    my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_;


    # Retrieve the issue
    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;

        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."
            unless Koha::Patrons->find( $anonymouspatron );
    }
    my $database = Koha::Database->new();
    my $schema   = $database->schema;
    my $dbh   = C4::Context->dbh;

    my $schema = Koha::Database->schema;
    my @bind;
    if ($dropbox_branch) {
        my $calendar = Koha::Calendar->new( branchcode => $dropbox_branch );
        my $dropboxdate = $calendar->addDate( DateTime->now( time_zone => C4::Context->tz), -1 );
        $query .= ' ? ';
        push @bind, $dropboxdate->strftime('%Y-%m-%d %H:%M');
    } elsif ($returndate) {
        $query .= ' ? ';
        push @bind, $returndate;
    } else {
        $query .= ' now() ';
    }
    $query .= ' WHERE issue_id = ?';
    push @bind, $issue_id;

    # FIXME Improve the return value and handle it from callers
    $schema->txn_do(sub {

        # Update the returndate value
        if ( $returndate ) {
            $issue->returndate( $returndate )->store->discard_changes; # update and refetch
        }
        else {
            $issue->returndate( \'NOW()' )->store->discard_changes; # update and refetch
        }

        # Create the old_issues entry
        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;

        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
        if ( $privacy == 2) {
            $old_checkout->borrowernumber($anonymouspatron)->store;
        }

        # And finally delete the issue

            #warn " $issues->{'borrowernumber'}  /  $itemnumber ";
        }

        MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
    }

    #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)

            MarkIssueReturned(
                $issue->{borrowernumber},
                $itemnumber,
                undef,
                $operation->{timestamp},
            );
            ModItem(

            MarkIssueReturned(
                $issue->{borrowernumber},
                $itemnumber,
                undef,
                $operation->{timestamp},
            );
        }

Lines 313-319 foreach my $startrange (sort keys %$lost) { Link Here

(-)a/misc/cronjobs/longoverdue.pl (-1 / +1 lines)
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++;




    # The next call will raise an error, because data are not correctly set
    t::lib::Mocks::mock_preference('AnonymousPatron', '');
    my $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
    like ( $@, qr<Fatal error: the patron \(\d+\) .* AnonymousPatron>, 'AnonymousPatron is not set - Fatal error on anonymization' );
    Koha::Checkouts->find( $issue->issue_id )->delete;

    my $anonymous_borrowernumber = Koha::Patron->new({categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode} })->store->borrowernumber;
    t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber);
    $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
    eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
    is ( $@, q||, 'AnonymousPatron is set correctly - no error expected');
};

my ( $issue_id, $issue );
# The next call will return undef for invalid item number
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, 'invalid_itemnumber', undef, 0 ) };
is( $@, '', 'No die triggered by invalid itemnumber' );
is( $issue_id, undef, 'No issue_id returned' );

# In the next call we return the item and try it another time
$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" );
eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
is( $@, '', 'No crash on returning item twice' );
is( $issue_id, undef, 'Cannot return an item twice' );

- 

Return to bug 22049

Lines 1909-1916 sub AddReturn { Link Here

(-)a/C4/Circulation.pm (-39 / +23 lines)
1909		1909
1910	if ($patron) {	1910	if ($patron) {
1911	eval {	1911	eval {
1912	MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},	1912	if ( $dropbox ) {
1913	$circControlBranch, $return_date, $patron->privacy );	1913	MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
		1914	$dropboxdate, $patron->privacy );
		1915	}
		1916	else {
		1917	MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
		1918	$return_date, $patron->privacy );
		1919	}
1914	};	1920	};
1915	unless ( $@ ) {	1921	unless ( $@ ) {
1916	if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) \|\| $return_date ) {	1922	if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) \|\| $return_date ) {
Lines 2087-2116 sub AddReturn { Link Here
2087		2093
2088	=head2 MarkIssueReturned	2094	=head2 MarkIssueReturned
2089		2095
2090	MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy);	2096	MarkIssueReturned($borrowernumber, $itemnumber, $returndate, $privacy);
2091		2097
2092	Unconditionally marks an issue as being returned by	2098	Unconditionally marks an issue as being returned by
2093	moving the C<issues> row to C<old_issues> and	2099	moving the C<issues> row to C<old_issues> and
2094	setting C<returndate> to the current date, or	2100	setting C<returndate> to the current date.
2095	the last non-holiday date of the branccode specified in
2096	C<dropbox_branch> . Assumes you've already checked that
2097	it's safe to do this, i.e. last non-holiday > issuedate.
2098		2101
2099	if C<$returndate> is specified (in iso format), it is used as the date	2102	if C<$returndate> is specified (in iso format), it is used as the date
2100	of the return. It is ignored when a dropbox_branch is passed in.	2103	of the return.
2101		2104
2102	C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,	2105	C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
2103	the old_issue is immediately anonymised	2106	the old_issue is immediately anonymised
2104		2107
2105	Ideally, this function would be internal to C<C4::Circulation>,	2108	Ideally, this function would be internal to C<C4::Circulation>,
2106	not exported, but it is currently needed by one	2109	not exported, but it is currently used in misc/cronjobs/longoverdue.pl.
2107	routine in C<C4::Accounts>.
2108		2110
2109	=cut	2111	=cut
2110		2112
2111	sub MarkIssueReturned {	2113	sub MarkIssueReturned {
2112	my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;	2114	my ( $borrowernumber, $itemnumber, $returndate, $privacy ) = @_;
2113
2114		2115
2115	# Retrieve the issue	2116	# Retrieve the issue
2116	my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;	2117	my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return;
Lines 2126-2166 sub MarkIssueReturned { Link Here
2126	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."	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."
2127	unless Koha::Patrons->find( $anonymouspatron );	2128	unless Koha::Patrons->find( $anonymouspatron );
2128	}	2129	}
2129	my $database = Koha::Database->new();
2130	my $schema = $database->schema;
2131	my $dbh = C4::Context->dbh;
2132		2130
2133	my $query = 'UPDATE issues SET returndate=';	2131	my $schema = Koha::Database->schema;
2134	my @bind;
2135	if ($dropbox_branch) {
2136	my $calendar = Koha::Calendar->new( branchcode => $dropbox_branch );
2137	my $dropboxdate = $calendar->addDate( DateTime->now( time_zone => C4::Context->tz), -1 );
2138	$query .= ' ? ';
2139	push @bind, $dropboxdate->strftime('%Y-%m-%d %H:%M');
2140	} elsif ($returndate) {
2141	$query .= ' ? ';
2142	push @bind, $returndate;
2143	} else {
2144	$query .= ' now() ';
2145	}
2146	$query .= ' WHERE issue_id = ?';
2147	push @bind, $issue_id;
2148		2132
2149	# FIXME Improve the return value and handle it from callers	2133	# FIXME Improve the return value and handle it from callers
2150	$schema->txn_do(sub {	2134	$schema->txn_do(sub {
2151		2135
2152	# Update the returndate	2136	# Update the returndate value
2153	$dbh->do( $query, undef, @bind );	2137	if ( $returndate ) {
2154		2138	$issue->returndate( $returndate )->store->discard_changes; # update and refetch
2155	# We just updated the returndate, so we need to refetch $issue	2139	}
2156	$issue->discard_changes;	2140	else {
		2141	$issue->returndate( \'NOW()' )->store->discard_changes; # update and refetch
		2142	}
2157		2143
2158	# Create the old_issues entry	2144	# Create the old_issues entry
2159	my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;	2145	my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
2160		2146
2161	# anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber	2147	# anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
2162	if ( $privacy == 2) {	2148	if ( $privacy == 2) {
2163	$dbh->do(q\|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?\|, undef, $anonymouspatron, $old_checkout->issue_id);	2149	$old_checkout->borrowernumber($anonymouspatron)->store;
2164	}	2150	}
2165		2151
2166	# And finally delete the issue	2152	# And finally delete the issue
Lines 3700-3706 sub LostItem{ Link Here
3700	#warn " $issues->{'borrowernumber'} / $itemnumber ";	3686	#warn " $issues->{'borrowernumber'} / $itemnumber ";
3701	}	3687	}
3702		3688
3703	MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;	3689	MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
3704	}	3690	}
3705		3691
3706	#When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)	3692	#When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
Lines 3771-3777 sub ProcessOfflineReturn { Link Here
3771	MarkIssueReturned(	3757	MarkIssueReturned(
3772	$issue->{borrowernumber},	3758	$issue->{borrowernumber},
3773	$itemnumber,	3759	$itemnumber,
3774	undef,
3775	$operation->{timestamp},	3760	$operation->{timestamp},
3776	);	3761	);
3777	ModItem(	3762	ModItem(
Lines 3806-3812 sub ProcessOfflineIssue { Link Here
3806	MarkIssueReturned(	3791	MarkIssueReturned(
3807	$issue->{borrowernumber},	3792	$issue->{borrowernumber},
3808	$itemnumber,	3793	$itemnumber,
3809	undef,
3810	$operation->{timestamp},	3794	$operation->{timestamp},
3811	);	3795	);
3812	}	3796	}

Lines 61-88 subtest 'anonymous patron' => sub { Link Here

(-)a/t/db_dependent/Circulation/MarkIssueReturned.t (-6 / +5 lines)
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	-