Attachment #83542 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

        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->update({ returndate => $returndate })->discard_changes; # update and refetch
        }
        else {
            $issue->update({ returndate => \'NOW()' })->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->update({ borrowernumber => $anonymouspatron });
        }

        # 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 (-38 / +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-2115 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		2115
2114		2116
2115	# Retrieve the issue	2117	# Retrieve the issue
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."	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."
2127	unless Koha::Patrons->find( $anonymouspatron );	2129	unless Koha::Patrons->find( $anonymouspatron );
2128	}	2130	}
2129	my $database = Koha::Database->new();
2130	my $schema = $database->schema;
2131	my $dbh = C4::Context->dbh;
2132		2131
2133	my $query = 'UPDATE issues SET returndate=';	2132	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		2133
2149	# FIXME Improve the return value and handle it from callers	2134	# FIXME Improve the return value and handle it from callers
2150	$schema->txn_do(sub {	2135	$schema->txn_do(sub {
2151		2136
2152	# Update the returndate	2137	# Update the returndate value
2153	$dbh->do( $query, undef, @bind );	2138	if ( $returndate ) {
2154		2139	$issue->update({ returndate => $returndate })->discard_changes; # update and refetch
2155	# We just updated the returndate, so we need to refetch $issue	2140	}
2156	$issue->discard_changes;	2141	else {
		2142	$issue->update({ returndate => \'NOW()' })->discard_changes; # update and refetch
		2143	}
2157		2144
2158	# Create the old_issues entry	2145	# Create the old_issues entry
2159	my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;	2146	my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
2160		2147
2161	# 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
2162	if ( $privacy == 2) {	2149	if ( $privacy == 2) {
2163	$dbh->do(q\|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?\|, undef, $anonymouspatron, $old_checkout->issue_id);	2150	$old_checkout->update({ borrowernumber => $anonymouspatron });
2164	}	2151	}
2165		2152
2166	# And finally delete the issue	2153	# And finally delete the issue
Lines 3700-3706 sub LostItem{ Link Here
3700	#warn " $issues->{'borrowernumber'} / $itemnumber ";	3687	#warn " $issues->{'borrowernumber'} / $itemnumber ";
3701	}	3688	}
3702		3689
3703	MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned;	3690	MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned;
3704	}	3691	}
3705		3692
3706	#When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)	3693	#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(	3758	MarkIssueReturned(
3772	$issue->{borrowernumber},	3759	$issue->{borrowernumber},
3773	$itemnumber,	3760	$itemnumber,
3774	undef,
3775	$operation->{timestamp},	3761	$operation->{timestamp},
3776	);	3762	);
3777	ModItem(	3763	ModItem(
Lines 3806-3812 sub ProcessOfflineIssue { Link Here
3806	MarkIssueReturned(	3792	MarkIssueReturned(
3807	$issue->{borrowernumber},	3793	$issue->{borrowernumber},
3808	$itemnumber,	3794	$itemnumber,
3809	undef,
3810	$operation->{timestamp},	3795	$operation->{timestamp},
3811	);	3796	);
3812	}	3797	}

Lines 62-89 subtest 'anonymous patron' => sub { Link Here

(-)a/t/db_dependent/Circulation/MarkIssueReturned.t (-6 / +5 lines)
62	# The next call will raise an error, because data are not correctly set	62	# The next call will raise an error, because data are not correctly set
63	t::lib::Mocks::mock_preference('AnonymousPatron', '');	63	t::lib::Mocks::mock_preference('AnonymousPatron', '');
64	my $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );	64	my $issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
65	eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, 'dropbox_branch', 'returndate', 2 ) };	65	eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
66	like ( $@, qr<Fatal error: the patron \(\d+\) .* AnonymousPatron>, 'AnonymousPatron is not set - Fatal error on anonymization' );	66	like ( $@, qr<Fatal error: the patron \(\d+\) .* AnonymousPatron>, 'AnonymousPatron is not set - Fatal error on anonymization' );
67	Koha::Checkouts->find( $issue->issue_id )->delete;	67	Koha::Checkouts->find( $issue->issue_id )->delete;
68		68
69	my $anonymous_borrowernumber = Koha::Patron->new({categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode} })->store->borrowernumber;	69	my $anonymous_borrowernumber = Koha::Patron->new({categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode} })->store->borrowernumber;
70	t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber);	70	t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous_borrowernumber);
71	$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );	71	$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
72	eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, 'dropbox_branch', 'returndate', 2 ) };	72	eval { C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 2 ) };
73	is ( $@, q\|\|, 'AnonymousPatron is set correctly - no error expected');	73	is ( $@, q\|\|, 'AnonymousPatron is set correctly - no error expected');
74	};	74	};
75		75
76	my ( $issue_id, $issue );	76	my ( $issue_id, $issue );
77	# The next call will return undef for invalid item number	77	# The next call will return undef for invalid item number
78	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, 'invalid_itemnumber', 'dropbox_branch', 'returndate', 0 ) };	78	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, 'invalid_itemnumber', undef, 0 ) };
79	is( $@, '', 'No die triggered by invalid itemnumber' );	79	is( $@, '', 'No die triggered by invalid itemnumber' );
80	is( $issue_id, undef, 'No issue_id returned' );	80	is( $issue_id, undef, 'No issue_id returned' );
81		81
82	# In the next call we return the item and try it another time	82	# In the next call we return the item and try it another time
83	$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );	83	$issue = C4::Circulation::AddIssue( $patron, $item->{barcode} );
84	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, undef, 0 ) };	84	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
85	is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" );	85	is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" );
86	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, undef, 0 ) };	86	eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->{borrowernumber}, $item->{itemnumber}, undef, 0 ) };
87	is( $@, '', 'No crash on returning item twice' );	87	is( $@, '', 'No crash on returning item twice' );
88	is( $issue_id, undef, 'Cannot return an item twice' );	88	is( $issue_id, undef, 'Cannot return an item twice' );
89		89
90	-