After updating a library we found due notices (send on the date a book is due) are no longer being generated. This bug is in 3.12.5 and I confirmed it's also a valid bug in master.
As it worked before I tried tracking the problem down with git bisect - it came up with the changes from bug 9362 which looks plausible.
Created attachment 22795 [details] [review] Bug 11218: Regression tests for Due notice generation Changes tests for GetUpcomingDueIssues: days_in_advance should be inclusive: 2 should find items due in 2 days from now. 1 should find items due tomorrow. 0 should only find items due today. Adds new tests for Bug 11218: As the advance_notices.pl, the only script using GetUpcomingDueIssues is not only used for PREDUE notices, but also for DUE notices, we need to also select items due today.
Created attachment 22796 [details] [review] Bug 11218: Due notices not being generated To test: - Check out 2 items, due dates should be today, sometime in the past and one day after tomorrow. - Edit the message options - activate DUE and PREDUE notices with days in advance = 2 - Run the advance_notices.pl script with -v -c Result: Only a PREDUE notice is generated - Run the advance_notices.pl script with -v -c -m 2 Result: Only the PREDUE message is generated correctly. - Run t/db_dependent/Circulation.t - without first patch: all tests pass. - with first patch: some tests will fail. Apply patch. - Rerun script, now PREDUE and DUE notices should be generated. - Run t/db_dependent/Circulation.t again, all tests should pass. Add more items with different due dates, rerun and check results. Run t/Circulation.t to confirm all tests pass. - Apply the patch
In my tests this seems to work now with the patches applied, please test thorougly and feedback welcome.
Created attachment 22799 [details] [review] Bug 11218: Regression tests for Due notice generation Changes tests for GetUpcomingDueIssues: days_in_advance should be inclusive: 2 should find items due in 2 days from now. 1 should find items due tomorrow. 0 should only find items due today. Adds new tests for Bug 11218: As the advance_notices.pl, the only script using GetUpcomingDueIssues is not only used for PREDUE notices, but also for DUE notices, we need to also select items due today. Bug 11218: Due notices not being generated To test: - Check out 2 items, due dates should be today, sometime in the past and one day after tomorrow. - Edit the message options - activate DUE and PREDUE notices with days in advance = 2 - Run the advance_notices.pl script with -v -c Result: Only a PREDUE notice is generated - Run the advance_notices.pl script with -v -c -m 2 Result: Only the PREDUE message is generated correctly. - Run t/db_dependent/Circulation.t - without first patch: all tests pass. - with first patch: some tests will fail. Apply patch. - Rerun script, now PREDUE and DUE notices should be generated. - Run t/db_dependent/Circulation.t again, all tests should pass. Add more items with different due dates, rerun and check results. Run t/Circulation.t to confirm all tests pass. - Apply the patch Signed-off-by: Liz Rea <liz@catalyst.net.nz> Passes functional and automated testing. Due notice functionality restored. Thanks Katrin!
Created attachment 22802 [details] [review] y# Attachment to Bug 11218 - Due notices broken / not generated Bug 11218: Regression tests for Due notice generation Changes tests for GetUpcomingDueIssues: days_in_advance should be inclusive: 2 should find items due in 2 days from now. 1 should find items due tomorrow. 0 should only find items due today. Adds new tests for Bug 11218: As the advance_notices.pl, the only script using GetUpcomingDueIssues is not only used for PREDUE notices, but also for DUE notices, we need to also select items due today. Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Created attachment 22803 [details] [review] Bug 11218: Due notices not being generated To test: - Check out 2 items, due dates should be today, sometime in the past and one day after tomorrow. - Edit the message options - activate DUE and PREDUE notices with days in advance = 2 - Run the advance_notices.pl script with -v -c Result: Only a PREDUE notice is generated - Run the advance_notices.pl script with -v -c -m 2 Result: Only the PREDUE message is generated correctly. - Run t/db_dependent/Circulation.t - without first patch: all tests pass. - with first patch: some tests will fail. Apply patch. - Rerun script, now PREDUE and DUE notices should be generated. - Run t/db_dependent/Circulation.t again, all tests should pass. Add more items with different due dates, rerun and check results. Run t/Circulation.t to confirm all tests pass. - Apply the patch Signed-off-by: Liz Rea <liz@catalyst.net.nz> Passes functional tests and automated tests.
Created attachment 22804 [details] [review] Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration Fixing some things the QA script pointed out this morning: - whitespace - variable declaration
Created attachment 22823 [details] [review] Bug 11218: Regression tests for Due notice generation Changes tests for GetUpcomingDueIssues: days_in_advance should be inclusive: 2 should find items due in 2 days from now. 1 should find items due tomorrow. 0 should only find items due today. Adds new tests for Bug 11218: As the advance_notices.pl, the only script using GetUpcomingDueIssues is not only used for PREDUE notices, but also for DUE notices, we need to also select items due today. Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 22824 [details] [review] Bug 11218: Due notices not being generated To test: - Check out 2 items, due dates should be today, sometime in the past and one day after tomorrow. - Edit the message options - activate DUE and PREDUE notices with days in advance = 2 - Run the advance_notices.pl script with -v -c Result: Only a PREDUE notice is generated - Run the advance_notices.pl script with -v -c -m 2 Result: Only the PREDUE message is generated correctly. - Run t/db_dependent/Circulation.t - without first patch: all tests pass. - with first patch: some tests will fail. Apply patch. - Rerun script, now PREDUE and DUE notices should be generated. - Run t/db_dependent/Circulation.t again, all tests should pass. Add more items with different due dates, rerun and check results. Run t/Circulation.t to confirm all tests pass. - Apply the patch Signed-off-by: Liz Rea <liz@catalyst.net.nz> Passes functional tests and automated tests. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 22825 [details] [review] Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration Fixing some things the QA script pointed out this morning: - whitespace - variable declaration Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, t and xt. Great unit tests!
Pushed to master and 3.14.x. Thanks, Katrin!
Will be needed in 3.12, 3.10 and 3.8. Thanks !
This patch has been pushed to 3.12.x, will be in 3.12.7. Thanks Katrina!
Created attachment 27531 [details] [review] Bug 12118 - Unnecessary YUI assets included in offline circ manifest The offline circulation asset manifest includes some YUI JavaScript files which are no longer used by any template in Koha except for acquisitions/basketgroup.tt. This patch removes them from the manifest. To test, apply the patch and test offline circulation. Everything should work correctly. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>