Some libraries have expressed interested in being able to purge old holidays from the database.
Created attachment 34177 [details] [review] 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
Patch seems to work cleanly, signing off.
Comment on attachment 34177 [details] [review] Bug 13409 - Add ability to purge old special holidays using cleanup_database.pl Review of attachment 34177 [details] [review]: ----------------------------------------------------------------- ::: 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?
Created attachment 43588 [details] [review] 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.
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.
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!
Created attachment 43608 [details] [review] 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
Created attachment 43609 [details] [review] Bug 13409 [QA Followup] - Remove MySQLism
Created attachment 43693 [details] [review] 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>
Created attachment 43694 [details] [review] Bug 13409 [QA Followup] - Remove MySQLism Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 43695 [details] [review] Bug 13409: Remove MySQLism - Replace DATE with CAST AS DATE Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(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.
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.
Ok, removing my signoff, obsoleting patches. Please (someone) move the sql to a pm and write tests. Marked as Failed QA. (Everybody's happy?)
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.
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.
Can we get this moving again?
*** This bug has been marked as a duplicate of bug 16672 ***