Bug 11445

Summary: it is possible for duplicate hold notifications to be sent
Product: Koha Reporter: Galen Charlton <gmcharlt>
Component: Hold requestsAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, gmcharlt
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 11445: regression test for duplicate hold notifications
Bug 11445: avoid sending duplicate hold waiting notifications
Bug 11445: regression test for duplicate hold notifications
Bug 11445: avoid sending duplicate hold waiting notifications
[PASSED QA] Bug 11445: regression test for duplicate hold notifications
[PASSED QA] Bug 11445: avoid sending duplicate hold waiting notifications

Description Galen Charlton 2013-12-25 17:26:19 UTC
If an item that fills a hold request is checked in twice and the hold is confirmed each time, it is possible for multiple hold notifications to be sent to the patron  even if the item was already marked as waiting on the hold shelf after the first checkin.

This is because ModReserveAffect() includes a call to GetReserveInfo() that was never updated when the holds API was updated to use reserve_id.  Consequently, ModReserveAffect fails to correctly determine if the item was already waiting.
Comment 1 Galen Charlton 2013-12-25 17:48:35 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-12-25 17:48:45 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-12-29 02:40:26 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2013-12-29 02:40:41 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-12-29 07:58:22 UTC
Created attachment 23852 [details] [review]
[PASSED QA] Bug 11445: regression test for duplicate hold notifications

This patch implements a regression test for verifying that
duplicate hold notifications aren't sent if ModReserveAffect() is
called repeatedly (as might happen if a circ operator accidentally
checks in an item and confirms its hold more than once).

Note that the test depends on the fact that _koha_notify_reserve()
defaults to sending a HOLD_PRINT letter if the borrower has not
specified an email or SMS hold notification.

To test:

[1] Run prove -v t/db_dependent/Reserves.t
[2] The 'patron not notified a second time (bug 11445)' test
    should fail.
[3] Apply the main patch and run prove -v t/db_dependent/Reserves.t
    again.  This time all tests should pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 6 Katrin Fischer 2013-12-29 07:58:39 UTC
Created attachment 23853 [details] [review]
[PASSED QA] Bug 11445: avoid sending duplicate hold waiting notifications

This patch fixes a problem where a patron could receive duplicate
hold waiting notifications.  For example, this could happen if a
circ operator checked in an item more than once and confirmed the
same hold each time.

To test:

[1] Set up a test patron that received hold waiting notifications.
[2] Put an item on hold for the patron, then check the item in
    and confirm the hold.  Verify that a hold notification is
    sent (or inspect the message_queue table).
[3] Check the item in again and confirm the hold again.  A duplicate
    hold notification will be generated.
[4] Apply the patch.
[5] Repeat steps 2 and 3.  This time, only one notification should be
    generated.
[6] Verify that prove -v t/db_dependent/Reserves.t passes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Works as described.
Comment 7 Galen Charlton 2013-12-30 15:11:13 UTC
Pushed to master.
Comment 8 Fridolin Somers 2014-01-27 13:21:49 UTC
Patches pushed to 3.14.x, will be in 3.14.3.