Bug 11218

Summary: Due notices broken / not generated
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: NoticesAssignee: Katrin Fischer <katrin.fischer>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: chris, gmcharlt, kyle, liz, sophie.meynieux, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9362
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 11218: Regression tests for Due notice generation
Bug 11218: Due notices not being generated
Bug 11218: Regression tests for Due notice generation
y# Attachment to Bug 11218 - Due notices broken / not generated
Bug 11218: Due notices not being generated
Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration
Bug 11218: Regression tests for Due notice generation
Bug 11218: Due notices not being generated
Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration
Bug 12118 - Unnecessary YUI assets included in offline circ manifest

Description Katrin Fischer 2013-11-07 21:25:36 UTC
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.
Comment 1 Katrin Fischer 2013-11-07 21:27:07 UTC
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.
Comment 2 Katrin Fischer 2013-11-07 23:43:33 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2013-11-07 23:44:10 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2013-11-07 23:47:06 UTC
In my tests this seems to work now with the patches applied, please test thorougly and feedback welcome.
Comment 5 Liz Rea 2013-11-08 01:12:50 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2013-11-08 07:07:15 UTC Comment hidden (obsolete)
Comment 7 Liz Rea 2013-11-08 07:08:52 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2013-11-08 07:39:26 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2013-11-08 14:35:55 UTC
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>
Comment 10 Kyle M Hall 2013-11-08 14:36:01 UTC
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>
Comment 11 Kyle M Hall 2013-11-08 14:36:06 UTC
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>
Comment 12 Kyle M Hall 2013-11-08 14:37:25 UTC
Passes koha-qa.pl, t and xt. Great unit tests!
Comment 13 Galen Charlton 2013-11-08 16:26:19 UTC
Pushed to master and 3.14.x.  Thanks, Katrin!
Comment 14 Sophie MEYNIEUX 2013-11-18 16:10:52 UTC
Will be needed in 3.12, 3.10 and 3.8. Thanks !
Comment 15 Tomás Cohen Arazi 2013-11-18 17:25:09 UTC
This patch has been pushed to 3.12.x, will be in 3.12.7.

Thanks Katrina!
Comment 16 Chris Cormack 2014-04-24 12:55:32 UTC Comment hidden (obsolete)