Bug 26618 - C4/RotatingCollections.pm should not use C4::Circulation::transferbook
Summary: C4/RotatingCollections.pm should not use C4::Circulation::transferbook
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Joonas Kylmälä
URL: https://gitlab.com/mrenvoize/Koha/-/t...
Keywords:
Depends on: 24446
Blocks: 22569
  Show dependency tree / graph
 
Reported: 2020-10-06 10:58 UTC by Martin Renvoize (ashimema)
Modified: 2021-12-13 21:12 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement updates the Rotating Collections system to use the recently introduced Koha:: methods for Item Transfers. We expose some extra feedback to the end user to highlight where transfers would have previously failed silently when used in combination with other systems that trigger transfers.
Version(s) released in:
21.05.00
Circulation function:

Attachments
Bug 26618: Remove use of transferbook in RotatingCollections (6.91 KB, patch)
2020-10-06 13:20 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 26618: Remove use of transferbook in RotatingCollections (7.68 KB, patch)
2020-10-06 15:56 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 26618: Remove use of transferbook in RotatingCollections (7.97 KB, patch)
2021-02-09 11:17 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 26618: Remove use of transferbook in RotatingCollections (7.97 KB, patch)
2021-03-03 14:33 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 26618: (QA follow-up) Update messages syntax (3.52 KB, patch)
2021-03-03 14:33 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 26618: (QA follow-up) Fix unit test for TranferCollection change (3.25 KB, patch)
2021-03-03 14:33 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 26618: Improve Exception Class Names (7.33 KB, patch)
2021-03-03 14:33 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2020-10-06 10:58:10 UTC 
We are going to deprecate transferbook and should therefore convert the logic of RotatingCollections to utilise the new methods available to us in Koha::Item and friends.
Comment 1 Martin Renvoize (ashimema) 2020-10-06 13:20:36 UTC 
Created attachment 111305 [details] [review]
Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.
Comment 2 Martin Renvoize (ashimema) 2020-10-06 15:56:34 UTC 
Created attachment 111313 [details] [review]
Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.

We also introduce additional feedback for the end user where it did not
exist before. Now we notify the user if some of the collection could not
be transfers or if transfers were queued rather than set to request
immediately.

Test plan
1/ Set up a rotating collection
2/ Transfer the collection
3/ Confirm the action succeeds
4/ Set up some branch transfer limits that will affect items in your
collection
5/ Transfer the collection
6/ Note that the transfer succeeds but some items are returned as
failures
Comment 3 Martin Renvoize (ashimema) 2020-10-20 13:27:34 UTC 
Please test this as part of the bug 22569 tree linked above.
Comment 4 Katrin Fischer 2021-02-07 13:41:51 UTC 
BLOCKED by dependent bug.
Comment 5 Martin Renvoize (ashimema) 2021-02-09 11:17:35 UTC 
Created attachment 116571 [details] [review]
Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.

We also introduce additional feedback for the end user where it did not
exist before. Now we notify the user if some of the collection could not
be transfers or if transfers were queued rather than set to request
immediately.

Test plan
1/ Set up a rotating collection
2/ Transfer the collection
3/ Confirm the action succeeds
4/ Set up some branch transfer limits that will affect items in your
collection
5/ Transfer the collection
6/ Note that the transfer succeeds but some items are returned as
failures

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 6 Jonathan Druart 2021-03-03 14:33:20 UTC 
Created attachment 117608 [details] [review]
Bug 26618: Remove use of transferbook in RotatingCollections

This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.

We also introduce additional feedback for the end user where it did not
exist before. Now we notify the user if some of the collection could not
be transfers or if transfers were queued rather than set to request
immediately.

Test plan
1/ Set up a rotating collection
2/ Transfer the collection
3/ Confirm the action succeeds
4/ Set up some branch transfer limits that will affect items in your
collection
5/ Transfer the collection
6/ Note that the transfer succeeds but some items are returned as
failures

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 7 Jonathan Druart 2021-03-03 14:33:23 UTC 
Created attachment 117609 [details] [review]
Bug 26618: (QA follow-up) Update messages syntax

As highlighted, we were not using the standard form of message passing
here. This patch updates the template and controller to adopt the more
usual syntax.
Comment 8 Jonathan Druart 2021-03-03 14:33:26 UTC 
Created attachment 117610 [details] [review]
Bug 26618: (QA follow-up) Fix unit test for TranferCollection change

We update TransferCollection to use the settled upon standard for
passing error messages back from a method. This patch updates the
corresponding unit test.
Comment 9 Jonathan Druart 2021-03-03 14:33:30 UTC 
Created attachment 117611 [details] [review]
Bug 26618: Improve Exception Class Names
Comment 10 Jonathan Druart 2021-03-03 14:37:00 UTC 
Pushed to master for 21.05, thanks to everybody involved!
Comment 11 Fridolin Somers 2021-03-04 12:25:37 UTC 
Enhancement not pushed to 20.11.x