Bug 28139 - Processing holds are not filled automatically
Summary: Processing holds are not filled automatically
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Joonas Kylmälä
QA Contact: Testopia
URL:
Keywords: rel_20_11_candidate
Depends on: 25690 28136
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-13 08:31 UTC by Joonas Kylmälä
Modified: 2021-05-24 15:09 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06


Attachments
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on (1.69 KB, patch)
2021-04-13 08:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 28139: Simplify logic for handling found holds in returns.pl (2.72 KB, patch)
2021-04-13 08:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on (1.75 KB, patch)
2021-04-14 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28139: Simplify logic for handling found holds in returns.pl (2.79 KB, patch)
2021-04-14 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on (1.81 KB, patch)
2021-04-14 15:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28139: Simplify logic for handling found holds in returns.pl (2.84 KB, patch)
2021-04-14 15:46 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-04-13 08:31:23 UTC
If you check in Processing hold and have HoldsAutoFill pref enabled the hold is not filled automatically as they should be according to the HoldsAutoFill syspref.
Comment 1 Joonas Kylmälä 2021-04-13 08:47:14 UTC
Created attachment 119511 [details] [review]
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on

To test:
 1: Turn on HoldsAutoFill setting
 2: Place a item-level hold on item X at Branch A
 3: Make the hold to Processing state:
 $ koha-mysql kohadev
 > select * from reserves; # look up the reserve_id
 > UPDATE reserves SET found = 'P', priority = 0 WHERE reserve_id = XXX;
 4: Check in the item X at Branch A - A pop-up asking confirmation comes
 5: Apply patch & restart plack
 6: Check in the item X again at Branch A - now the hold is confirmed automatically
Comment 2 Joonas Kylmälä 2021-04-13 08:47:17 UTC
Created attachment 119512 [details] [review]
Bug 28139: Simplify logic for handling found holds in returns.pl

We are handling in this if-else block 3 cases:
 - Hold found and waiting
 - Hold found but not waiting AND whether HoldsAutoFill is enabled
 - Hold found but not waiting AND whether HoldsAutoFill disabled

If we simply first handle hold found = Waiting case first then we
don't have to individually list all those other found cases and that
simplifies this code a lot.

To test:
 1. Apply patch
 2. Make sure HoldsAutoFill is disabled
 3. Make item-level hold on branch A
 4. Check-in the item at branch A and you should get pop-up confirming
    the hold, ignore it
 5. Set HoldsAutoFill is enabled
 4. Check-in the item again and you now the hold should have been
    automatically filled
Comment 3 Martin Renvoize 2021-04-14 15:19:29 UTC
Created attachment 119586 [details] [review]
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on

To test:
 1: Turn on HoldsAutoFill setting
 2: Place a item-level hold on item X at Branch A
 3: Make the hold to Processing state:
 $ koha-mysql kohadev
 > select * from reserves; # look up the reserve_id
 > UPDATE reserves SET found = 'P', priority = 0 WHERE reserve_id = XXX;
 4: Check in the item X at Branch A - A pop-up asking confirmation comes
 5: Apply patch & restart plack
 6: Check in the item X again at Branch A - now the hold is confirmed automatically

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2021-04-14 15:19:33 UTC
Created attachment 119587 [details] [review]
Bug 28139: Simplify logic for handling found holds in returns.pl

We are handling in this if-else block 3 cases:
 - Hold found and waiting
 - Hold found but not waiting AND whether HoldsAutoFill is enabled
 - Hold found but not waiting AND whether HoldsAutoFill disabled

If we simply first handle hold found = Waiting case first then we
don't have to individually list all those other found cases and that
simplifies this code a lot.

To test:
 1. Apply patch
 2. Make sure HoldsAutoFill is disabled
 3. Make item-level hold on branch A
 4. Check-in the item at branch A and you should get pop-up confirming
    the hold, ignore it
 5. Set HoldsAutoFill is enabled
 4. Check-in the item again and you now the hold should have been
    automatically filled

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2021-04-14 15:20:32 UTC
Nice work, this makes the code a fair bit clearer and fixes the Processing status as the test plan describes.

Signing off, thanks Joonas
Comment 6 Nick Clemens 2021-04-14 15:46:49 UTC
Created attachment 119590 [details] [review]
Bug 28139: Handle Processing status with HoldsAutoFill pref turned on

To test:
 1: Turn on HoldsAutoFill setting
 2: Place a item-level hold on item X at Branch A
 3: Make the hold to Processing state:
 $ koha-mysql kohadev
 > select * from reserves; # look up the reserve_id
 > UPDATE reserves SET found = 'P', priority = 0 WHERE reserve_id = XXX;
 4: Check in the item X at Branch A - A pop-up asking confirmation comes
 5: Apply patch & restart plack
 6: Check in the item X again at Branch A - now the hold is confirmed automatically

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2021-04-14 15:46:53 UTC
Created attachment 119591 [details] [review]
Bug 28139: Simplify logic for handling found holds in returns.pl

We are handling in this if-else block 3 cases:
 - Hold found and waiting
 - Hold found but not waiting AND whether HoldsAutoFill is enabled
 - Hold found but not waiting AND whether HoldsAutoFill disabled

If we simply first handle hold found = Waiting case first then we
don't have to individually list all those other found cases and that
simplifies this code a lot.

To test:
 1. Apply patch
 2. Make sure HoldsAutoFill is disabled
 3. Make item-level hold on branch A
 4. Check-in the item at branch A and you should get pop-up confirming
    the hold, ignore it
 5. Set HoldsAutoFill is enabled
 4. Check-in the item again and you now the hold should have been
    automatically filled

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Jonathan Druart 2021-04-16 10:30:31 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 9 Fridolin Somers 2021-04-29 09:17:24 UTC
Pushed to 20.11.x for 20.11.06
Comment 10 Andrew Fuerste-Henry 2021-05-24 15:09:15 UTC
Missing dependency, not backported to 20.05