[Bug 13409] New: Add ability to purge old special holidays using cleanup_database.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Bug ID: 13409 Summary: Add ability to purge old special holidays using cleanup_database.pl Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Some libraries have expressed interested in being able to purge old holidays from the database. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34177 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34177&action=edit Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl Some libraries have expressed interested in being able to purge old holidays from the database. Test Plan: 1) Create some special holidays in the past 2) Use the new --holidays DAYS option to test deleting special holidays 3) Verify only special holidays past the given amount of days were deleted -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |aleishaamohia@hotmail.com --- Comment #2 from Aleisha Amohia <aleishaamohia@hotmail.com> --- Patch seems to work cleanly, signing off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 34177 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34177 Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl Review of attachment 34177: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13409&attachment=34177) ----------------------------------------------------------------- ::: misc/cronjobs/cleanup_database.pl @@ +340,5 @@
+sub PurgeHolidays { + $sth = $dbh->prepare( + q{ + DELETE FROM special_holidays + WHERE DATE( CONCAT( year, '-', month, '-', day ) ) < DATE_SUB( CURDATE(), INTERVAL ? DAY )
It would be good to have a DB agnostic way to day that. For instance "DATE_SUB( CURDATE(), INTERVAL ? DAY )" could be done in Perl, don't you think? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #4 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 43588 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43588&action=edit Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl v2 We are having a massive amount of special holidays in the calendar and those are cluttering the calendar views. A simple solution is to remove stale special holidays from the koha.special_holidays-table. Test Plan: 1) Create some special holidays in the past 2) Use the new --calendar DAYS option to test deleting special holidays 3) Verify only special holidays past the given amount of days were deleted P.S. No test included! Do not push without proper integration tests, preferably with Cucumber + PageObject and some kind of a cronjob-runner. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |olli-antti.kivilahti@jns.fi Status|In Discussion |Failed QA --- Comment #5 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi there! I accidentally implemented this simple fix and found Kyle's patch later. Sharing my solution regardless. My solution actually could be tested if somebody decided to write tests for it. Not implementing this with DBIx because it is very counterproductive and I don't feel like spending a day doing an 2-hour job. Failed QA for both commits due to a lack of tests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle.m.hall@gmail.com Attachment #43588|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #6 from Kyle M Hall <kyle.m.hall@gmail.com> --- Thanks Olli! I do like your version ( except for the odd default value ; ). However it does break from the standard way of doing things in cleanup_database. That being said, if you *were* to add a unit test I'd say that would be a worthwhile improvement and I'd be more than happy to deprecate my version and sign off on yours! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34177|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 43608 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43608&action=edit Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl Some libraries have expressed interested in being able to purge old holidays from the database. Test Plan: 1) Create some special holidays in the past 2) Use the new --holidays DAYS option to test deleting special holidays 3) Verify only special holidays past the given amount of days were deleted -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 43609 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43609&action=edit Bug 13409 [QA Followup] - Remove MySQLism -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 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.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43608|0 |1 is obsolete| | Attachment #43609|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 43693 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43693&action=edit Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl Some libraries have expressed interested in being able to purge old holidays from the database. Test Plan: 1) Create some special holidays in the past 2) Use the new --holidays DAYS option to test deleting special holidays 3) Verify only special holidays past the given amount of days were deleted Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 43694 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43694&action=edit Bug 13409 [QA Followup] - Remove MySQLism Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 43695 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43695&action=edit Bug 13409: Remove MySQLism - Replace DATE with CAST AS DATE Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #12 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- (In reply to Kyle M Hall from comment #6)
Thanks Olli! I do like your version ( except for the odd default value ; ). Heh, you should see the odd default values in acquisitions.module. (if don't know what to do order, order one piece of this Biblio)
However it does break from the standard way of doing things in cleanup_database. Erm... The cleanup_database is from the 1990's based on the programming patterns. Why do you need to follow such a standard? Is it too much to ask to expose business logic inside the core API rather than access DB directly from the Controller/script? This is the root of many issues in Koha and we need to break that crappy cycle of untestable/automatable features.
That being said, if you *were* to add a unit test I'd say that would be a worthwhile improvement and I'd be more than happy to deprecate my version and sign off on yours!
Well now nobody can test it, unless they refactor the feature by moving the business logic inside the API. This modification would need to pass yet another community process. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #13 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- I could still fail QA due to missing tests. But back then nobody wrote test, so according to the spirit of cleanup_database.pl this behaviour is acceptable. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43608|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43609|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43693|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43694|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Ok, removing my signoff, obsoleting patches. Please (someone) move the sql to a pm and write tests. Marked as Failed QA. (Everybody's happy?) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #15 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- No. I think everybody is really pissed off :) But I guess we first need to get really pissed and then start thinking on how we can improve ourselves/statuq quo. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 --- Comment #16 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi! I just implemented Bug 10744. This created special holidays for Koha::Calendar to use with the test cases. Oh boy how much fun it would be to be able to remove special holidays calling an internal module instead of writing direct SQL-code in the test script. I wonder where could we find one patch which would introduce such an internal API call? You guys have so many years of Koha development under your hood, I am sure you both know what is best for Koha, and won't fail to keep developing Koha, just as it was developed 10 years ago. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13409 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10744 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10744 [Bug 10744] ExpireReservesMaxPickUpDelay works with hold(s) over report -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org