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

(-)a/Koha/Booking.pm (-19 / +50 lines)
Lines 173-197 sub store { Link Here
173
                value     => $self->biblio_id,
173
                value     => $self->biblio_id,
174
            ) unless ( $self->biblio );
174
            ) unless ( $self->biblio );
175
175
176
            # Throw exception for item level booking clash
176
            # Skip clash detection when transitioning to a final
177
            Koha::Exceptions::Booking::Clash->throw()
177
            # status.  During checkout C4::Circulation sets status
178
                if $self->item_id && !$self->item->check_booking(
178
            # to 'completed' and calls ->store; the clash checks
179
                {
179
            # are irrelevant at that point and can produce false
180
                    start_date => $self->start_date,
180
            # positives.
181
                    end_date   => $self->end_date,
181
            unless ( $self->_is_final_status_transition ) {
182
                    booking_id => $self->in_storage ? $self->booking_id : undef
182
183
                }
183
                # Throw exception for item level booking clash
184
                );
184
                Koha::Exceptions::Booking::Clash->throw()
185
185
                    if $self->item_id && !$self->item->check_booking(
186
            # Throw exception for biblio level booking clash
186
                    {
187
            Koha::Exceptions::Booking::Clash->throw()
187
                        start_date => $self->start_date,
188
                if !$self->biblio->check_booking(
188
                        end_date   => $self->end_date,
189
                {
189
                        booking_id => $self->in_storage ? $self->booking_id : undef
190
                    start_date => $self->start_date,
190
                    }
191
                    end_date   => $self->end_date,
191
                    );
192
                    booking_id => $self->in_storage ? $self->booking_id : undef
192
193
                }
193
                # Throw exception for biblio level booking clash
194
                );
194
                Koha::Exceptions::Booking::Clash->throw()
195
                    if !$self->biblio->check_booking(
196
                    {
197
                        start_date => $self->start_date,
198
                        end_date   => $self->end_date,
199
                        booking_id => $self->in_storage ? $self->booking_id : undef
200
                    }
201
                    );
202
            }
195
203
196
            # FIXME: We should be able to combine the above two functions into one
204
            # FIXME: We should be able to combine the above two functions into one
197
205
Lines 336-341 sub to_api_mapping { Link Here
336
344
337
=head2 Internal methods
345
=head2 Internal methods
338
346
347
=head3 _is_final_status_transition
348
349
    my $bool = $self->_is_final_status_transition;
350
351
Returns true when the booking is being transitioned to a final status
352
(cancelled or completed).  Used to skip clash detection that is only
353
meaningful for active bookings.
354
355
=cut
356
357
sub _is_final_status_transition {
358
    my ($self) = @_;
359
360
    return 0 unless $self->in_storage;
361
362
    my $updated_columns = { $self->_result->get_dirty_columns };
363
    my $new_status      = $updated_columns->{status};
364
365
    return 0 unless $new_status;
366
    return 1 if any { $_ eq $new_status } qw( cancelled completed );
367
    return 0;
368
}
369
339
=head3 _select_optimal_item
370
=head3 _select_optimal_item
340
371
341
    my $item_id = $self->_select_optimal_item($available_items);
372
    my $item_id = $self->_select_optimal_item($available_items);
(-)a/t/db_dependent/Koha/Booking.t (-2 / +48 lines)
Lines 20-26 Link Here
20
use Modern::Perl;
20
use Modern::Perl;
21
use utf8;
21
use utf8;
22
22
23
use Test::More tests => 6;
23
use Test::More tests => 7;
24
use Test::NoWarnings;
24
use Test::NoWarnings;
25
25
26
use Test::Warn;
26
use Test::Warn;
Lines 1180-1182 subtest 'Integration test: Full optimal selection workflow' => sub { Link Here
1180
1180
1181
    $schema->storage->txn_rollback;
1181
    $schema->storage->txn_rollback;
1182
};
1182
};
1183
- 
1183
1184
subtest 'store() skips clash detection on terminal status transition' => sub {
1185
    plan tests => 3;
1186
    $schema->storage->txn_begin;
1187
1188
    my $patron = $builder->build_object( { class => "Koha::Patrons" } );
1189
    t::lib::Mocks::mock_userenv( { patron => $patron } );
1190
1191
    my $biblio            = $builder->build_sample_biblio();
1192
    my $bookable_item     = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 1 } );
1193
    my $non_bookable_item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 0 } );
1194
1195
    my $start = dt_from_string()->truncate( to => 'day' );
1196
    my $end   = $start->clone->add( days => 7 );
1197
1198
    # Create booking first, before any checkouts exist so
1199
    # store() succeeds without interference from Bug 41886.
1200
    my $booking = Koha::Booking->new(
1201
        {
1202
            patron_id         => $patron->borrowernumber,
1203
            biblio_id         => $biblio->biblionumber,
1204
            item_id           => $bookable_item->itemnumber,
1205
            pickup_library_id => $bookable_item->homebranch,
1206
            start_date        => $start,
1207
            end_date          => $end,
1208
        }
1209
    )->store();
1210
    ok( $booking->in_storage, 'Booking on bookable item stored OK' );
1211
1212
    # Now check out the non-bookable sibling item. This
1213
    # checkout inflates the unavailable count in
1214
    # Biblio::check_booking (see Bug 41886) and causes a
1215
    # false clash when there is only one bookable item.
1216
    C4::Circulation::AddIssue( $patron, $non_bookable_item->barcode );
1217
1218
    # Without the fix, transitioning to 'completed' runs clash
1219
    # detection which sees the non-bookable checkout and throws
1220
    # Koha::Exceptions::Booking::Clash → 500 error.
1221
    lives_ok { $booking->status('completed')->store() }
1222
    'Transition to completed skips clash detection';
1223
1224
    # Cancellation from completed is also a terminal transition
1225
    lives_ok { $booking->status('cancelled')->store() }
1226
    'Transition to cancelled skips clash detection';
1227
1228
    $schema->storage->txn_rollback;
1229
};

Return to bug 41887