Bug 28294 - C4::Circulation::updateWrongTransfer should be moved into Koha::
Summary: C4::Circulation::updateWrongTransfer should be moved into Koha::
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 24434 27064 35721
Blocks: 28527 35736
  Show dependency treegraph
 
Reported: 2021-05-06 12:59 UTC by Martin Renvoize (ashimema)
Modified: 2024-12-05 21:57 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 28294: WIP (1.33 KB, patch)
2021-05-06 15:32 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.67 KB, patch)
2021-05-17 16:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Pass 'replace' through to Koha::Item::Transfer->cancel (1.91 KB, patch)
2021-05-17 16:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (1.78 KB, patch)
2021-05-17 16:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.73 KB, patch)
2021-05-17 23:25 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28294: Pass 'replace' through to Koha::Item::Transfer->cancel (1.80 KB, patch)
2021-05-17 23:25 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (1.87 KB, patch)
2021-05-17 23:25 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.73 KB, patch)
2021-05-18 08:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (3.00 KB, patch)
2021-05-18 08:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.12 KB, patch)
2021-05-18 08:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.76 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (3.00 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.12 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.43 KB, patch)
2022-02-02 10:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer (3.23 KB, patch)
2022-02-02 10:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Unit Tests (3.45 KB, patch)
2022-02-02 10:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.54 KB, patch)
2022-02-02 10:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer method (1.53 KB, patch)
2022-02-02 10:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove unit tests for updateWrongTransfer (3.43 KB, patch)
2022-02-02 10:49 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.54 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer (3.27 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Unit Tests (3.48 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.58 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer method (1.57 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Remove unit tests for updateWrongTransfer (3.47 KB, patch)
2022-03-25 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.44 KB, patch)
2024-01-04 12:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer (3.23 KB, patch)
2024-01-04 12:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Unit Tests (3.45 KB, patch)
2024-01-04 12:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.54 KB, patch)
2024-01-04 12:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer method (1.54 KB, patch)
2024-01-04 12:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove unit tests for updateWrongTransfer (3.43 KB, patch)
2024-01-04 12:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: [ALT] Remove updateWrongTransfer (14.60 KB, patch)
2024-01-04 15:01 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer (14.74 KB, patch)
2024-01-09 13:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer (14.75 KB, patch)
2024-06-28 16:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer (14.75 KB, patch)
2024-10-15 14:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer (14.79 KB, patch)
2024-10-15 14:44 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 28294: Remove updateWrongTransfer (14.89 KB, patch)
2024-11-01 08:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28294: (follow-up) Adjust call to request transfer (1.25 KB, patch)
2024-11-19 18:30 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-05-06 12:59:38 UTC
Bug 24434 highlighted a case where this method is restored to use.. we should really move to using Koha:: methods instead.
Comment 1 Martin Renvoize (ashimema) 2021-05-06 15:32:09 UTC
Created attachment 120637 [details] [review]
Bug 28294: WIP
Comment 2 Martin Renvoize (ashimema) 2021-05-17 16:22:25 UTC
Created attachment 121062 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.
Comment 3 Martin Renvoize (ashimema) 2021-05-17 16:22:28 UTC
Created attachment 121063 [details] [review]
Bug 28294: Pass 'replace' through to Koha::Item::Transfer->cancel

The `replace` option found in Koha::Item->request_transfer should be
passed through to Koha::Item::Transfer->cancel and prevent any reverse
transfers from being queued in such cases.

This allows us to replace updateWrongTransfer entirely with
Koha::Item->request_transfer with the 'replace' parameter enabled.
Comment 4 Martin Renvoize (ashimema) 2021-05-17 16:22:32 UTC
Created attachment 121064 [details] [review]
Bug 28294: Convert updateWrongTransfer to use request_transfer

This patch is an interim patch to prove that the updated
request_transfer method achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.
Comment 5 Martin Renvoize (ashimema) 2021-05-17 16:24:00 UTC
To Do
* Add tests for 'replace' on 'Cancel' and 'Request' methods.
* Drop updateWrongTransfer and use request_transfer directly in the controller.
Comment 6 Victor Grousset/tuxayo 2021-05-17 23:25:21 UTC
Created attachment 121077 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Victor Grousset/tuxayo 2021-05-17 23:25:25 UTC
Created attachment 121078 [details] [review]
Bug 28294: Pass 'replace' through to Koha::Item::Transfer->cancel

The `replace` option found in Koha::Item->request_transfer should be
passed through to Koha::Item::Transfer->cancel and prevent any reverse
transfers from being queued in such cases.

This allows us to replace updateWrongTransfer entirely with
Koha::Item->request_transfer with the 'replace' parameter enabled.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Victor Grousset/tuxayo 2021-05-17 23:25:29 UTC
Created attachment 121079 [details] [review]
Bug 28294: Convert updateWrongTransfer to use request_transfer

This patch is an interim patch to prove that the updated
request_transfer method achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.
  run updatedatabase when applying

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 9 Victor Grousset/tuxayo 2021-05-17 23:26:26 UTC
Switching to "assigned" due to comment 5
Does that makes sense?
Comment 10 Victor Grousset/tuxayo 2021-05-17 23:53:29 UTC
git-bz trap!
Set back to "need SO" because git-bz when applying bug 27064 won't ask to follow the dependency to this bug :o
Comment 11 Martin Renvoize (ashimema) 2021-05-18 07:56:40 UTC
I'm moving the middle patch into bug 27064 to prevent hold up of that bug whilst I continue to refine this one. - Bug 28294: Pass 'replace' through to Koha::Item::Transfer->cancel
Comment 12 Martin Renvoize (ashimema) 2021-05-18 08:46:14 UTC
Created attachment 121095 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 13 Martin Renvoize (ashimema) 2021-05-18 08:46:18 UTC
Created attachment 121096 [details] [review]
Bug 28294: Convert updateWrongTransfer to use request_transfer

This patch is an interim patch to prove that the updated
request_transfer method achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 14 Martin Renvoize (ashimema) 2021-05-18 08:46:26 UTC
Created attachment 121097 [details] [review]
Bug 28294: Remove use of updateWrongTransfer
Comment 15 Victor Grousset/tuxayo 2021-05-18 10:50:29 UTC
Ok, switching back to "assigned", no need to keep in "need signoff" as a workaround for git-bz isn't need as it's no more a dependency of another patch.
Comment 16 Victor Grousset/tuxayo 2021-06-01 16:26:07 UTC
Does that still need need work about the 1st half of comment 5?
Comment 17 Martin Renvoize (ashimema) 2021-06-08 10:58:28 UTC
Created attachment 121714 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 18 Martin Renvoize (ashimema) 2021-06-08 10:58:32 UTC
Created attachment 121715 [details] [review]
Bug 28294: Convert updateWrongTransfer to use request_transfer

This patch is an interim patch to prove that the updated
request_transfer method achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 19 Martin Renvoize (ashimema) 2021-06-08 10:58:36 UTC
Created attachment 121716 [details] [review]
Bug 28294: Remove use of updateWrongTransfer
Comment 20 Marcel de Rooy 2021-07-09 06:34:40 UTC
Please add tests for these module changes.
Comment 21 Martin Renvoize (ashimema) 2022-02-02 10:48:43 UTC
Created attachment 130082 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 22 Martin Renvoize (ashimema) 2022-02-02 10:48:46 UTC
Created attachment 130083 [details] [review]
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer

This patch is an interim patch to prove that the new remedy method
achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 23 Martin Renvoize (ashimema) 2022-02-02 10:48:50 UTC
Created attachment 130084 [details] [review]
Bug 28294: Unit Tests

This patch adds unit tests for the new 'remedy' method we are
introducing to Koha::Item::Transfer.
Comment 24 Martin Renvoize (ashimema) 2022-02-02 10:48:54 UTC
Created attachment 130085 [details] [review]
Bug 28294: Remove use of updateWrongTransfer

This patch removes the last remaining use of updateWrongTransfer.

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.
Comment 25 Martin Renvoize (ashimema) 2022-02-02 10:48:57 UTC
Created attachment 130086 [details] [review]
Bug 28294: Remove updateWrongTransfer method

This patch removes the updateWrongTransfer method entirely from
C4/Circulation.pm.

Test plan
1/ Confirm that there are no longer any mentions of updateWrongTransfer
in the codebase.
Comment 26 Martin Renvoize (ashimema) 2022-02-02 10:49:00 UTC
Created attachment 130087 [details] [review]
Bug 28294: Remove unit tests for updateWrongTransfer
Comment 27 Martin Renvoize (ashimema) 2022-02-02 10:50:01 UTC
Rebased and unit tests added.
Comment 28 Marcel de Rooy 2022-03-25 09:41:02 UTC
Created attachment 132212 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 29 Marcel de Rooy 2022-03-25 09:41:05 UTC
Created attachment 132213 [details] [review]
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer

This patch is an interim patch to prove that the new remedy method
achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 30 Marcel de Rooy 2022-03-25 09:41:09 UTC
Created attachment 132214 [details] [review]
Bug 28294: Unit Tests

This patch adds unit tests for the new 'remedy' method we are
introducing to Koha::Item::Transfer.
Comment 31 Marcel de Rooy 2022-03-25 09:41:12 UTC
Created attachment 132215 [details] [review]
Bug 28294: Remove use of updateWrongTransfer

This patch removes the last remaining use of updateWrongTransfer.

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.
Comment 32 Marcel de Rooy 2022-03-25 09:41:16 UTC
Created attachment 132216 [details] [review]
Bug 28294: Remove updateWrongTransfer method

This patch removes the updateWrongTransfer method entirely from
C4/Circulation.pm.

Test plan
1/ Confirm that there are no longer any mentions of updateWrongTransfer
in the codebase.
Comment 33 Marcel de Rooy 2022-03-25 09:41:19 UTC
Created attachment 132217 [details] [review]
Bug 28294: Remove unit tests for updateWrongTransfer
Comment 34 Marcel de Rooy 2022-03-25 09:41:38 UTC
Looking here, started with a rebase
Comment 35 Marcel de Rooy 2022-03-25 09:50:38 UTC
# Subtest: AddRenewal and AddIssuingCharge tests
    1..13
    not ok 1 - Exception is thrown when renewal update to issues fails
    #   Failed test 'Exception is thrown when renewal update to issues fails'
    #   at t/db_dependent/Circulation.t line 4147.
    # expecting: Koha::Exceptions::Checkout::FailedRenewal
    # found: normal exit
    ok 2 - renew log not added because of the syspref RenewalLog
    ok 3 - AddRenewal should set the renewal date with the time part
    not ok 4
    #   Failed test at t/db_dependent/Circulation.t line 4170.
    #          got: '3'
    #     expected: '2'
    ok 5 - The issue of item with issuing charge generates an accountline of the correct type
    ok 6 - AddIssuingCharge correctly sets branchcode
    ok 7 - AddIssue does not set a hardcoded description for the accountline
    ok 8 - The renewal of item with issuing charge generates an accountline of the correct type
    ok 9 - AddRenewal correctly sets branchcode
    ok 10 - AddRenewal does not set a hardcoded description for the accountline
    ok 11 - renew log successfully added
    ok 12 - renew statistic successfully added with passed branch
    not ok 13 - Passing skipfinecalc causes fine calculation on renewal to be skipped
    #   Failed test 'Passing skipfinecalc causes fine calculation on renewal to be skipped'
    #   at t/db_dependent/Circulation.t line 4207.
    #          got: '6'
    #     expected: '5'
    # Looks like you failed 3 tests of 13.
not ok 45 - AddRenewal and AddIssuingCharge tests

I am not sure if Circulation.t fails due to changes here. But it did not fail on Jenkins on current master?
Comment 36 Marcel de Rooy 2022-03-25 09:57:49 UTC
+FIXME: remedy or rectify
+    # FIXME: Should we remedy a transfer if it's just gone back to the originating branch, or just leave it in transit as is.

Two FIXME's added. Can we do something about them here?

+  my $new_transfer = $transfer->remedy(
+    {
+        to     => $to_library,
+        reason => $reason,
+        [ ignore_limits => 0, force => 0 ]
+    }
+  );

There is no code for ignore_limits and force ?
Comment 37 Marcel de Rooy 2022-03-25 09:58:50 UTC
+            MODIFY COLUMN
+                `reason` enum(
+                    'Manual',
+                    'StockrotationAdvance',
+                    'StockrotationRepatriation',
+                    'ReturnToHome',
+                    'ReturnToHolding',
+                    'RotatingCollection',
+                    'Reserve',
+                    'LostReserve',
+                    'CancelReserve',
+                    'TransferCancellation',
+                    'WrongTransfer'
+                )
+            AFTER `comments`

Why modify a column after something ?
Comment 38 Marcel de Rooy 2022-03-25 10:00:08 UTC
Please provide some feedback. Generally looking good !
Comment 39 Martin Renvoize (ashimema) 2024-01-04 12:29:52 UTC
Created attachment 160516 [details] [review]
Bug 28294: Add 'WrongTransfer' to transfer reason enum

This patch adds the 'WrongTransfer' value to the 'reason' enum in the
branchtransfers table.  This is so we can use it as a reason for the
subsequent transfer when a trainsfer is triggered by a checkin at an
incorrect branch after transfer has been initiated.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 40 Martin Renvoize (ashimema) 2024-01-04 12:29:55 UTC
Created attachment 160517 [details] [review]
Bug 28294: Add Koha::Item::Transfer->remedy method and use in updateWrongTransfer

This patch is an interim patch to prove that the new remedy method
achieves the same as updateWrongTransfer.

Test plan
1/ Run t/db_dependant/Circulation.t before and after applying this
patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 41 Martin Renvoize (ashimema) 2024-01-04 12:29:57 UTC
Created attachment 160518 [details] [review]
Bug 28294: Unit Tests

This patch adds unit tests for the new 'remedy' method we are
introducing to Koha::Item::Transfer.
Comment 42 Martin Renvoize (ashimema) 2024-01-04 12:30:00 UTC
Created attachment 160519 [details] [review]
Bug 28294: Remove use of updateWrongTransfer

This patch removes the last remaining use of updateWrongTransfer.

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.
Comment 43 Martin Renvoize (ashimema) 2024-01-04 12:30:03 UTC
Created attachment 160520 [details] [review]
Bug 28294: Remove updateWrongTransfer method

This patch removes the updateWrongTransfer method entirely from
C4/Circulation.pm.

Test plan
1/ Confirm that there are no longer any mentions of updateWrongTransfer
in the codebase.
Comment 44 Martin Renvoize (ashimema) 2024-01-04 12:30:06 UTC
Created attachment 160521 [details] [review]
Bug 28294: Remove unit tests for updateWrongTransfer
Comment 45 Martin Renvoize (ashimema) 2024-01-04 12:36:44 UTC
Rebased, but not I'm revisiting the code I'm wondering why it's needed.. I think we could perhaps just replace the call to updateWrongTransfer with a call to request_transfer instead (and pass 'replace' in with a reason of 'wrongTransfer').
Comment 46 Martin Renvoize (ashimema) 2024-01-04 15:01:02 UTC
Created attachment 160536 [details] [review]
Bug 28294: [ALT] 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.
Comment 47 Martin Renvoize (ashimema) 2024-01-04 15:16:47 UTC
I like the alternative a lot better.. simpler DRYer code.

Setting back to NSO
Comment 48 Martin Renvoize (ashimema) 2024-01-09 13:25:14 UTC
Created attachment 160692 [details] [review]
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.
Comment 49 Martin Renvoize (ashimema) 2024-06-28 16:29:26 UTC
Created attachment 168281 [details] [review]
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.
Comment 50 Martin Renvoize (ashimema) 2024-10-15 14:29:18 UTC
Created attachment 172778 [details] [review]
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.
Comment 51 Martin Renvoize (ashimema) 2024-10-15 14:29:53 UTC
Nothing like another rebase party to get a devs blood pressure rising.
Comment 52 Paul Derscheid 2024-10-15 14:44:06 UTC
Created attachment 172781 [details] [review]
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.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 53 Martin Renvoize (ashimema) 2024-10-15 14:46:34 UTC
The unit tests are included here and the bug is signed off again.. any chance of getting it over the line on QA Marcel?
Comment 54 Marcel de Rooy 2024-10-15 14:54:56 UTC
(In reply to Martin Renvoize (ashimema) from comment #53)
> The unit tests are included here and the bug is signed off again.. any
> chance of getting it over the line on QA Marcel?

Trying to get here soon
Comment 55 Marcel de Rooy 2024-10-18 06:19:21 UTC
Depends on report in FQA. Please change status back if resolved.
Comment 56 Marcel de Rooy 2024-11-01 07:50:24 UTC
QA: Looking here now
Comment 57 Marcel de Rooy 2024-11-01 08:16:01 UTC
Created attachment 173856 [details] [review]
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.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 58 Marcel de Rooy 2024-11-01 08:16:13 UTC
 FAIL   C4/Circulation.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
=> Ignoring this one. Seems to be false. We are just removing one routine.

 WARN   Koha/Item.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 278, now: 279)
=> Ignoring just one space or so :)

@@ -400,7 +400,7 @@ sub ModItemTransfer {
        $transfer = $item->request_transfer(
            {
                to            => $to_library,
                reason        => $trigger,
[...]
                         reason        => $trigger,
-                        replace       => 1
+                        replace       => $trigger
Just for the record. Reads a bit odd. But unfortunately request_transfer does cancel with the "replace reason"..
Comment 59 Katrin Fischer 2024-11-07 17:51:09 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 60 Nick Clemens (kidclamp) 2024-11-19 18:30:30 UTC
Created attachment 174805 [details] [review]
Bug 28294: (follow-up) Adjust call to request transfer

The call expects a library object, not a  branchcode, likely rebasing issue

To test:
1 - Place a hold for an item at a different library than current library
2 - Check item in and confirm transfer
3 - Check item in again
4 - Kaboom!
5 - Apply patch/restart_all
6 - Check item in again
7 - Ok!
Comment 61 Katrin Fischer 2024-11-20 10:53:38 UTC
(In reply to Nick Clemens (kidclamp) from comment #60)
> Created attachment 174805 [details] [review] [review]
> Bug 28294: (follow-up) Adjust call to request transfer
> 
> The call expects a library object, not a  branchcode, likely rebasing issue
> 
> To test:
> 1 - Place a hold for an item at a different library than current library
> 2 - Check item in and confirm transfer
> 3 - Check item in again
> 4 - Kaboom!
> 5 - Apply patch/restart_all
> 6 - Check item in again
> 7 - Ok!

Please switch status back to PQA if you need me to push a follow-up!
Comment 62 Katrin Fischer 2024-11-20 10:54:18 UTC
Follow-up "Adjust call to request transfer" picked for main.