Description
Martin Renvoize (ashimema)
2019-06-13 07:51:52 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 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 (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. (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. (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 An alternative specification has been sent to the customer and agreed. Can the alternative be shared? There seemed to be agreement on this one (see dicussion on wiki and comments here) It's on the wiki Created attachment 91987 [details] [review] Bug 23122: Add CirculateILL syspref Created attachment 91988 [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 Created attachment 91989 [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. Created attachment 91990 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Created attachment 91991 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method 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 Created attachment 92000 [details] [review] Bug 23122: Add CirculateILL syspref Sponsored-by: Loughborough University Created attachment 92001 [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 Created attachment 92002 [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 Created attachment 92003 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Sponsored-by: Loughborough University Created attachment 92004 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method Sponsored-by: Loughborough University (In reply to Andrew Isherwood from comment #15) > Created attachment 92000 [details] [review] [review] > Bug 23122: Add CirculateILL syspref > > Sponsored-by: Loughborough University There is a typo in the bug number. It should be 23112, not 23122. (In reply to Magnus Enger from comment #20) > (In reply to Andrew Isherwood from comment #15) > > Created attachment 92000 [details] [review] [review] [review] > > Bug 23122: Add CirculateILL syspref > > > > Sponsored-by: Loughborough University > > There is a typo in the bug number. It should be 23112, not 23122. Never mind, looks like I can fix it. Created attachment 93530 [details] [review] Bug 23112: Add CirculateILL syspref Sponsored-by: Loughborough University 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 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 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 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 Thanks for that Magnus, the bug number is now fixed, as it the failure to apply 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? 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. 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. 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. 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 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> 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 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 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 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 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 Created attachment 95565 [details] [review] Bug 23112: Add CirculateILL syspref Sponsored-by: Loughborough University Signed-off-by: Chris Walton <C.J.Walton@lboro.ac.uk> Created attachment 95566 [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 Created attachment 95567 [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 Created attachment 95568 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Sponsored-by: Loughborough University Created attachment 95569 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method Sponsored-by: Loughborough University Created attachment 95570 [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 on attachment 95566 [details] [review] Bug 23112: Add circulation to ILL requests Review of attachment 95566 [details] [review]: ----------------------------------------------------------------- Also, I think you should use patron-title.inc to display patron's info. ::: Koha/Illrequest.pm @@ +1211,5 @@ > + # For some reason, AddIssue requires an unblessed Patron > + $issue_args[0] = $patron->unblessed; > + my $issue = C4::Circulation::AddIssue(@issue_args); > + > + if ($issue && %{$issue}) { AddIssue return a Koha::Checkout object. The test will work but I think it's confusing. ::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ +704,4 @@ > </script> > [% INCLUDE 'ill-list-table-strings.inc' %] > [% Asset.js("js/ill-list-table.js") | $raw %] > + [% Asset.js("js/ill-check-out.js") | $raw %] Where does this file come from? (In reply to Jonathan Druart from comment #45) > Comment on attachment 95566 [details] [review] [review] > Bug 23112: Add circulation to ILL requests > > Review of attachment 95566 [details] [review] [review]: > ----------------------------------------------------------------- > > Also, I think you should use patron-title.inc to display patron's info. Aha, good spot, thanks! I'm about to add a patch to address that. > > ::: Koha/Illrequest.pm > @@ +1211,5 @@ > > + # For some reason, AddIssue requires an unblessed Patron > > + $issue_args[0] = $patron->unblessed; > > + my $issue = C4::Circulation::AddIssue(@issue_args); > > + > > + if ($issue && %{$issue}) { > > AddIssue return a Koha::Checkout object. The test will work but I think it's > confusing. Do you mean the variable name $issue is confusing? If so, I thought it made sense since the function returning it is called AddIssue and, internally, AddIssue also calls the returned value $issue. > > ::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt > @@ +704,4 @@ > > </script> > > [% INCLUDE 'ill-list-table-strings.inc' %] > > [% Asset.js("js/ill-list-table.js") | $raw %] > > + [% Asset.js("js/ill-check-out.js") | $raw %] > > Where does this file come from? Another good spot, this include was leftover junk from an earlier attempt at this bug. About to attach a patch that removes this include. Created attachment 96834 [details] [review] Bug 23112: (follow-up) Address QA feedback As per comment #46 (In reply to Andrew Isherwood from comment #46) > > ::: Koha/Illrequest.pm > > @@ +1211,5 @@ > > > + # For some reason, AddIssue requires an unblessed Patron > > > + $issue_args[0] = $patron->unblessed; > > > + my $issue = C4::Circulation::AddIssue(@issue_args); > > > + > > > + if ($issue && %{$issue}) { > > > > AddIssue return a Koha::Checkout object. The test will work but I think it's > > confusing. > > Do you mean the variable name $issue is confusing? If so, I thought it made > sense since the function returning it is called AddIssue and, internally, > AddIssue also calls the returned value $issue. You should only test for $issue, the "&& %$issue" does not make sense to me. Created attachment 96870 [details] [review] Bug 23112: (follow-up) Fix test as per feedback As per comment #48 https://bugs.koha-community.org/show_bug.cgi?id=23113 (In reply to Jonathan Druart from comment #48) > (In reply to Andrew Isherwood from comment #46) > > > ::: Koha/Illrequest.pm > > > @@ +1211,5 @@ > > > > + # For some reason, AddIssue requires an unblessed Patron > > > > + $issue_args[0] = $patron->unblessed; > > > > + my $issue = C4::Circulation::AddIssue(@issue_args); > > > > + > > > > + if ($issue && %{$issue}) { > > > > > > AddIssue return a Koha::Checkout object. The test will work but I think it's > > > confusing. > > > > Do you mean the variable name $issue is confusing? If so, I thought it made > > sense since the function returning it is called AddIssue and, internally, > > AddIssue also calls the returned value $issue. > > You should only test for $issue, the "&& %$issue" does not make sense to me. I've now attached a patch to address this. I'm puzzled why I did this extra test, it seems like the sort of thing that would be deliberate. Looking at AddIssue, I can't see a case where an empty hashref could be returned, so I'm not sure why I would have added this extra test. Anyway, thanks for pointing it out Jonathan. Created attachment 99910 [details] [review] Bug 23112: Add CirculateILL syspref Sponsored-by: Loughborough University Signed-off-by: Chris Walton <C.J.Walton@lboro.ac.uk> Created attachment 99911 [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 Created attachment 99912 [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 Created attachment 99913 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Sponsored-by: Loughborough University Created attachment 99914 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method Sponsored-by: Loughborough University Created attachment 99915 [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 Created attachment 99916 [details] [review] Bug 23112: (follow-up) Address QA feedback As per comment #46 Created attachment 99917 [details] [review] Bug 23112: (follow-up) Fix test as per feedback As per comment #48 https://bugs.koha-community.org/show_bug.cgi?id=23113 Needed rebase due to dependent patch in bug 23384 changing Created attachment 100100 [details] [review] Bug 23112: Add CirculateILL syspref Sponsored-by: Loughborough University Signed-off-by: Chris Walton <C.J.Walton@lboro.ac.uk> Created attachment 100101 [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 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 Sponsored-by: Loughborough University Created attachment 100102 [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 Created attachment 100103 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Sponsored-by: Loughborough University Created attachment 100104 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method Sponsored-by: Loughborough University Created attachment 100105 [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 Created attachment 100106 [details] [review] Bug 23112: (follow-up) Address QA feedback As per comment #46 Created attachment 100107 [details] [review] Bug 23112: (follow-up) Fix test as per feedback As per comment #48 https://bugs.koha-community.org/show_bug.cgi?id=23113 Created attachment 100108 [details] [review] Bug 23112: (QA follow-up) Fix database update, add filters, remove tabs Looks like the atomicupdate and sysprefs.sql changes were lost at some point. Don't forget to run QA tools on your patches before submitting This all works as expected and is a great enhancement. I added a follow-up to address a few minor issues. The one complaint I have is that if I create a 'chapter' request there is no biblio created but I still get a 'checkout' button on the request, but have no way to attach/create a biblio - we should not show the button in this case Thanks for this Nick, it is much appreciated. Many thanks for the follow up, I do wish my editor would not randomly switch to tabs sometimes! Good point about the Checkout button, I'll create a follow up shortly and re-submit :) Created attachment 100169 [details] [review] Bug 23051: (follow-up) Conditionally prevent checkout As mentioned by Nick in comment #69, the checkout button displays even for requests without a biblio attached (such as chapters). This patch fixes that. Created attachment 100224 [details] [review] Bug 23112: (follow-up) Conditionally prevent checkout As mentioned by Nick in comment #69, the checkout button displays even for requests without a biblio attached (such as chapters). This patch fixes that. Created attachment 100225 [details] [review] Bug 23112: (follow-up) Unit test fixes As requested by Nick, fixes unit tests and added a test for the new Koha::Illrequest::biblio method Created attachment 100252 [details] [review] Bug 23112: Add CirculateILL syspref Sponsored-by: Loughborough University Signed-off-by: Chris Walton <C.J.Walton@lboro.ac.uk> https://bugs.koha-community.org/show_bug.cgi?id=23112 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100253 [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 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 Sponsored-by: Loughborough University Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100254 [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 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100255 [details] [review] Bug 23112: Update request status on return This patch updates a request's status to RET upon return Sponsored-by: Loughborough University Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100256 [details] [review] Bug 23112: Add unit tests This patch adds unit test for the new Koha::Illrequest::check_out method Sponsored-by: Loughborough University Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100257 [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 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100258 [details] [review] Bug 23112: (follow-up) Address QA feedback As per comment #46 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100259 [details] [review] Bug 23112: (follow-up) Fix test as per feedback As per comment #48 https://bugs.koha-community.org/show_bug.cgi?id=23113 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100260 [details] [review] Bug 23112: (QA follow-up) Fix database update, add filters, remove tabs Looks like the atomicupdate and sysprefs.sql changes were lost at some point. Don't forget to run QA tools on your patches before submitting Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100261 [details] [review] Bug 23112: (follow-up) Conditionally prevent checkout As mentioned by Nick in comment #69, the checkout button displays even for requests without a biblio attached (such as chapters). This patch fixes that. https://bugs.koha-community.org/show_bug.cgi?id=23112 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100262 [details] [review] Bug 23112: (follow-up) Unit test fixes As requested by Nick, fixes unit tests and added a test for the new Koha::Illrequest::biblio method Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 100263 [details] [review] Bug 23112: (QA follow-up) Clean up tests, catch warnings Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Nice work everyone! Pushed to master for 20.05 new feature not backported to 19.11.x +use Carp::Always; That should be removed, right? (In reply to Jonathan Druart from comment #88) > +use Carp::Always; > > That should be removed, right? Should we file a new bug? (In reply to Katrin Fischer from comment #89) > (In reply to Jonathan Druart from comment #88) > > +use Carp::Always; > > > > That should be removed, right? > > Should we file a new bug? Yes, Andrew can you deal with it please? |