[Bug 28158] New: Lost items not charging when marked lost from batch item modification
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Bug ID: 28158 Summary: Lost items not charging when marked lost from batch item modification Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org This is a regression - the functionality works in 20.05 and 20.11 To test: 1 - In the default framework make sure the 952$1 is visible in opac/catalog/editor 2 - Set WhenLostChargeReplacementFee to charge 3 - Find an item with a replacement cost 4 - Check out the item to a patron 5 - Set MarkLostitemsAsReturned to return from batch modification 6 - Batch modify the item 7 - Set the lost status 8 - Item is not returned or charged -- 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=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28209 -- 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=28158 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=28158 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- One for Monday and a fresh brain ;P -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Struggling with this one.. If I add a bunch of warnings in.. I have to trigger the job to run in the foreground (else we throw away the warnings into a black hole).. It all works perfectly.. revert it to using background processing.. the item gets marked as lost.. the return happens.. but the fee is NOT charged! Completely confused.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- I used Koha::Logger to trace the brackground job execution The eval around line 306 seems to catch an error from ModItemFromMarc: [2021/04/28 14:43:28] [WARN] DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /kohadevbox/koha/Koha/Object.pm line 169 main::__ANON__ /kohadevbox/koha/tools/batchMod.pl (328) This does not happen if you force the job to the foreground: 1 - Set up your changes 2 - Inspect and edit the form 3 - Delete the hidden param 'runinbrackground' 4 - Force runinbackground to 0 in the form action: action="batchMod.pl?runinbrackground=0" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Nick Clemens <nick@bywatersolutions.com> 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=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_21_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Investigated a bit here. It's a bit crazy when you read the DBIC_TRACE: BEGIN WORK SAVEPOINT savepoint_0 UPDATE `items` SET `cn_sort` = ?, `itemlost` = ?, `itemlost_on` = ? WHERE ( `itemnumber` = ? ): 'CN_1', '1', '2021-05- 24 12:20:47', '1' RELEASE SAVEPOINT savepoint_0 DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] (then the rollback fails) ROLLBACK rollback ineffective with AutoCommit enabled But my guess is that this is caused by the fork, and more precisely the following statement: $job->progress($i) if $runinbackground; It is retrieving the session (in the DB is SessionStorage is MySQL), and the SAVEPOINT does no longer exist. I confirmed that switching SessionStorage to Memcached and I don't see the RELEASE error. The long-term solution is obviously to implement the (new) background job, but here we need a quick fix. I don't have much to suggest. Either we remove the transaction (and so the correct error handling/rollback), or we remove the progress. We would opt for the latter, removing the transaction is a step backward. This is definitely blocker for the release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #4)
The long-term solution is obviously to implement the (new) background job, but here we need a quick fix.
-.- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=17427 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Another solution is to replace CGI::Session, see bug 17427 comment 0 "If AutoCommit is off, it will commit anyway..." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think drop progress for release, then look at dropping CGI::Session or migrating to Background job for .01 perhaps. Down with CGI baggage.. hehe -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #6)
Another solution is to replace CGI::Session, see bug 17427 comment 0 "If AutoCommit is off, it will commit anyway..."
I rebased the patch there, and it does not fix the problem for us here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Another bad news, it's not only the progress, but also the report. Both $job->progress and $job->set create the problem apparently. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #8)
(In reply to Jonathan Druart from comment #6)
Another solution is to replace CGI::Session, see bug 17427 comment 0 "If AutoCommit is off, it will commit anyway..."
I rebased the patch there, and it does not fix the problem for us here.
Ho wait, this is wrong, I didn't test correctly! It actually fixes the problem! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #10)
(In reply to Jonathan Druart from comment #8)
(In reply to Jonathan Druart from comment #6)
Another solution is to replace CGI::Session, see bug 17427 comment 0 "If AutoCommit is off, it will commit anyway..."
I rebased the patch there, and it does not fix the problem for us here.
Ho wait, this is wrong, I didn't test correctly! It actually fixes the problem!
No, it's not :D getting tired in testing the same things, forgot to comment the STD* closes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121340&action=edit Bug 28158: Remove backgroundjob from batchMod -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This patch is rude and I am open to any other alternatives. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121340|0 |1 is obsolete| | --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121345&action=edit Bug 28158: Remove backgroundjob from batchMod Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121352&action=edit Bug 28158: Remove additional backgroundjob related code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I am considering this ready for signoff, because we are only few days before the release. I am still hopping for a better alternative solution however. This patch removes code related to "extra_headers", coming from bug 19889. I don't understand what it was trying to achieve but I don't think it was working, please confirm if you are familiar with that code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121352|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121353&action=edit Bug 28158: Remove additional backgroundjob related code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|martin.renvoize@ptfs-europe |jonathan.druart+koha@gmail. |.com |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 121366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121366&action=edit Bug 28158: [alternate] Use try/catch instead of eval This could be improved, we could actually handle any exceptions, but switching to 'try/catch' seems to resolve the issue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Nick Clemens from comment #18)
Created attachment 121366 [details] [review] Bug 28158: [alternate] Use try/catch instead of eval
This could be improved, we could actually handle any exceptions, but switching to 'try/catch' seems to resolve the issue
NVM, I can't make the problem happen without this, I am doing something...right/wrong -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121366|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Nick Clemens <nick@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=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121345|0 |1 is obsolete| | --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 121367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121367&action=edit Bug 28158: Remove backgroundjob from batchMod Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <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=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121353|0 |1 is obsolete| | --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 121368 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121368&action=edit Bug 28158: Remove additional backgroundjob related code Signed-off-by: Nick Clemens <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=28158 --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121369 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121369&action=edit Bug 28158: Allow batchMod to run under plack -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28445 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28445 [Bug 28445] Use the task queue for the batch delete and update items tool -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121367|0 |1 is obsolete| | --- Comment #23 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121415 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121415&action=edit Bug 28158: Remove backgroundjob from batchMod == Test plan == 1 - In the default framework make sure the 952$1 is visible in opac & catalog & editor 2 - Set WhenLostChargeReplacementFee to charge 3 - Set MarkLostitemsAsReturned to return from batch modification 4 - Find/edit an item with a replacement cost 5 - Check out the item to a patron 6 - Batch modify the item 7 - Set the lost status 8 - Item is not returned or charged 9 - with the patch, it is Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121368|0 |1 is obsolete| | --- Comment #24 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121416 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121416&action=edit Bug 28158: Remove additional backgroundjob related code Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121369|0 |1 is obsolete| | --- Comment #25 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121417 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121417&action=edit Bug 28158: Allow batchMod to run under plack Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #26 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Still works as expected. Added test plan to 1st commit. No grasp of the code, though that's still a signoff that allows Nick or someone else to QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- I think I agree with Jonathan here, this fix is not the best, I expect we might see some timeouts on large batches, but this is better than silently failing to charge as expected. Moving my SO to PQA, feel free to ask for another pair of eyes Jonathan -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I agree with Nick.. I've been testing it yesterday and so far it all worked reasonably well, but I've not had a massive batch to play with yet.. I'll add my QA SO lines this morning after the school run ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #29 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 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=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_21_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00 |21.05.00,20.11.07 released in| | Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #30 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 --- Comment #31 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Fridolin Somers from comment #30)
Pushed to 20.11.x for 20.11.07
I wouldn't have backported this, it's a big change. Moreover Nick reported that it was working on 20.11. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to stable |RESOLVED --- Comment #32 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (20.05.x). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.07 |21.05.00 released in| | --- Comment #33 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Jonathan Druart from comment #31)
(In reply to Fridolin Somers from comment #30)
Pushed to 20.11.x for 20.11.07
I wouldn't have backported this, it's a big change. Moreover Nick reported that it was working on 20.11.
OK I revert. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28158 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28745 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28745 [Bug 28745] Batch item modifications no longer displayed modified items -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org