Attachment #171533 for bug #35100

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




use List::MoreUtils qw( any );
use DateTime::Format::MySQL;
                  # debugging; so please don't remove this
use Try::Tiny qw( catch try );

use Koha::AuthorisedValues;
use Koha::DateUtils qw( dt_from_string );

sub ModItemTransfer {
    my ( $itemnumber, $frombranch, $tobranch, $trigger, $params ) = @_;

    my $dbh  = C4::Context->dbh;
    my $item = Koha::Items->find($itemnumber);

    # NOTE: This retains the existing hard coded behaviour by ignoring transfer limits
    # and always replacing any existing transfers. (In theory, calls to ModItemTransfer
    # will have been preceded by a check of branch transfer limits)
    my $to_library = Koha::Libraries->find($tobranch);
    my $transfer;
    try {
        $transfer = $item->request_transfer(
            {
                to            => $to_library,
                reason        => $trigger,
                ignore_limits => 1,
            }
        );
    } catch {
        if ( $_->isa('Koha::Exceptions::Item::Transfer::InQueue') ) {
            my $exception      = $_;
            my $found_transfer = $_->transfer;

            # If StockRotationAdvance, leave in place but ensure transit state is reset
            if ( $found_transfer->reason eq 'StockrotationAdvance' ) {
                $transfer = $item->request_transfer(
                    {
                        to            => $to_library,
                        reason        => $trigger,
                        ignore_limits => 1,
                        enqueue       => 1
                    }
                );

                # Ensure transit is reset
                $found_transfer->datesent(undef)->store;
            } else {
                $transfer = $item->request_transfer(
                    {
                        to            => $to_library,
                        reason        => $trigger,
                        ignore_limits => 1,
                        replace       => 1
                    }
                );
            }
        } else {
            $_->rethrow();
        }
    };

    # Immediately set the item to in transit if it is checked in
    if ( !$item->checkout ) {
        $item->holdingbranch($frombranch)->store(
            {
                log_action        => 0,
                skip_record_index => 1,    # avoid indexing duplication, let ->transit handle it
            }
        );
        $transfer->transit( { skip_record_index => $params->{skip_record_index} } );
    }

    return $transfer;

Lines 487-494 sub TransferCollection { Link Here

(-)a/C4/RotatingCollections.pm (-2 / +5 lines)
487	replace => 1	487	replace => 1
488	}	488	}
489	); # Replace transfer	489	); # Replace transfer
490	# NOTE: If we just replaced a StockRotationAdvance,	490	# FIXME: If we just replaced a StockRotationAdvance,
491	# it will get enqueued afresh on the next cron run	491	# it will get enqueued afresh on the next cron run.. but
		492	# that will also push the stage on too.. and what about if
		493	# we were at the first stage.. then there won't be a datearrived
		494	# to calculate against. See bug 35100
492	}	495	}
493	}	496	}
494	elsif ( $_->isa('Koha::Exceptions::Item::Transfer::Limit') ) {	497	elsif ( $_->isa('Koha::Exceptions::Item::Transfer::Limit') ) {

Lines 776-782 sub request_transfer { Link Here

(-)a/Koha/Item.pm (-1 / +1 lines)
776	if ( $request && !$params->{enqueue} && !$params->{replace} );	776	if ( $request && !$params->{enqueue} && !$params->{replace} );
777		777
778	$request->cancel( { reason => $params->{reason}, force => 1 } )	778	$request->cancel( { reason => $params->{reason}, force => 1 } )
779	if ( defined($request) && $params->{replace} && ( $request->reason ne 'StockrotationAdvance' ) );	779	if ( defined($request) && $params->{replace} );
780		780
781	my $transfer = Koha::Item::Transfer->new(	781	my $transfer = Koha::Item::Transfer->new(
782	{	782	{




};

subtest 'ModItemTransfer tests' => sub {
    plan tests => 14;

    $schema->storage->txn_begin;


    my $transfer3 = $transfers->next;
    is($transfer3->reason, 'Manual', "Reason set via ModItemTransfer");

    # Ensure StockrotationAdvance transfers are not cancelled
    my $stock_transfer = $transfer3->reason('StockrotationAdvance')->store();
    is(
        $stock_transfer->reason, 'StockrotationAdvance',
        'StockrotationAdvance transfer set'
    );
    ok(
        $stock_transfer->datesent,
        'StockrotationAdvance transfer is in transit'
    );

    ModItemTransfer( $item->itemnumber, $library1->{branchcode}, $library2->{branchcode}, 'Manual' );
    $transfers = Koha::Item::Transfers->search(
        { itemnumber => $item->itemnumber, },
        { order_by   => { '-desc' => 'branchtransfer_id' } }
    );

    my $replaced_transfer = $transfers->next;
    is(
        $replaced_transfer->reason, 'Manual',
        'StockrotationAdvance transfer "replaced" by manual transfer at top of queue'
    );
    $stock_transfer->discard_changes;
    is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer not cancelled" );
    is( $stock_transfer->datesent,      undef, "StockrotationAdvance transfer no longer in transit" );
    $transfers = $item->get_transfers;
    is( $transfers->count, 2, "There are now 2 live transfers in the queue" );

    $schema->storage->txn_rollback;
};





};

subtest 'request_transfer' => sub {
    plan tests => 13;
    $schema->storage->txn_begin;

    my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );

    is($transfers->count, 1, "There is only 1 live transfer in the queue");
    $replaced_transfer->datearrived(dt_from_string)->store();

    # Replace StockrotationAdvance transfer
    my $stock_transfer = $item->request_transfer( { to => $library1, reason => 'StockrotationAdvance' } );
    is(
        ref($stock_transfer), 'Koha::Item::Transfer',
        'Koha::Item->request_transfer added StockrotationAdvance transfer'
    );
    $replaced_transfer = $item->request_transfer( { to => $library2, reason => 'Manual', replace => 1 } );
    is(
        ref($replaced_transfer), 'Koha::Item::Transfer',
        'Koha::Item->request_transfer allowed when replace is set'
    );
    $stock_transfer->discard_changes;
    is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer left intact" );
    $transfers = $item->get_transfers;
    is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
    $replaced_transfer->datearrived(dt_from_string)->store();
    $stock_transfer->datearrived(dt_from_string)->store();

    # BranchTransferLimits
    t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
    t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
- 

Return to bug 35100

Lines 54-59 use C4::Log qw( logaction ); Link Here

(-)a/C4/Items.pm (-11 / +45 lines)
54	use List::MoreUtils qw( any );	54	use List::MoreUtils qw( any );
55	use DateTime::Format::MySQL;	55	use DateTime::Format::MySQL;
56	# debugging; so please don't remove this	56	# debugging; so please don't remove this
		57	use Try::Tiny qw( catch try );
57		58
58	use Koha::AuthorisedValues;	59	use Koha::AuthorisedValues;
59	use Koha::DateUtils qw( dt_from_string );	60	use Koha::DateUtils qw( dt_from_string );
Lines 359-389 The last optional parameter allows for passing skip_record_index through to the Link Here
359	sub ModItemTransfer {	360	sub ModItemTransfer {
360	my ( $itemnumber, $frombranch, $tobranch, $trigger, $params ) = @_;	361	my ( $itemnumber, $frombranch, $tobranch, $trigger, $params ) = @_;
361		362
362	my $dbh = C4::Context->dbh;	363	my $dbh = C4::Context->dbh;
363	my $item = Koha::Items->find( $itemnumber );	364	my $item = Koha::Items->find($itemnumber);
364		365
365	# NOTE: This retains the existing hard coded behaviour by ignoring transfer limits	366	# NOTE: This retains the existing hard coded behaviour by ignoring transfer limits
366	# and always replacing any existing transfers. (In theory, calls to ModItemTransfer	367	# and always replacing any existing transfers. (In theory, calls to ModItemTransfer
367	# will have been preceded by a check of branch transfer limits)	368	# will have been preceded by a check of branch transfer limits)
368	my $to_library = Koha::Libraries->find($tobranch);	369	my $to_library = Koha::Libraries->find($tobranch);
369	my $transfer = $item->request_transfer(	370	my $transfer;
370	{	371	try {
371	to => $to_library,	372	$transfer = $item->request_transfer(
372	reason => $trigger,	373	{
373	ignore_limits => 1,	374	to => $to_library,
374	replace => 1	375	reason => $trigger,
		376	ignore_limits => 1,
		377	}
		378	);
		379	} catch {
		380	if ( $_->isa('Koha::Exceptions::Item::Transfer::InQueue') ) {
		381	my $exception = $_;
		382	my $found_transfer = $_->transfer;
		383
		384	# If StockRotationAdvance, leave in place but ensure transit state is reset
		385	if ( $found_transfer->reason eq 'StockrotationAdvance' ) {
		386	$transfer = $item->request_transfer(
		387	{
		388	to => $to_library,
		389	reason => $trigger,
		390	ignore_limits => 1,
		391	enqueue => 1
		392	}
		393	);
		394
		395	# Ensure transit is reset
		396	$found_transfer->datesent(undef)->store;
		397	} else {
		398	$transfer = $item->request_transfer(
		399	{
		400	to => $to_library,
		401	reason => $trigger,
		402	ignore_limits => 1,
		403	replace => 1
		404	}
		405	);
		406	}
		407	} else {
		408	$_->rethrow();
375	}	409	}
376	);	410	};
377		411
378	# Immediately set the item to in transit if it is checked in	412	# Immediately set the item to in transit if it is checked in
379	if ( !$item->checkout ) {	413	if ( !$item->checkout ) {
380	$item->holdingbranch($frombranch)->store(	414	$item->holdingbranch($frombranch)->store(
381	{	415	{
382	log_action => 0,	416	log_action => 0,
383	skip_record_index => 1, # avoid indexing duplication, let ->transit handle it	417	skip_record_index => 1, # avoid indexing duplication, let ->transit handle it
384	}	418	}
385	);	419	);
386	$transfer->transit({ skip_record_index => $params->{skip_record_index} });	420	$transfer->transit( { skip_record_index => $params->{skip_record_index} } );
387	}	421	}
388		422
389	return $transfer;	423	return $transfer;

Lines 130-136 subtest 'General Add, Get and Del tests' => sub { Link Here

(-)a/t/db_dependent/Items.t (-1 / +29 lines)
130	};	130	};
131		131
132	subtest 'ModItemTransfer tests' => sub {	132	subtest 'ModItemTransfer tests' => sub {
133	plan tests => 8;	133	plan tests => 14;
134		134
135	$schema->storage->txn_begin;	135	$schema->storage->txn_begin;
136		136
Lines 189-194 subtest 'ModItemTransfer tests' => sub { Link Here
189	my $transfer3 = $transfers->next;	189	my $transfer3 = $transfers->next;
190	is($transfer3->reason, 'Manual', "Reason set via ModItemTransfer");	190	is($transfer3->reason, 'Manual', "Reason set via ModItemTransfer");
191		191
		192	# Ensure StockrotationAdvance transfers are not cancelled
		193	my $stock_transfer = $transfer3->reason('StockrotationAdvance')->store();
		194	is(
		195	$stock_transfer->reason, 'StockrotationAdvance',
		196	'StockrotationAdvance transfer set'
		197	);
		198	ok(
		199	$stock_transfer->datesent,
		200	'StockrotationAdvance transfer is in transit'
		201	);
		202
		203	ModItemTransfer( $item->itemnumber, $library1->{branchcode}, $library2->{branchcode}, 'Manual' );
		204	$transfers = Koha::Item::Transfers->search(
		205	{ itemnumber => $item->itemnumber, },
		206	{ order_by => { '-desc' => 'branchtransfer_id' } }
		207	);
		208
		209	my $replaced_transfer = $transfers->next;
		210	is(
		211	$replaced_transfer->reason, 'Manual',
		212	'StockrotationAdvance transfer "replaced" by manual transfer at top of queue'
		213	);
		214	$stock_transfer->discard_changes;
		215	is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer not cancelled" );
		216	is( $stock_transfer->datesent, undef, "StockrotationAdvance transfer no longer in transit" );
		217	$transfers = $item->get_transfers;
		218	is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
		219
192	$schema->storage->txn_rollback;	220	$schema->storage->txn_rollback;
193	};	221	};
194		222

Lines 846-852 subtest 'pickup_locations() tests' => sub { Link Here

(-)a/t/db_dependent/Koha/Item.t (-20 / +1 lines)
846	};	846	};
847		847
848	subtest 'request_transfer' => sub {	848	subtest 'request_transfer' => sub {
849	plan tests => 17;	849	plan tests => 13;
850	$schema->storage->txn_begin;	850	$schema->storage->txn_begin;
851		851
852	my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );	852	my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
Lines 906-929 subtest 'request_transfer' => sub { Link Here
906	is($transfers->count, 1, "There is only 1 live transfer in the queue");	906	is($transfers->count, 1, "There is only 1 live transfer in the queue");
907	$replaced_transfer->datearrived(dt_from_string)->store();	907	$replaced_transfer->datearrived(dt_from_string)->store();
908		908
909	# Replace StockrotationAdvance transfer
910	my $stock_transfer = $item->request_transfer( { to => $library1, reason => 'StockrotationAdvance' } );
911	is(
912	ref($stock_transfer), 'Koha::Item::Transfer',
913	'Koha::Item->request_transfer added StockrotationAdvance transfer'
914	);
915	$replaced_transfer = $item->request_transfer( { to => $library2, reason => 'Manual', replace => 1 } );
916	is(
917	ref($replaced_transfer), 'Koha::Item::Transfer',
918	'Koha::Item->request_transfer allowed when replace is set'
919	);
920	$stock_transfer->discard_changes;
921	is( $stock_transfer->datecancelled, undef, "StockrotationAdvance transfer left intact" );
922	$transfers = $item->get_transfers;
923	is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
924	$replaced_transfer->datearrived(dt_from_string)->store();
925	$stock_transfer->datearrived(dt_from_string)->store();
926
927	# BranchTransferLimits	909	# BranchTransferLimits
928	t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);	910	t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
929	t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');	911	t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
930	-