Bug 25755

Summary: Add a Koha::Item->request_transfer method
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize <martin.renvoize>
Status: Pushed to master --- QA Contact: Joonas Kylmälä <joonas.kylmala>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jonathan.druart+koha, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
This work adds a `request_transfer` method to `Koha::Item` that allows a transfer to be requested. The method will throw exceptions for missing parameters and it obeys the transfer limits. Optionally, one can pass 'ignore_limits' and 'enqueue'/'replace' parameters to override the transfer limits and handle cases where a transfer request may already exist.
Version(s) released in:
21.05.00
Bug Depends on: 23092    
Bug Blocks: 25757    
Attachments: Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Unit tests for update get_transfer
Bug 25755: Fix typo in POD
Bug 25755: (follow-up) Fix unit test
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Unit tests for update get_transfer
Bug 25755: Fix typo in POD
Bug 25755: (follow-up) Fix unit test
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Unit tests for update get_transfer
Bug 25755: Fix typo in POD
Bug 25755: (follow-up) Fix unit test
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Unit tests for update to get_transfer
Bug 25755: Fix typo in POD
Bug 25755: (follow-up) Fix unit test
Bug 25755: (QA follow-up) Further POD clarifications
Bug 25755: Add Koha::Item->request_transfer method
Bug 25755: (QA follow-up) Clarify POD and parameters
Bug 25755: Add Unit tests for update to get_transfer
Bug 25755: Fix typo in POD
Bug 25755: (follow-up) Fix unit test
Bug 25755: (QA follow-up) Further POD clarifications

Description Martin Renvoize 2020-06-15 14:54:21 UTC
We should have a `request_transfer` method available within the Koha::Item object.
Comment 1 Martin Renvoize 2020-06-15 15:18:37 UTC
Created attachment 105887 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

TODO: Complete work of LIFO queues for get_transfer and add tests for
that
Comment 2 Martin Renvoize 2020-07-23 13:49:18 UTC
Created attachment 107240 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff
Comment 3 Martin Renvoize 2020-07-24 09:56:03 UTC
Created attachment 107294 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff
Comment 4 Martin Renvoize 2020-09-17 16:19:28 UTC
Created attachment 110273 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Comment 5 Martin Renvoize 2020-09-17 16:20:53 UTC
Tested in use by Kathleen as part of relying on bug 22569
Comment 6 Katrin Fischer 2020-09-17 20:44:03 UTC
I am a little confused by this one. The POD documents a force parameter, but it doesn't appear to be implemented (see FIXME). And there is also another FIXME about adding tests. Can you please check?
Comment 7 Martin Renvoize 2020-09-18 11:19:12 UTC
Created attachment 110376 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-09-18 11:23:56 UTC
(In reply to Katrin Fischer from comment #6)
> I am a little confused by this one. The POD documents a force parameter, but
> it doesn't appear to be implemented (see FIXME). And there is also another
> FIXME about adding tests. Can you please check?

Fair questions; The FIXME was left behind during various rebases and signified possible future work to be done. I have removed it in the followup and left myself a local note instead.  As for the force parameter, it was in-fact implemented but only for the 'transfer limits' case and not the 'transfer in progress' case.  I've renamed the parameter to clarify it's use in the followup.

This is really just foundational work.. I've tried hard to think of future use cases, but I have a feeling I'll be making minor adjustments to this method as I start replacing uses of C4::Circulation::transferbook and C4::Items::ModItemTransfer with calls to this new method.
Comment 9 Martin Renvoize 2020-09-18 13:59:58 UTC
Created attachment 110392 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Comment 10 Martin Renvoize 2020-09-18 14:00:04 UTC
Created attachment 110393 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-09-18 14:00:09 UTC
Created attachment 110394 [details] [review]
Bug 25755: Add Unit tests for update get_transfer

We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.

This patch update the unit tests to test for this.
Comment 12 Katrin Fischer 2020-09-20 17:43:47 UTC
Created attachment 110430 [details] [review]
Bug 25755: Fix typo in POD
Comment 13 Katrin Fischer 2020-09-20 17:45:18 UTC
I am sorry, but now tests are failing:


#   Failed test 'request_transfer'
#   at t/db_dependent/Koha/Item.t line 403.
[Transfer not allowed]# Looks like your test exited with 255 just after 5.
t/db_dependent/Koha/Item.t ... Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 4/8 subtests 
t/db_dependent/Koha/Items.t .. ok     

Test Summary Report
-------------------
t/db_dependent/Koha/Item.t (Wstat: 65280 Tests: 5 Failed: 1)
  Failed test:  5
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 8 tests but ran 5.
Files=2, Tests=17, 17 wallclock secs ( 0.05 usr  0.02 sys + 12.27 cusr  2.00 csys = 14.34 CPU)
Result: FAIL
Comment 14 Martin Renvoize 2020-09-21 13:35:58 UTC
Created attachment 110466 [details] [review]
Bug 25755: (follow-up) Fix unit test
Comment 15 Martin Renvoize 2020-09-21 13:36:34 UTC
Oops.. I just forgot to update the unit tests for the change in parameter name (ignore_limits vs force)
Comment 16 Katrin Fischer 2020-09-21 20:49:22 UTC
Created attachment 110502 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2020-09-21 20:49:27 UTC
Created attachment 110503 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Katrin Fischer 2020-09-21 20:49:32 UTC
Created attachment 110504 [details] [review]
Bug 25755: Add Unit tests for update get_transfer

We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.

This patch update the unit tests to test for this.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Katrin Fischer 2020-09-21 20:49:37 UTC
Created attachment 110505 [details] [review]
Bug 25755: Fix typo in POD

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2020-09-21 20:49:44 UTC
Created attachment 110506 [details] [review]
Bug 25755: (follow-up) Fix unit test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Tomás Cohen Arazi 2020-10-02 12:32:55 UTC
Created attachment 111142 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Tomás Cohen Arazi 2020-10-02 12:33:03 UTC
Created attachment 111143 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Tomás Cohen Arazi 2020-10-02 12:33:09 UTC
Created attachment 111144 [details] [review]
Bug 25755: Add Unit tests for update get_transfer

We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.

This patch update the unit tests to test for this.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Tomás Cohen Arazi 2020-10-02 12:33:16 UTC
Created attachment 111145 [details] [review]
Bug 25755: Fix typo in POD

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Tomás Cohen Arazi 2020-10-02 12:33:24 UTC
Created attachment 111146 [details] [review]
Bug 25755: (follow-up) Fix unit test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2021-02-07 13:40:49 UTC
According to the graph this would be the next step in the transfers tree, but the patches appear to have been removed. Last I see is a move from PQA to Blocked in the history, but no comment.

I'd be happy to continue working on the transfers patches in small steps, but not confident enough to do them all together I think.

There are some other patches in the QA queue further down - I am going to move them to BLOCKED for now as well.
Comment 27 Martin Renvoize 2021-02-09 10:47:11 UTC
Created attachment 116541 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 28 Martin Renvoize 2021-02-09 10:47:14 UTC
Created attachment 116542 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 29 Martin Renvoize 2021-02-09 10:47:17 UTC
Created attachment 116543 [details] [review]
Bug 25755: Add Unit tests for update to get_transfer

We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.

This patch update the unit tests to test for this.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 30 Martin Renvoize 2021-02-09 10:47:20 UTC
Created attachment 116544 [details] [review]
Bug 25755: Fix typo in POD

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 31 Martin Renvoize 2021-02-09 10:47:23 UTC
Created attachment 116545 [details] [review]
Bug 25755: (follow-up) Fix unit test

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 32 Martin Renvoize 2021-02-09 10:47:26 UTC
Created attachment 116546 [details] [review]
Bug 25755: (QA follow-up) Further POD clarifications

This patch attempts to further clarify the queueing nature of transfer
requests in the get_transfer method POD.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 33 Jonathan Druart 2021-03-03 14:28:36 UTC
Created attachment 117574 [details] [review]
Bug 25755: Add Koha::Item->request_transfer method

This patch adds the request_transfer method to the Koha::Item object.
The new method requires `to` and `reason` parameters and will throw an
exception if an active transfer request is found on the item.

Test plan
1/ Run the included updated tests in t/db_dependent/Koha/Item.t
2/ Verify the tests pass
3/ Signoff

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 34 Jonathan Druart 2021-03-03 14:28:39 UTC
Created attachment 117575 [details] [review]
Bug 25755: (QA follow-up) Clarify POD and parameters

This patch removes a FIXME that signified possible future work and also
updates the POD and optional parameters to more clearly specify their
function.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 35 Jonathan Druart 2021-03-03 14:28:43 UTC
Created attachment 117576 [details] [review]
Bug 25755: Add Unit tests for update to get_transfer

We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.

This patch update the unit tests to test for this.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 36 Jonathan Druart 2021-03-03 14:28:46 UTC
Created attachment 117577 [details] [review]
Bug 25755: Fix typo in POD

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 37 Jonathan Druart 2021-03-03 14:28:49 UTC
Created attachment 117578 [details] [review]
Bug 25755: (follow-up) Fix unit test

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 38 Jonathan Druart 2021-03-03 14:28:53 UTC
Created attachment 117579 [details] [review]
Bug 25755: (QA follow-up) Further POD clarifications

This patch attempts to further clarify the queueing nature of transfer
requests in the get_transfer method POD.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 39 Jonathan Druart 2021-03-03 14:36:42 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 40 Fridolin Somers 2021-03-04 12:22:57 UTC
Enhancement not pushed to 20.11.x