[Bug 26208] New: Overdues restrictions not consistently removed when renewing multiple items at once
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Bug ID: 26208 Summary: Overdues restrictions not consistently removed when renewing multiple items at once Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com To recreate: - have multiple overdue items on one patron - run overdues.pl with triggers set to generate a restriction - renew all overdues with the Renew All button - restriction is not removed even though patron no longer has overdue items I did not get this error with 5 overdues, but I did get it with 10. Seems like it's due to some messy sequencing when sending a bunch of renewals at once, so it may be inconsistent to recreate. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 madamyk@ckls.org changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madamyk@ckls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Amber Wilde <amber.wilde@cityofspearfish.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amber.wilde@cityofspearfish | |.com --- Comment #1 from Amber Wilde <amber.wilde@cityofspearfish.com> --- We have found that the overdues restriction is not automatically clearing even after using renew all on just two overdue items on an account. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 114165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114165&action=edit Bug 26208: Perform batch checkin/renewals serially rather than asynchronously The issue here seems to be that when multiple requests hit at once they may not register that the renewal performed by the request should lift restrictions on the account. To mitigate this we can simply perform the renewals one after the other. To test: 1 - have multiple overdue items on one patron 2 - run overdues.pl with triggers set to generate a restriction 3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items 5 - Apply patch 6 - Make all items overdue again 7 - Click Renew All 8 - Items are renewed and restriction removed 9 - Checkout items to patron again (overdue or not) 10 - Click 'Select all' in checkin column 11 - Click 'Renew/Checkin selected items' 12 - Confirm checkin succeeds as before patches -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26457 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- This sounds like an anti-feature to me. Nick's note on Bug 26457 says that renewals are silently failing. I'd say it would be better to make them fail loudly rather than making this work serially? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to David Cook from comment #3)
This sounds like an anti-feature to me.
Nick's note on Bug 26457 says that renewals are silently failing. I'd say it would be better to make them fail loudly rather than making this work serially?
In these cases with restrictions there are no failures, just a race condition: # Remove any OVERDUES related debarment if the borrower has no overdues if ( $patron && $patron->is_debarred && ! $patron->has_overdues && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } ) { DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); } When all the transactions hit at once they are getting conflicting information and not removing the debarment, so all items renewed but restrictions stay I filed bug 27152, but currently this is a bug affecting many sites and this is a simple solution. There is a performance cost, but there will be with a bulk endpoint too -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- (In reply to Nick Clemens from comment #4)
In these cases with restrictions there are no failures, just a race condition: # Remove any OVERDUES related debarment if the borrower has no overdues if ( $patron && $patron->is_debarred && ! $patron->has_overdues && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } ) { DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); }
When all the transactions hit at once they are getting conflicting information and not removing the debarment, so all items renewed but restrictions stay
Ok that's interesting. So the last 2 renewals for overdues will be happening around the same time, so $patron->has_overdues will be true for both. Yeah that's not good. Thanks for the clarification.
I filed bug 27152, but currently this is a bug affecting many sites and this is a simple solution. There is a performance cost, but there will be with a bulk endpoint too
I would say that a bulk endpoint could actually have less of a performance cost, because with a bulk endpoint you could do less database I/O by fetching all the database data with 1 query. Although I guess with our use of DBIC we don't really do that as we currently prioritize convenience over performance. Anyway, food for thought. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com --- Comment #6 from Kelly McElligott <kelly@bywatersolutions.com> --- Nick, I am going through the test plan, and with the patch, the "renew all" does not remove the overdue restriction on the patron account. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114165|0 |1 is obsolete| | --- Comment #7 from Kelly McElligott <kelly@bywatersolutions.com> --- Created attachment 114241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114241&action=edit Bug 26208: Perform batch checkin/renewals serially rather than asynchronously The issue here seems to be that when multiple requests hit at once they may not register that the renewal performed by the request should lift restrictions on the account. To mitigate this we can simply perform the renewals one after the other. To test: 1 - have multiple overdue items on one patron 2 - run overdues.pl with triggers set to generate a restriction 3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items 5 - Apply patch 6 - Make all items overdue again 7 - Click Renew All 8 - Items are renewed and restriction removed 9 - Checkout items to patron again (overdue or not) 10 - Click 'Select all' in checkin column 11 - Click 'Renew/Checkin selected items' 12 - Confirm checkin succeeds as before patches -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Margaret Hade <margaret@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |margaret@bywatersolutions.c | |om -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114241|0 |1 is obsolete| | --- Comment #8 from Kelly McElligott <kelly@bywatersolutions.com> --- Created attachment 114440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114440&action=edit Bug 26208: Perform batch checkin/renewals serially rather than asynchronously The issue here seems to be that when multiple requests hit at once they may not register that the renewal performed by the request should lift restrictions on the account. To mitigate this we can simply perform the renewals one after the other. To test: 1 - have multiple overdue items on one patron 2 - run overdues.pl with triggers set to generate a restriction 3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items 5 - Apply patch 6 - Make all items overdue again 7 - Click Renew All 8 - Items are renewed and restriction removed 9 - Checkout items to patron again (overdue or not) 10 - Click 'Select all' in checkin column 11 - Click 'Renew/Checkin selected items' 12 - Confirm checkin succeeds as before patches Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #9 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Hi, trying to do a second signoff. overdue_notices.pl + the right trigger generated a restriction as expected. But then
3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items
The restriction is removed without the patch being applied. No idea what when wrong, retried and same result. Database is just the fresh sample data. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #9)
Hi, trying to do a second signoff.
overdue_notices.pl + the right trigger generated a restriction as expected.
But then
3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items
The restriction is removed without the patch being applied. No idea what when wrong, retried and same result. Database is just the fresh sample data.
Hi Victor, How many overdues did you have? Try with more? It's random failure from asynchronous renewals as far as I can tell -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hc@interleaf.ie --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 27435 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 114440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114440 Bug 26208: Perform batch checkin/renewals serially rather than asynchronously Review of attachment 114440: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=26208&attachment=114440) ----------------------------------------------------------------- Generally, I'm happy with this, I can't find any real regressions and it appears to resolve the issue. However, we introduce a few debugging lines which should really be removed before it can pass QA ::: koha-tmpl/intranet-tmpl/prog/js/checkouts.js @@ +182,5 @@
+ success: function( data ) { + var id = "#renew_" + data.itemnumber; + console.log( data); + console.log( data.renew_okay); + console.log( data.itemnumber);
Can we removed/comment these console.log() statements? @@ +211,3 @@
} + console.log(content); + console.log(id);
and another couple... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 115681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115681&action=edit Bug 26208: (follow-up) Remove debugging statements -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27707 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114440|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117162 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117162&action=edit Bug 26208: Perform batch checkin/renewals serially rather than asynchronously The issue here seems to be that when multiple requests hit at once they may not register that the renewal performed by the request should lift restrictions on the account. To mitigate this we can simply perform the renewals one after the other. To test: 1 - have multiple overdue items on one patron 2 - run overdues.pl with triggers set to generate a restriction 3 - renew all overdues with the Renew All button 4 - restriction is not removed even though patron no longer has overdue items 5 - Apply patch 6 - Make all items overdue again 7 - Click Renew All 8 - Items are renewed and restriction removed 9 - Checkout items to patron again (overdue or not) 10 - Click 'Select all' in checkin column 11 - Click 'Renew/Checkin selected items' 12 - Confirm checkin succeeds as before patches Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115681|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117163&action=edit Bug 26208: (follow-up) Remove debugging statements Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks Nick, Fixes issue and no regressions found. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- On master, follow-up squashed with first patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|21.05.00 |21.05.00,20.11.04 released in| | --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.04 |21.05.00,20.11.04,20.05.10 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED --- Comment #21 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Can't backport to 19.11.x: can't solve conflicts without risking messing up stuff. If there is an interest in having this backported, please submit a patch for 19.11. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26208 Michael Adamyk <madamyk@ckls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|madamyk@ckls.org | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org