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!

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

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.

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.

(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)

Created attachment 104512 [details] [review]
Bug 25232: Add ability to specify multiple notforloan values to skip

Thank you, Kyle! Hope to see this in the QA queue soon.

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>

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>

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>

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>

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.)

Pushed to master for 20.11, thanks to everybody involved!

backported to 20.05.x for 20.05.02

enhancement, not backported to 19.11.x

Opening up a new report for two nice perl errors here that seem to compensate one another at least in some cases.

Noting that the English word "trap" confuses me largely in understanding what these preferences do as a non-native speaker.

(In reply to Marcel de Rooy from comment #17)
> Noting that the English word "trap" confuses me largely in understanding
> what these preferences do as a non-native speaker.

Trying to improve description a bit.