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

(-)a/C4/Circulation.pm (-22 / +9 lines)
Lines 1935-1955 sub AddReturn { Link Here
1935
1935
1936
        if ($borrowernumber) {
1936
        if ($borrowernumber) {
1937
            eval {
1937
            eval {
1938
                my $issue_id = MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1938
                MarkIssueReturned( $borrowernumber, $item->{'itemnumber'},
1939
                    $circControlBranch, $return_date, $borrower->{'privacy'} );
1939
                    $circControlBranch, $return_date, $borrower->{privacy} );
1940
                $issue->{issue_id} = $issue_id;
1941
            };
1940
            };
1942
            unless ( $@ ) {
1941
            unless ( $@ ) {
1943
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
1942
                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
1944
                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
1943
                    _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
1945
                }
1944
                }
1946
            } else {
1945
            } else {
1947
                $messages->{'Wrongbranch'} = {
1946
                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 );
1948
                    Wrongbranch => $branch,
1947
1949
                    Rightbranch => $message
1948
                return ( 0, { WasReturned => 0, DataCorrupted => 1 }, $issue, $borrower );
1950
                };
1951
                carp $@;
1952
                return ( 0, { WasReturned => 0 }, $issue, $borrower );
1953
            }
1949
            }
1954
1950
1955
            # FIXME is the "= 1" right?  This could be the borrower hash.
1951
            # FIXME is the "= 1" right?  This could be the borrower hash.
Lines 2174-2196 sub MarkIssueReturned { Link Here
2174
    # FIXME Improve the return value and handle it from callers
2170
    # FIXME Improve the return value and handle it from callers
2175
    $schema->txn_do(sub {
2171
    $schema->txn_do(sub {
2176
2172
2173
        # Update the returndate
2177
        $dbh->do( $query, undef, @bind );
2174
        $dbh->do( $query, undef, @bind );
2178
2175
2176
        # Retrieve the issue
2179
        my $issue = Koha::Checkouts->find( $issue_id ); # FIXME should be fetched earlier
2177
        my $issue = Koha::Checkouts->find( $issue_id ); # FIXME should be fetched earlier
2180
2178
2181
        # Create the old_issues entry
2179
        # Create the old_issues entry
2182
        my $old_checkout_data = $issue->unblessed;
2180
        my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
2183
2184
        if ( Koha::Old::Checkouts->find( $issue_id ) ) {
2185
            my $new_issue_id = ( Koha::Old::Checkouts->search(
2186
                {},
2187
                { columns => [ { max_issue_id => { max => 'issue_id' } } ] }
2188
            )->get_column('max_issue_id') )[0];
2189
            $new_issue_id++;
2190
            $issue_id = $new_issue_id;
2191
        }
2192
        $old_checkout_data->{issue_id} = $issue_id;
2193
        my $old_checkout = Koha::Old::Checkout->new($old_checkout_data)->store;
2194
2181
2195
        # Update the fines
2182
        # Update the fines
2196
        $dbh->do(q|UPDATE accountlines SET issue_id = ? WHERE issue_id = ?|, undef, $old_checkout->issue_id, $issue->issue_id);
2183
        $dbh->do(q|UPDATE accountlines SET issue_id = ? WHERE issue_id = ?|, undef, $old_checkout->issue_id, $issue->issue_id);
Lines 2200-2206 sub MarkIssueReturned { Link Here
2200
            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $old_checkout->issue_id);
2187
            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $old_checkout->issue_id);
2201
        }
2188
        }
2202
2189
2203
        # Delete the issue
2190
        # And finally delete the issue
2204
        $issue->delete;
2191
        $issue->delete;
2205
2192
2206
        ModItem( { 'onloan' => undef }, undef, $itemnumber );
2193
        ModItem( { 'onloan' => undef }, undef, $itemnumber );
(-)a/circ/returns.pl (+3 lines)
Lines 550-555 foreach my $code ( keys %$messages ) { Link Here
550
    elsif ( $code eq 'NotForLoanStatusUpdated' ) {
550
    elsif ( $code eq 'NotForLoanStatusUpdated' ) {
551
        $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
551
        $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
552
    }
552
    }
553
    elsif ( $code eq 'DataCorrupted' ) {
554
        $err{data_corrupted} = 1;
555
    }
553
    else {
556
    else {
554
        die "Unknown error code $code";    # note we need all the (empty) elsif's above, or we die.
557
        die "Unknown error code $code";    # note we need all the (empty) elsif's above, or we die.
555
        # This forces the issue of staying in sync w/ Circulation.pm
558
        # This forces the issue of staying in sync w/ Circulation.pm
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt (+5 lines)
Lines 237-242 $(document).ready(function () { Link Here
237
        <p>This item must be checked in at following library: <strong>[% Branches.GetName( rightbranch ) %]</strong></p>
237
        <p>This item must be checked in at following library: <strong>[% Branches.GetName( rightbranch ) %]</strong></p>
238
    </div>
238
    </div>
239
[% END %]
239
[% END %]
240
240
<!-- case of a mistake in transfer loop -->
241
<!-- case of a mistake in transfer loop -->
241
[% IF ( WrongTransfer ) %]
242
[% IF ( WrongTransfer ) %]
242
    <div id="return2" class="dialog message">
243
    <div id="return2" class="dialog message">
Lines 640-645 $(document).ready(function () { Link Here
640
            [% IF ( errmsgloo.foreverdebarred ) %]
641
            [% IF ( errmsgloo.foreverdebarred ) %]
641
                <p class="problem"><b>Reminder: </b>Patron has an indefinite restriction.</p>
642
                <p class="problem"><b>Reminder: </b>Patron has an indefinite restriction.</p>
642
            [% END %]
643
            [% END %]
644
645
            [% IF errmsgloo.data_corrupted %]
646
                <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>
647
            [% END %]
643
        [% END %]
648
        [% END %]
644
    </div>
649
    </div>
645
[% END %]
650
[% END %]
(-)a/t/db_dependent/Circulation/Returns.t (-12 / +22 lines)
Lines 275-281 subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { Link Here
275
};
275
};
276
276
277
subtest 'Handle ids duplication' => sub {
277
subtest 'Handle ids duplication' => sub {
278
    plan tests => 4;
278
    plan tests => 6;
279
279
280
    t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
280
    t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
281
    t::lib::Mocks::mock_preference( 'CalculateFinesOnReturn', 1 );
281
    t::lib::Mocks::mock_preference( 'CalculateFinesOnReturn', 1 );
Lines 300-320 subtest 'Handle ids duplication' => sub { Link Here
300
    $patron = Koha::Patrons->find( $patron->{borrowernumber} );
300
    $patron = Koha::Patrons->find( $patron->{borrowernumber} );
301
301
302
    my $original_checkout = AddIssue( $patron->unblessed, $item->{barcode}, dt_from_string->subtract( days => 50 ) );
302
    my $original_checkout = AddIssue( $patron->unblessed, $item->{barcode}, dt_from_string->subtract( days => 50 ) );
303
    $builder->build({ source => 'OldIssue', value => { issue_id => $original_checkout->issue_id } });
304
    my $old_checkout = Koha::Old::Checkouts->find( $original_checkout->issue_id );
305
303
306
    AddRenewal( $patron->borrowernumber, $item->{itemnumber} );
304
    my $issue_id = $original_checkout->issue_id;
305
    # Create an existing entry in old_issue
306
    $builder->build({ source => 'OldIssue', value => { issue_id => $issue_id } });
307
308
    my $old_checkout = Koha::Old::Checkouts->find( $issue_id );
307
309
308
    my ($doreturn, $messages, $new_checkout, $borrower) = AddReturn( $item->{barcode}, undef, undef, undef, dt_from_string );
310
    my ($doreturn, $messages, $new_checkout, $borrower);
311
    warning_like {
312
        ( $doreturn, $messages, $new_checkout, $borrower ) =
313
          AddReturn( $item->{barcode}, undef, undef, undef, dt_from_string );
314
    }
315
    [
316
        qr{.*DBD::mysql::st execute failed: Duplicate entry.*},
317
        { carped => qr{The checkin for the following issue failed.*DBIx::Class::Storage::DBI::_dbh_execute.*} }
318
    ],
319
    'DBD should have raised an error about dup primary key';
309
320
310
    my $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $original_checkout->issue_id });
321
    is( $doreturn, 0, 'Return should not have been done' );
311
    is( $account_lines->count, 0, 'No account lines should exist on old issue_id' );
322
    is( $messages->{WasReturned}, 0, 'messages should have the WasReturned flag set to 0' );
323
    is( $messages->{DataCorrupted}, 1, 'messages should have the DataCorrupted flag set to 1' );
312
324
313
    $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $new_checkout->{issue_id} });
325
    my $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $issue_id });
314
    is( $account_lines->count, 2, 'Two account lines should exist on new issue_id' );
326
    is( $account_lines->count, 0, 'No account lines should exist for this issue_id, patron should not have been charged' );
315
327
316
    isnt( $original_checkout->issue_id, $new_checkout->{issue_id}, 'AddReturn should return the issue with the new issue_id' );
328
    is( Koha::Checkouts->find( $issue_id )->issue_id, $issue_id, 'The issues entry should not have been removed' );
317
    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' );
318
};
329
};
319
330
320
1;
331
1;
321
- 

Return to bug 18966