Bug 25232

Summary: Add ability to skip trapping items with a given notforloan value
Product: Koha Reporter: Kyle M Hall <kyle>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: Pushed to stable --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: aleisha, andrew, bwsdonna, gmcharlt, kelly, kyle.m.hall, lucas, martin.renvoize, sally.healey
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.02
Bug Depends on: 25184    
Bug Blocks:    
Attachments: Bug 25232: Add ability to skip trapping items with a given notforloan value
Bug 25232: Add ability to specify multiple notforloan values to skip
Bug 25232: Add ability to skip trapping items with a given notforloan value
Bug 25232: Add ability to specify multiple notforloan values to skip
Bug 25232: Add ability to skip trapping items with a given notforloan value
Bug 25232: Add ability to specify multiple notforloan values to skip

Description Kyle M Hall 2020-04-21 17:32:47 UTC
This is a companion/alternative to bug 25184, in that it allows an explicit workflow for placing returned books into temporary storage for a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In Decontamination" or something along along those lines. This notforloan value would never be trappable. At the end of decon, UpdateNotForLoanStatusOnCheckin  could be used to remove the notforloan status and allow checkins to be trapped to fill holds.
Comment 1 Kyle M Hall 2020-04-21 17:57:19 UTC
Created attachment 103400 [details] [review]
Bug 25232: Add ability to skip trapping items with a given notforloan value

This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!
Comment 2 Andrew Fuerste-Henry 2020-04-21 21:20:57 UTC
Patch wouldn't apply for me. Got these errors:
error: sha1 information is lacking or useless (C4/Reserves.pm).
error: could not build fake ancestor
Comment 3 Martin Renvoize 2020-05-06 18:41:22 UTC
sha1 error is because the dependency listed isn't actually a dependency but rather an alternative.

I've PQA's the other one, though I like this as a future followup perhaps to enhance the feature.
Comment 4 Andrew Fuerste-Henry 2020-05-06 18:50:10 UTC
If 25184 is moving forward and solving the immediate need, then I'd like to see this get tweaked a little to allow more than one not for loan value to be entered into SkipHoldTrapOnNotForLoanValue.
Comment 5 Katrin Fischer 2020-05-06 21:30:10 UTC
(In reply to Andrew Fuerste-Henry from comment #4)
> If 25184 is moving forward and solving the immediate need, then I'd like to
> see this get tweaked a little to allow more than one not for loan value to
> be entered into SkipHoldTrapOnNotForLoanValue.

That makes sense to me - a list of pipe separated maybe? (bug 25184 just passed QA and has its own pref)
Comment 6 Kyle M Hall 2020-05-07 13:16:56 UTC
Created attachment 104512 [details] [review]
Bug 25232: Add ability to specify multiple notforloan values to skip
Comment 7 Katrin Fischer 2020-05-09 22:28:09 UTC
Thank you, Kyle! Hope to see this in the QA queue soon.
Comment 8 ByWater Sandboxes 2020-06-23 10:28:29 UTC
Created attachment 106198 [details] [review]
Bug 25232: Add ability to skip trapping items with a given notforloan value

This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 9 ByWater Sandboxes 2020-06-23 10:28:32 UTC
Created attachment 106199 [details] [review]
Bug 25232: Add ability to specify multiple notforloan values to skip

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 10 Martin Renvoize 2020-06-23 13:22:24 UTC
Created attachment 106202 [details] [review]
Bug 25232: Add ability to skip trapping items with a given notforloan value

This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-06-23 13:22:27 UTC
Created attachment 106203 [details] [review]
Bug 25232: Add ability to specify multiple notforloan values to skip

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-06-23 13:23:24 UTC
Works well, adds units tests, passes qa scripts, no regressions found.

Passing QA

(I'll leave it to the RM to remove the superfluous comment from the atomicupdate at merge time.)
Comment 13 Jonathan Druart 2020-06-25 09:12:37 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 14 Lucas Gass 2020-07-13 15:48:04 UTC
backported to 20.05.x for 20.05.02
Comment 15 Aleisha Amohia 2020-07-16 04:19:44 UTC
enhancement, not backported to 19.11.x