[Bug 17952] New: Lost items not skipped by overdue_notices.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Bug ID: 17952 Summary: Lost items not skipped by overdue_notices.pl Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com If a library does not use --mark-returned when running longoverdue.pl, all those lost item checkouts are selected by overdue_notices.pl. This causes much unnecessary overhead. In addition Koha::Calendar is instantiated many times for each branchcode which is not necessary. -- 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=17952 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@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=17952 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 59378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59378&action=edit Bug 17952 - Lost items not skipped by overdue_notices.pl If a library does not use --mark-returned when running longoverdue.pl, all those lost item checkouts are selected by overdue_notices.pl. This causes much unnecessary overhead. In addition Koha::Calendar is instantiated many times for each branchcode which is not necessary. Test Plan: 1) Run overdue_notices.pl, note output 2) Apply this patch 3) Run overdue_notices.pl again, note output is the same -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Kyle M Hall <kyle@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=17952 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59378|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 59551 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59551&action=edit Bug 17952 - Lost items not skipped by overdue_notices.pl If a library does not use --mark-returned when running longoverdue.pl, all those lost item checkouts are selected by overdue_notices.pl. This causes much unnecessary overhead. In addition Koha::Calendar is instantiated many times for each branchcode which is not necessary. Test Plan: 1) Run overdue_notices.pl, note output 2) Apply this patch 3) Run overdue_notices.pl again, note output is the same Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jane Leven <jleven@camdencountylibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kyle, could you move the "$calendar" fix to its own bug? That sounds like a major bug as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 18509 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=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #59551|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 62928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62928&action=edit Bug 17952 - Lost items not skipped by overdue_notices.pl If a library does not use --mark-returned when running longoverdue.pl, all those lost item checkouts are selected by overdue_notices.pl. This causes much unnecessary overhead. In addition Koha::Calendar is instantiated many times for each branchcode which is not necessary. Test Plan: 1) Run overdue_notices.pl, note output 2) Apply this patch 3) Run overdue_notices.pl again, note output is the same Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jane Leven <jleven@camdencountylibrary.org> 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=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Martin Renvoize <martin.renvoize@ptfs-europe.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=17952 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, So in an ideal world I'd like to have seen these as separate bugs, but as it's still a fairly trivial change and the Koha::Calendar instantiation bug is fixed in the exact same way as I was intending on bug 18509 then I'm happy to sign off as a whole. Tested, but not definitively benchmarked to prove the improvement. It 'feels' more performant on the tests I ran. Signed off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|martin.renvoize@ptfs-europe |testopia@bugs.koha-communit |.com |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62928|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 62984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62984&action=edit Bug 17952 - Lost items not skipped by overdue_notices.pl If a library does not use --mark-returned when running longoverdue.pl, all those lost item checkouts are selected by overdue_notices.pl. This causes much unnecessary overhead. In addition Koha::Calendar is instantiated many times for each branchcode which is not necessary. Test Plan: 1) Run overdue_notices.pl, note output 2) Apply this patch 3) Run overdue_notices.pl again, note output is the same Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jane Leven <jleven@camdencountylibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #3)
Kyle, could you move the "$calendar" fix to its own bug? That sounds like a major bug as well.
I do not understand my own comment. It does not sound a major bug, just a speed improvement. It would have been good to have it on its own bug anyway :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am not clear on this one - did we calculate fines on lost items before this patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- To clarify, this patch adds two performance enhancements in one patch. a) It prevents re-instantiation of Koha::Calendar objects inside the loop (no functionality change here) b) It enhances the SQL query to skip all lost items during overdue calculation. (this is a change of logic, but I don't think it's a change of functionality) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm, on second read.. it is a change of functionality in so much as listitems were not skipped before. In all my own test cases we have overdue rules set at times long before itemlost marking rules.. so you would never see the change.. I imagine that is the usual case.. hmm.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Yeah, I am mostly worried about pushing a behaviour change to the stable release. We don't use the longoverdues script atm, but I know libraries are manually setting the lost status when an item is reported lost. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |benjamin.rokseth@kul.oslo.k | |ommune.no --- Comment #14 from Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> --- seems a slash snuck into the patch? @@ -466,7 +467,8 @@ SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, branchname AND biblio.biblionumber = biblioitems.biblionumber AND issues.borrowernumber = ? AND issues.branchcode = ? - AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0 + AND items.itemlost = 0 +/ AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0 END_SQL this breaks the query -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17952 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18835 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Benjamin Rokseth from comment #14)
seems a slash snuck into the patch?
@@ -466,7 +467,8 @@ SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, branchname AND biblio.biblionumber = biblioitems.biblionumber AND issues.borrowernumber = ? AND issues.branchcode = ? - AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0 + AND items.itemlost = 0 +/ AND TO_DAYS($date)-TO_DAYS(issues.date_due) >= 0 END_SQL
this breaks the query
This is terrible, I should have caught that. See bug 18835. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18835 [Bug 18835] SQL syntax error in overdue_notices.pl -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org