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

(-)a/C4/Circulation.pm (-24 / +18 lines)
Lines 53-58 use Koha::Patron::Debarments; Link Here
53
use Koha::Database;
53
use Koha::Database;
54
use Koha::Libraries;
54
use Koha::Libraries;
55
use Koha::Holds;
55
use Koha::Holds;
56
use Koha::Issues;
57
use Koha::OldIssues;
56
use Carp;
58
use Carp;
57
use List::MoreUtils qw( uniq );
59
use List::MoreUtils qw( uniq );
58
use Scalar::Util qw( looks_like_number );
60
use Scalar::Util qw( looks_like_number );
Lines 1920-1940 sub AddReturn { Link Here
1920
        }
1922
        }
1921
1923
1922
        if ($borrowernumber) {
1924
        if ($borrowernumber) {
1923
            if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
1924
                _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
1925
            }
1926
1927
            eval {
1925
            eval {
1928
                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1926
                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1929
                    $circControlBranch, $return_date, $borrower->{'privacy'} );
1927
                    $circControlBranch, $return_date, $borrower->{'privacy'} );
1930
            };
1928
            };
1931
            if ( $@ ) {
1929
            unless ( $@ ) {
1932
                $messages->{'Wrongbranch'} = {
1930
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
1933
                    Wrongbranch => $branch,
1931
                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
1934
                    Rightbranch => $message
1932
                }
1935
                };
1933
            } else {
1936
                carp $@;
1934
                carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue );
1937
                return ( 0, { WasReturned => 0 }, $issue, $borrower );
1935
1936
                return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $borrower );
1938
            }
1937
            }
1939
1938
1940
            # FIXME is the "= 1" right?  This could be the borrower hash.
1939
            # FIXME is the "= 1" right?  This could be the borrower hash.
Lines 2147-2167 sub MarkIssueReturned { Link Here
2147
2146
2148
    # FIXME Improve the return value and handle it from callers
2147
    # FIXME Improve the return value and handle it from callers
2149
    $schema->txn_do(sub {
2148
    $schema->txn_do(sub {
2149
2150
        # Update the returndate
2150
        $dbh->do( $query, undef, @bind );
2151
        $dbh->do( $query, undef, @bind );
2151
2152
2152
        my $id_already_exists = $dbh->selectrow_array(
2153
        # Retrieve the issue
2153
            q|SELECT COUNT(*) FROM old_issues WHERE issue_id = ?|,
2154
        my $issue = Koha::Issues->find( $issue_id ); # FIXME should be fetched earlier
2154
            undef, $issue_id
2155
        );
2156
2155
2157
        if ( $id_already_exists ) {
2156
        # Create the old_issues entry
2158
            my $new_issue_id = $dbh->selectrow_array(q|SELECT MAX(issue_id)+1 FROM old_issues|);
2157
        my $old_checkout = Koha::OldIssue->new($issue->unblessed)->store;
2159
            $dbh->do(
2160
                q|UPDATE issues SET issue_id = ? WHERE issue_id = ?|,
2161
                undef, $new_issue_id, $issue_id
2162
            );
2163
            $issue_id = $new_issue_id;
2164
        }
2165
2158
2166
        $dbh->do(q|INSERT INTO old_issues SELECT * FROM issues WHERE issue_id = ?|, undef, $issue_id);
2159
        $dbh->do(q|INSERT INTO old_issues SELECT * FROM issues WHERE issue_id = ?|, undef, $issue_id);
2167
2160
Lines 2170-2176 sub MarkIssueReturned { Link Here
2170
            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $issue_id);
2163
            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $issue_id);
2171
        }
2164
        }
2172
2165
2173
        $dbh->do(q|DELETE FROM issues WHERE issue_id = ?|, undef, $issue_id);
2166
        # And finally delete the issue
2167
        $issue->delete;
2174
2168
2175
        ModItem( { 'onloan' => undef }, undef, $itemnumber );
2169
        ModItem( { 'onloan' => undef }, undef, $itemnumber );
2176
2170
(-)a/circ/returns.pl (+3 lines)
Lines 546-551 foreach my $code ( keys %$messages ) { Link Here
546
    elsif ( $code eq 'NotForLoanStatusUpdated' ) {
546
    elsif ( $code eq 'NotForLoanStatusUpdated' ) {
547
        $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
547
        $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
548
    }
548
    }
549
    elsif ( $code eq 'DataCorrupted' ) {
550
        $err{data_corrupted} = 1;
551
    }
549
    else {
552
    else {
550
        die "Unknown error code $code";    # note we need all the (empty) elsif's above, or we die.
553
        die "Unknown error code $code";    # note we need all the (empty) elsif's above, or we die.
551
        # This forces the issue of staying in sync w/ Circulation.pm
554
        # This forces the issue of staying in sync w/ Circulation.pm
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt (-1 / +5 lines)
Lines 205-210 $(document).ready(function () { Link Here
205
        <p>This item must be checked in at following library: <strong>[% Branches.GetName( rightbranch ) %]</strong></p>
205
        <p>This item must be checked in at following library: <strong>[% Branches.GetName( rightbranch ) %]</strong></p>
206
    </div>
206
    </div>
207
[% END %]
207
[% END %]
208
208
<!-- case of a mistake in transfer loop -->
209
<!-- case of a mistake in transfer loop -->
209
[% IF ( WrongTransfer ) %]
210
[% IF ( WrongTransfer ) %]
210
    <div id="return2" class="dialog message">
211
    <div id="return2" class="dialog message">
Lines 514-520 $(document).ready(function () { Link Here
514
                    [% END %]
515
                    [% END %]
515
516
516
            [% END %]
517
            [% END %]
517
        [% ELSE %]
518
519
            [% IF errmsgloo.data_corrupted %]
520
                <p class="problem">The item has not been checked in due to a configuration issue in your system. You must ask an administrator to take a look at the <a href="/cgi-bin/koha/about.pl#sysinfo">about page</a> and search for the "data problems" section</p>
521
            [% END %]
518
        [% END %]
522
        [% END %]
519
    </div>
523
    </div>
520
524
(-)a/t/db_dependent/Circulation/Returns.t (-7 / +36 lines)
Lines 1-6 Link Here
1
use Modern::Perl;
1
use Modern::Perl;
2
2
3
use Test::More tests => 3;
3
use Test::More tests => 3;
4
use Test::Warn;
4
5
5
use t::lib::Mocks;
6
use t::lib::Mocks;
6
use C4::Biblio;
7
use C4::Biblio;
Lines 9-14 use C4::Items; Link Here
9
use C4::Members;
10
use C4::Members;
10
use Koha::Database;
11
use Koha::Database;
11
use Koha::DateUtils;
12
use Koha::DateUtils;
13
use Koha::Issues;
12
use Koha::OldIssues;
14
use Koha::OldIssues;
13
15
14
use t::lib::TestBuilder;
16
use t::lib::TestBuilder;
Lines 60-66 sub Mock_userenv { Link Here
60
}
62
}
61
63
62
subtest 'Handle ids duplication' => sub {
64
subtest 'Handle ids duplication' => sub {
63
    plan tests => 1;
65
    plan tests => 6;
66
67
    t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
68
    t::lib::Mocks::mock_preference( 'CalculateFinesOnReturn', 1 );
69
    t::lib::Mocks::mock_preference( 'finesMode', 'production' );
70
    my $dbh = C4::Context->dbh;
71
    $dbh->do(q|UPDATE issuingrules SET chargeperiod = 1, fine = 1, firstremind = 1|);
64
72
65
    my $biblio = $builder->build( { source => 'Biblio' } );
73
    my $biblio = $builder->build( { source => 'Biblio' } );
66
    my $item = $builder->build(
74
    my $item = $builder->build(
Lines 76-88 subtest 'Handle ids duplication' => sub { Link Here
76
        }
84
        }
77
    );
85
    );
78
    my $patron = $builder->build({source => 'Borrower'});
86
    my $patron = $builder->build({source => 'Borrower'});
87
    $patron = Koha::Patrons->find( $patron->{borrowernumber} );
88
89
    my $original_checkout = AddIssue( $patron->unblessed, $item->{barcode}, dt_from_string->subtract( days => 50 ) );
90
91
    my $issue_id = $original_checkout->issue_id;
92
    # Create an existing entry in old_issue
93
    $builder->build({ source => 'OldIssue', value => { issue_id => $issue_id } });
94
95
    my $old_checkout = Koha::OldIssues->find( $issue_id );
96
97
    my ($doreturn, $messages, $new_checkout, $borrower);
98
    warning_like {
99
        ( $doreturn, $messages, $new_checkout, $borrower ) =
100
          AddReturn( $item->{barcode}, undef, undef, undef, dt_from_string );
101
    }
102
    [
103
        qr{.*DBD::mysql::st execute failed: Duplicate entry.*},
104
        { carped => qr{The checkin for the following issue failed.*DBIx::Class::Storage::DBI::_dbh_execute.*} }
105
    ],
106
    'DBD should have raised an error about dup primary key';
107
108
    is( $doreturn, 0, 'Return should not have been done' );
109
    is( $messages->{WasReturned}, 0, 'messages should have the WasReturned flag set to 0' );
110
    is( $messages->{DataCorrupted}, 1, 'messages should have the DataCorrupted flag set to 1' );
79
111
80
    my $checkout = AddIssue( $patron, $item->{barcode} );
112
    my $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $issue_id });
81
    $builder->build({ source => 'OldIssue', value => { issue_id => $checkout->issue_id } });
113
    is( $account_lines->count, 0, 'No account lines should exist for this issue_id, patron should not have been charged' );
82
114
83
    my @a = AddReturn( $item->{barcode} );
115
    is( Koha::Issues->find( $issue_id )->issue_id, $issue_id, 'The issues entry should not have been removed' );
84
    my $old_checkout = Koha::OldIssues->find( $checkout->issue_id );
85
    isnt( $old_checkout->itemnumber, $item->{itemnumber}, 'If an item is checked-in, it should be moved to old_issues even if the issue_id already existed in the table' );
86
};
116
};
87
117
88
1;
118
1;
89
- 

Return to bug 18966