Bug 23112 - Circulation of ILL items
Summary: Circulation of ILL items
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Andrew Isherwood
QA Contact:
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on: 23384 7137
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-13 07:51 UTC by Martin Renvoize
Modified: 2019-10-16 09:04 UTC (History)
8 users (show)

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


Attachments
Bug 23122: Add CirculateILL syspref (3.36 KB, patch)
2019-08-05 13:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add circulation to ILL requests (24.59 KB, patch)
2019-08-05 13:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add permission checks (2.79 KB, patch)
2019-08-05 13:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Update request status on return (2.21 KB, patch)
2019-08-05 13:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add unit tests (6.70 KB, patch)
2019-08-05 13:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23122: Add CirculateILL syspref (3.40 KB, patch)
2019-08-05 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add circulation to ILL requests (24.63 KB, patch)
2019-08-05 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add permission checks (2.82 KB, patch)
2019-08-05 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Update request status on return (2.25 KB, patch)
2019-08-05 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add unit tests (6.74 KB, patch)
2019-08-05 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add CirculateILL syspref (3.45 KB, patch)
2019-10-03 08:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add circulation to ILL requests (24.67 KB, patch)
2019-10-03 08:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add permission checks (2.82 KB, patch)
2019-10-03 08:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Update request status on return (2.25 KB, patch)
2019-10-03 08:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add unit tests (6.74 KB, patch)
2019-10-03 08:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: (follow-up) Call C4::Koha::GetAuthorisedValues (978 bytes, patch)
2019-10-10 14:43 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add CirculateILL syspref (3.47 KB, patch)
2019-10-16 09:03 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add circulation to ILL requests (24.67 KB, patch)
2019-10-16 09:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add permission checks (2.82 KB, patch)
2019-10-16 09:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Update request status on return (2.25 KB, patch)
2019-10-16 09:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: Add unit tests (6.74 KB, patch)
2019-10-16 09:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23112: (follow-up) Call C4::Koha::GetAuthorisedValues (978 bytes, patch)
2019-10-16 09:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-06-13 07:51:52 UTC
The ILL framework we have supports the tracking of ILL requests well, but once the item has arrived at the library there isn't a clear workflow for lending it to the end user.

https://wiki.koha-community.org/wiki/ILL_Circulation_RFC
Comment 1 Lisette Scheer 2019-06-13 19:54:10 UTC
Martin, 
I think this is a great idea. 

In the Step: Places a hold on the item for the patron and marks it ready for collection, I'd like to recommend a preference if it marks it as available for pickup and also to make sure that if the person handling incoming ILLs isn't at the same branch as the patron, it would be put in transit to the correct library instead of ready for pickup. 

Lisette
Comment 2 Magnus Enger 2019-06-14 07:27:49 UTC
As I said on the RFC comments page, we have implemented a lot of this in the Swedish "Libris" backend. It would be great to move the functionality from the backend to the module! https://github.com/Libriotech/koha-illbackend-libris
Comment 3 Martin Renvoize 2019-06-14 07:38:12 UTC
(In reply to Lisette Scheer from comment #1)
> In the Step: Places a hold on the item for the patron and marks it ready for
> collection, I'd like to recommend a preference if it marks it as available
> for pickup.

Happy to add a preference, though I do wonder how the item is then meant to get to 'ready for collection'?

> and also to make sure that if the person handling incoming ILLs
> isn't at the same branch as the patron, it would be put in transit to the
> correct library instead of ready for pickup. 

Great idea, I hadn't thought about that requirement when we wrote this spec.. I'll add it to the spec now.
Comment 4 Martin Renvoize 2019-06-14 07:42:37 UTC
(In reply to Magnus Enger from comment #2)
> As I said on the RFC comments page, we have implemented a lot of this in the
> Swedish "Libris" backend. It would be great to move the functionality from
> the backend to the module!
> https://github.com/Libriotech/koha-illbackend-libris

Yeah, I think it quickly became a pattern in a few backends, make sense to move to core and prevent every backend having to impliment their own version.

I do however still like the idea of introducing hooks in a few places in circulation, could make for some interesting plugins and enable alterations to the generic flow if a particular backend needed to make such modifications.
Comment 5 Lisette Scheer 2019-06-14 16:02:01 UTC
(In reply to Martin Renvoize from comment #3)
> (In reply to Lisette Scheer from comment #1)
> > In the Step: Places a hold on the item for the patron and marks it ready for
> > collection, I'd like to recommend a preference if it marks it as available
> > for pickup.
> 
> Happy to add a preference, though I do wonder how the item is then meant to
> get to 'ready for collection'?
> 
If a hold is still placed for the patron, the hold could be captured by checking the item in, perhaps a place hold and marks it ready for collection button and a separate place hold button where they would have to scan it in again to mark the hold ready? That would solve both issues I brought up in my comment. 

Lisette
Comment 6 Martin Renvoize 2019-07-10 10:29:43 UTC
An alternative specification has been sent to the customer and agreed.
Comment 7 Katrin Fischer 2019-07-10 11:10:40 UTC
Can the alternative be shared? There seemed to be agreement on this one (see dicussion on wiki and comments here)
Comment 8 Martin Renvoize 2019-07-10 11:17:18 UTC
It's on the wiki
Comment 9 Andrew Isherwood 2019-08-05 13:02:08 UTC
Created attachment 91987 [details] [review]
Bug 23122: Add CirculateILL syspref
Comment 10 Andrew Isherwood 2019-08-05 13:02:12 UTC Comment hidden (obsolete)
Comment 11 Andrew Isherwood 2019-08-05 13:02:15 UTC Comment hidden (obsolete)
Comment 12 Andrew Isherwood 2019-08-05 13:02:18 UTC Comment hidden (obsolete)
Comment 13 Andrew Isherwood 2019-08-05 13:02:21 UTC Comment hidden (obsolete)
Comment 14 Andrew Isherwood 2019-08-05 13:05:23 UTC
This development has been carried out according to the originally stated requirements of the customer that sponsored it, detailed here: https://wiki.koha-community.org/wiki/ILL_Circulation_RFC

Test plan:

1. Ensure the FreeForm ILL backend is available
2. Enable the "CirculateILL" syspref
3. Ensure you have a statistical patron category defined (patron category type "Statistical")
4. Ensure you have at least one patron in your statistical patron category
5. Create a new FreeForm request (make a note of the library you select when creating it)
6. Mark the new request as confirmed by clicking the "Confirm request" button on the "Manage ILL request" page
7. TEST: Observe that a "Check out" button is now displayed in the request toolbar
8. Click the "Check out" button in the "Manage ILL request" page
9. In the "Issue requested item to..." screen:
  a. Do not select a statistical patron at this time
  b. You can at this point choose an item type, this will determine the type of the item that will be created for this request
  c. TEST: Observe that the default selected "Library" matches that that was defined when creating the request
  d. Do not select a due date at this time
10. Click "Submit"
11. TEST: Observe that the "Item checked out" screen displays, issued to the requesting patron with a due date corresponding to appropriate circ rules
12. Click "Return to request"
13. TEST: Observe that the request's status is now "Checked out"
14. Click the "Bibliographic record ID" link
15. TEST: Observe that the bibliographic record now has one item attached to it which is checked out
16. TEST: Observe that the item barcode is "ILL-" + the ILL request ID
17. Return to step 5., however, this time select a statistical patron and test that the item use is recorded and the item is not issued
18. Return to step 5., however, this time manually select a due date and test that the item's due date is set correctly on check out
19. Check in the item
20. TEST: Observe that the request's status is updated to "Returned to library"
21. Now implement a restriction on the patron (perhaps a fine) which would prevent them from checking out an item
22. Return to step 5. follow the instructions to step 10.
23. TEST: Observe that a banner is displayed at the top of the screen informing you that there was a problem checking the item out, containing a link to the patron's account page
24. Resolve the problem with the patron's account
25. Return to step 8.
26. TEST: Observe that the item is now successfully checked out
27. Disable the "CirculateILL" syspref
28. Return to step 5. at step 7. Observe that the "Check out" button is NOT displayed
Comment 15 Andrew Isherwood 2019-08-05 14:33:05 UTC Comment hidden (obsolete)
Comment 16 Andrew Isherwood 2019-08-05 14:33:08 UTC Comment hidden (obsolete)
Comment 17 Andrew Isherwood 2019-08-05 14:33:11 UTC Comment hidden (obsolete)
Comment 18 Andrew Isherwood 2019-08-05 14:33:15 UTC Comment hidden (obsolete)
Comment 19 Andrew Isherwood 2019-08-05 14:33:18 UTC Comment hidden (obsolete)
Comment 20 Magnus Enger 2019-10-03 08:40:54 UTC Comment hidden (obsolete)
Comment 21 Magnus Enger 2019-10-03 08:44:47 UTC Comment hidden (obsolete)
Comment 22 Andrew Isherwood 2019-10-03 08:48:33 UTC
Created attachment 93530 [details] [review]
Bug 23112: Add CirculateILL syspref

Sponsored-by: Loughborough University
Comment 23 Andrew Isherwood 2019-10-03 08:48:36 UTC
Created attachment 93531 [details] [review]
Bug 23112: Add circulation to ILL requests

This patch adds the ability to circulate ILL requests. Once a request has a suitable status, a "Check out" button is displayed on the "Manage request" toolbar. Clicking this will enable the user to check out the item either to the user who made the request or an in-house statistical  user. A due date can be specified, but if not circ rules are used.

Prior to the check out, an item is created which is attached to the biblio record that was created when the request was added

Sponsored-by: Loughborough University
Comment 24 Andrew Isherwood 2019-10-03 08:48:39 UTC
Created attachment 93532 [details] [review]
Bug 23112: Add permission checks

We only want the ILL circulation functionality to be available if the
appropriate syspref and perm are available, this patch adds those
checks.

Sponsored-by: Loughborough University
Comment 25 Andrew Isherwood 2019-10-03 08:48:42 UTC
Created attachment 93533 [details] [review]
Bug 23112: Update request status on return

This patch updates a request's status to RET upon return

Sponsored-by: Loughborough University
Comment 26 Andrew Isherwood 2019-10-03 08:48:46 UTC
Created attachment 93534 [details] [review]
Bug 23112: Add unit tests

This patch adds unit test for the new Koha::Illrequest::check_out method

Sponsored-by: Loughborough University
Comment 27 Andrew Isherwood 2019-10-03 08:49:43 UTC
Thanks for that Magnus, the bug number is now fixed, as it the failure to apply
Comment 28 Magnus Enger 2019-10-03 09:20:51 UTC
Thanks for fixing the number the applicability! 

Sorry, I get an error on step 5: "Create a new FreeForm request":

Template process failed: undef error - Undefined subroutine &Koha::Illrequest::Logger::GetAuthorisedValues called at /home/vagrant/kohaclone/Koha/Illrequest/Logger.pm line 213.
 at /home/vagrant/kohaclone/C4/Templates.pm line 122

This happens when I click on "New ILL request", fill in the form an click on "Create". It does not happen on current master. 

It does seem weird though, since the patches on the current bug do not seem to touch Koha/Illrequest/Logger.pm?
Comment 29 Magnus Enger 2019-10-03 09:25:42 UTC
Same thing happes when I go to "View ILL requests" an then click on "Manage request":

Template process failed: undef error - Undefined subroutine &Koha::Illrequest::Logger::GetAuthorisedValues called at /home/vagrant/kohaclone/Koha/Illrequest/Logger.pm line 213.
 at /home/vagrant/kohaclone/C4/Templates.pm line 122

Again, it does not happen on master without the current patches.
Comment 30 Andrew Isherwood 2019-10-09 11:16:54 UTC
Hi Magnus

I've just applied this bug on current master and cannot replicate either of these problems, I've just run through the whole test plan without a problem.

The error you're seeing is weird, it's not aware that GetAuthorisedValues should be coming from C4::Koha, it's trying to find it in Illrequest::Logger. I'm "use"ing C4::Koha, but not explicitly importing GetAuthorisedValues, which shouldn't be necessary since it's exported by C4::Koha. This is done all over the place in Koha, so you should be seeing this problem everywhere.

Explicitly importing GetAuthorisedValues would fix it, but that feels like a hack, I'd rather know why it's not being imported by default.

Any chance you could try again? It could I suppose have been a merge problem, I'd like to eliminate that.
Comment 31 Andrew Isherwood 2019-10-10 14:39:22 UTC
Hi Magnus

I just managed to replicate this on one of our test instances which is running plack, so I'm wondering if it's a plack thing. I've learnt the hard way not to try and understand why plack does what it does, so I'm just going to create a patch now which will just explicity import GetAuthorisedValues and hope that'll keep plack happy.
Comment 32 Andrew Isherwood 2019-10-10 14:43:41 UTC
Created attachment 93958 [details] [review]
Bug 23112: (follow-up) Call C4::Koha::GetAuthorisedValues

In response to the problem Magnus was experiencing in comment #28 & #29
we now call GetAuthorisedValues by it's full path
Comment 33 Andrew Isherwood 2019-10-16 09:03:58 UTC
Created attachment 94264 [details] [review]
Bug 23112: Add CirculateILL syspref

Sponsored-by: Loughborough University
Signed-off-by: Chris Walton <C.J.Walton@lboro.ac.uk>
Comment 34 Andrew Isherwood 2019-10-16 09:04:01 UTC
Created attachment 94265 [details] [review]
Bug 23112: Add circulation to ILL requests

This patch adds the ability to circulate ILL requests. Once a request has a suitable status, a "Check out" button is displayed on the "Manage request" toolbar. Clicking this will enable the user to check out the item either to the user who made the request or an in-house statistical  user. A due date can be specified, but if not circ rules are used.

Prior to the check out, an item is created which is attached to the biblio record that was created when the request was added

Sponsored-by: Loughborough University
Comment 35 Andrew Isherwood 2019-10-16 09:04:05 UTC
Created attachment 94266 [details] [review]
Bug 23112: Add permission checks

We only want the ILL circulation functionality to be available if the
appropriate syspref and perm are available, this patch adds those
checks.

Sponsored-by: Loughborough University
Comment 36 Andrew Isherwood 2019-10-16 09:04:08 UTC
Created attachment 94267 [details] [review]
Bug 23112: Update request status on return

This patch updates a request's status to RET upon return

Sponsored-by: Loughborough University
Comment 37 Andrew Isherwood 2019-10-16 09:04:13 UTC
Created attachment 94268 [details] [review]
Bug 23112: Add unit tests

This patch adds unit test for the new Koha::Illrequest::check_out method

Sponsored-by: Loughborough University
Comment 38 Andrew Isherwood 2019-10-16 09:04:16 UTC
Created attachment 94269 [details] [review]
Bug 23112: (follow-up) Call C4::Koha::GetAuthorisedValues

In response to the problem Magnus was experiencing in comment #28 & #29
we now call GetAuthorisedValues by it's full path