Bugzilla – Attachment 172778 Details for
Bug 28294
C4::Circulation::updateWrongTransfer should be moved into Koha::
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28294: Remove updateWrongTransfer
Bug-28294-Remove-updateWrongTransfer.patch (text/plain), 14.75 KB, created by
Martin Renvoize (ashimema)
on 2024-10-15 14:29:18 UTC
(
hide
)
Description:
Bug 28294: Remove updateWrongTransfer
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2024-10-15 14:29:18 UTC
Size:
14.75 KB
patch
obsolete
>From ff23fcb3f0af24ad1aa6b4e47a4e9562f56c2093 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 17 May 2021 17:14:16 +0100 >Subject: [PATCH] Bug 28294: Remove updateWrongTransfer > >This patch removes the last remaining use of updateWrongTransfer and the >method itself. > >We replace it with a call to request_transfer that passes the 'replace' >reason of 'WrongTransfer' through to Koha::Item::Transfer->cancel. > >Test plan >1/ git grep updateWrongTransfer to confirm there are no more uses of the >method. >2/ Confirm an incorrect transfer still behaves as expected at >circulation returns. >--- > C4/Circulation.pm | 31 ---------------- > C4/Items.pm | 2 +- > C4/RotatingCollections.pm | 2 +- > Koha/Item.pm | 6 ++-- > Koha/StockRotationItem.pm | 2 +- > circ/returns.pl | 7 ++-- > t/db_dependent/Circulation.t | 44 ++--------------------- > t/db_dependent/Koha/Item.t | 68 +++++++++++++++++++++--------------- > 8 files changed, 52 insertions(+), 110 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index bd8ecfc58fa..9188620711f 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -107,7 +107,6 @@ BEGIN { > transferbook > TooMany > GetTransfersFromTo >- updateWrongTransfer > CalcDateDue > CheckValidBarcode > IsBranchTransferAllowed >@@ -3836,36 +3835,6 @@ sub SendCirculationAlert { > return; > } > >-=head2 updateWrongTransfer >- >- $items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary); >- >-This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation >- >-=cut >- >-sub updateWrongTransfer { >- my ( $itemNumber,$waitingAtLibrary,$FromLibrary ) = @_; >- >- # first step: cancel the original transfer >- my $item = Koha::Items->find($itemNumber); >- my $transfer = $item->get_transfer; >- $transfer->set({ datecancelled => dt_from_string, cancellation_reason => 'WrongTransfer' })->store(); >- >- # second step: create a new transfer to the right location >- my $new_transfer = $item->request_transfer( >- { >- to => $transfer->to_library, >- reason => $transfer->reason, >- comment => $transfer->comments, >- ignore_limits => 1, >- enqueue => 1 >- } >- ); >- >- return $new_transfer; >-} >- > =head2 CalcDateDue > > $newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower); >diff --git a/C4/Items.pm b/C4/Items.pm >index 129a36f650c..c7d21ce036a 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -400,7 +400,7 @@ sub ModItemTransfer { > to => $to_library, > reason => $trigger, > ignore_limits => 1, >- replace => 1 >+ replace => $trigger > } > ); > } >diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm >index a31938343b2..abadf85afe8 100644 >--- a/C4/RotatingCollections.pm >+++ b/C4/RotatingCollections.pm >@@ -484,7 +484,7 @@ sub TransferCollection { > to => $to_library, > reason => "RotatingCollection", > ignore_limits => 0, >- replace => 1 >+ replace => "RotatingCollection" > } > ); # Replace transfer > # FIXME: If we just replaced a StockRotationAdvance, >diff --git a/Koha/Item.pm b/Koha/Item.pm >index b325570910a..c5e0ca867b4 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -736,7 +736,7 @@ sub check_booking { > { > to => $to_library, > reason => $reason, >- [ ignore_limits => 0, enqueue => 1, replace => 1 ] >+ [ ignore_limits => 0, enqueue => 1, replace => 'reason' ] > } > ); > >@@ -775,8 +775,8 @@ sub request_transfer { > Koha::Exceptions::Item::Transfer::InQueue->throw( transfer => $request ) > if ( $request && !$params->{enqueue} && !$params->{replace} ); > >- $request->cancel( { reason => $params->{reason}, force => 1 } ) >- if ( defined($request) && $params->{replace} ); >+ $request->cancel( { reason => $params->{replace}, force => 1 } ) >+ if ( defined($request) && $params->{replace} ); > > my $transfer = Koha::Item::Transfer->new( > { >diff --git a/Koha/StockRotationItem.pm b/Koha/StockRotationItem.pm >index 66578d83b01..48f8e4d5852 100644 >--- a/Koha/StockRotationItem.pm >+++ b/Koha/StockRotationItem.pm >@@ -265,7 +265,7 @@ sub advance { > to => $new_stage->branchcode, > reason => "StockrotationAdvance", > ignore_limits => 1, >- replace => 1 >+ replace => "StockrotationAdvance" > } > ); # Replace transfer > } >diff --git a/circ/returns.pl b/circ/returns.pl >index 4512460e975..db4b1ef14a9 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -35,7 +35,7 @@ use CGI qw ( -utf8 ); > use DateTime; > > use C4::Auth qw( get_template_and_user get_session haspermission ); >-use C4::Circulation qw( barcodedecode GetBranchItemRule AddReturn updateWrongTransfer LostItem ); >+use C4::Circulation qw( barcodedecode GetBranchItemRule AddReturn LostItem ); > use C4::Context; > use C4::Members::Messaging; > use C4::Members; >@@ -578,7 +578,10 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) { > ); > > # Update the transfer to reflect the new item holdingbranch >- my $new_transfer = updateWrongTransfer($messages->{'WrongTransferItem'},$messages->{'WrongTransfer'}, $userenv_branch); >+ my $item = Koha::Items->find($messages->{'WrongTransferItem'}); >+ my $old_transfer = $item->get_transfer; >+ my $new_transfer = $item->request_transfer( >+ { to => $old_transfer->tobranch, reason => $old_transfer->reason, replace => 'WrongTransfer' } ); > $template->param( > NewTransfer => $new_transfer->id > ); >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 009c968c445..4336e9a48d8 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 75; >+use Test::More tests => 74; > use Test::Exception; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); >@@ -33,7 +33,7 @@ use t::lib::TestBuilder; > > use C4::Accounts; > use C4::Calendar qw( new insert_single_holiday insert_week_day_holiday delete_holiday ); >-use C4::Circulation qw( AddIssue AddReturn CanBookBeRenewed GetIssuingCharges AddRenewal GetSoonestRenewDate GetLatestAutoRenewDate LostItem GetUpcomingDueIssues CanBookBeIssued AddIssuingCharge MarkIssueReturned ProcessOfflinePayment transferbook updateWrongTransfer ); >+use C4::Circulation qw( AddIssue AddReturn CanBookBeRenewed GetIssuingCharges AddRenewal GetSoonestRenewDate GetLatestAutoRenewDate LostItem GetUpcomingDueIssues CanBookBeIssued AddIssuingCharge MarkIssueReturned ProcessOfflinePayment transferbook ); > use C4::Biblio; > use C4::Items qw( ModItemTransfer ); > use C4::Log; >@@ -6404,46 +6404,6 @@ subtest "Item's onloan value should be set if checked out item is checked out to > ok( $item->get_from_storage->onloan, "Item's onloan column is set after second checkout" ); > }; > >-subtest "updateWrongTransfer tests" => sub { >- plan tests => 5; >- >- my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); >- my $item = $builder->build_sample_item( >- { >- homebranch => $library1->branchcode, >- holdingbranch => $library2->branchcode, >- datelastseen => undef >- } >- ); >- >- my $transfer = $builder->build_object( >- { >- class => 'Koha::Item::Transfers', >- value => { >- itemnumber => $item->itemnumber, >- frombranch => $library2->branchcode, >- tobranch => $library1->branchcode, >- daterequested => dt_from_string, >- datesent => dt_from_string, >- datecancelled => undef, >- datearrived => undef, >- reason => 'Manual' >- } >- } >- ); >- is( ref($transfer), 'Koha::Item::Transfer', 'Mock transfer added' ); >- >- my $new_transfer = C4::Circulation::updateWrongTransfer($item->itemnumber, $library1->branchcode); >- is(ref($new_transfer), 'Koha::Item::Transfer', "updateWrongTransfer returns a 'Koha::Item::Transfer' object"); >- ok( !$new_transfer->in_transit, "New transfer is NOT created as in transit (or cancelled)"); >- >- my $original_transfer = $transfer->get_from_storage; >- ok( defined($original_transfer->datecancelled), "Original transfer was cancelled"); >- is( $original_transfer->cancellation_reason, 'WrongTransfer', "Original transfer cancellation reason is 'WrongTransfer'"); >-}; >- > subtest "SendCirculationAlert" => sub { > plan tests => 3; > >diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t >index 9fa642be693..796244d2c8f 100755 >--- a/t/db_dependent/Koha/Item.t >+++ b/t/db_dependent/Koha/Item.t >@@ -846,7 +846,7 @@ subtest 'pickup_locations() tests' => sub { > }; > > subtest 'request_transfer' => sub { >- plan tests => 13; >+ plan tests => 14; > $schema->storage->txn_begin; > > my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); >@@ -861,15 +861,16 @@ subtest 'request_transfer' => sub { > # Mandatory fields tests > throws_ok { $item->request_transfer( { to => $library1 } ) } > 'Koha::Exceptions::MissingParameter', >- 'Exception thrown if `reason` parameter is missing'; >+ 'Exception thrown if `reason` parameter is missing'; > > throws_ok { $item->request_transfer( { reason => 'Manual' } ) } > 'Koha::Exceptions::MissingParameter', >- 'Exception thrown if `to` parameter is missing'; >+ 'Exception thrown if `to` parameter is missing'; > > # Successful request >- my $transfer = $item->request_transfer({ to => $library1, reason => 'Manual' }); >- is( ref($transfer), 'Koha::Item::Transfer', >+ my $transfer = $item->request_transfer( { to => $library1, reason => 'Manual' } ); >+ is( >+ ref($transfer), 'Koha::Item::Transfer', > 'Koha::Item->request_transfer should return a Koha::Item::Transfer object' > ); > my $original_transfer = $transfer->get_from_storage; >@@ -877,50 +878,59 @@ subtest 'request_transfer' => sub { > # Transfer already in progress > throws_ok { $item->request_transfer( { to => $library2, reason => 'Manual' } ) } > 'Koha::Exceptions::Item::Transfer::InQueue', >- 'Exception thrown if transfer is already in progress'; >+ 'Exception thrown if transfer is already in progress'; > > my $exception = $@; >- is( ref( $exception->transfer ), >+ is( >+ ref( $exception->transfer ), > 'Koha::Item::Transfer', >- 'The exception contains the found Koha::Item::Transfer' ); >+ 'The exception contains the found Koha::Item::Transfer' >+ ); > > # Queue transfer >- my $queued_transfer = $item->request_transfer( >- { to => $library2, reason => 'Manual', enqueue => 1 } ); >- is( ref($queued_transfer), 'Koha::Item::Transfer', >- 'Koha::Item->request_transfer allowed when enqueue is set' ); >+ my $queued_transfer = $item->request_transfer( { to => $library2, reason => 'Manual', enqueue => 1 } ); >+ is( >+ ref($queued_transfer), 'Koha::Item::Transfer', >+ 'Koha::Item->request_transfer allowed when enqueue is set' >+ ); > my $transfers = $item->get_transfers; >- is($transfers->count, 2, "There are now 2 live transfers in the queue"); >+ is( $transfers->count, 2, "There are now 2 live transfers in the queue" ); > $transfer = $transfer->get_from_storage; >- is_deeply($transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged"); >+ is_deeply( $transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged" ); > $queued_transfer->datearrived(dt_from_string)->store(); > > # Replace transfer >- my $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' ); >+ my $replaced_transfer = >+ $item->request_transfer( { to => $library2, reason => 'Manual', replace => 'WrongTransfer' } ); >+ is( >+ ref($replaced_transfer), 'Koha::Item::Transfer', >+ 'Koha::Item->request_transfer allowed when replace is set' >+ ); > $original_transfer->discard_changes; >- ok($original_transfer->datecancelled, "Original transfer cancelled"); >+ ok( $original_transfer->datecancelled, "Original transfer cancelled" ); >+ is( $original_transfer->cancellation_reason, "WrongTransfer", "Original cancellation_reason set correctly" ); > $transfers = $item->get_transfers; >- is($transfers->count, 1, "There is only 1 live transfer in the queue"); >+ is( $transfers->count, 1, "There is only 1 live transfer in the queue" ); > $replaced_transfer->datearrived(dt_from_string)->store(); > > # BranchTransferLimits >- t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); >- t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); >- my $limit = Koha::Item::Transfer::Limit->new({ >- fromBranch => $library2->branchcode, >- toBranch => $library1->branchcode, >- itemtype => $item->effective_itemtype, >- })->store; >+ t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', 1 ); >+ t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); >+ my $limit = Koha::Item::Transfer::Limit->new( >+ { >+ fromBranch => $library2->branchcode, >+ toBranch => $library1->branchcode, >+ itemtype => $item->effective_itemtype, >+ } >+ )->store; > > throws_ok { $item->request_transfer( { to => $library1, reason => 'Manual' } ) } > 'Koha::Exceptions::Item::Transfer::Limit', >- 'Exception thrown if transfer is prevented by limits'; >+ 'Exception thrown if transfer is prevented by limits'; > > my $forced_transfer = $item->request_transfer( { to => $library1, reason => 'Manual', ignore_limits => 1 } ); >- is( ref($forced_transfer), 'Koha::Item::Transfer', >+ is( >+ ref($forced_transfer), 'Koha::Item::Transfer', > 'Koha::Item->request_transfer allowed when ignore_limits is set' > ); > >-- >2.47.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28294
:
120637
|
121062
|
121063
|
121064
|
121077
|
121078
|
121079
|
121095
|
121096
|
121097
|
121714
|
121715
|
121716
|
130082
|
130083
|
130084
|
130085
|
130086
|
130087
|
132212
|
132213
|
132214
|
132215
|
132216
|
132217
|
160516
|
160517
|
160518
|
160519
|
160520
|
160521
|
160536
|
160692
|
168281
|
172778
|
172781
|
173856
|
174805