Bug 28294 - C4::Circulation::updateWrongTransfer should be moved into Koha::
Summary: C4::Circulation::updateWrongTransfer should be moved into Koha::
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 24434 27064
Blocks: 28527
  Show dependency treegraph
 
Reported: 2021-05-06 12:59 UTC by Martin Renvoize
Modified: 2021-07-09 06:34 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28294: WIP (1.33 KB, patch)
2021-05-06 15:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.67 KB, patch)
2021-05-17 16:22 UTC, Martin Renvoize
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
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (1.78 KB, patch)
2021-05-17 16:22 UTC, Martin Renvoize
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
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (3.00 KB, patch)
2021-05-18 08:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.12 KB, patch)
2021-05-18 08:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28294: Add 'WrongTransfer' to transfer reason enum (3.76 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28294: Convert updateWrongTransfer to use request_transfer (3.00 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28294: Remove use of updateWrongTransfer (1.12 KB, patch)
2021-06-08 10:58 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 2021-05-06 15:32:09 UTC
Created attachment 120637 [details] [review]
Bug 28294: WIP
Comment 2 Martin Renvoize 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 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 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 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 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 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 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 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 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 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 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.