Bug 11218 - Due notices broken / not generated
Summary: Due notices broken / not generated
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Target Milestone: Oldversions
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-07 21:25 UTC by Katrin Fischer
Modified: 2018-12-16 19:34 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11218: Regression tests for Due notice generation (4.72 KB, patch)
2013-11-07 23:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11218: Due notices not being generated (1.51 KB, patch)
2013-11-07 23:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11218: Regression tests for Due notice generation (6.14 KB, patch)
2013-11-08 01:12 UTC, Liz Rea
Details | Diff | Splinter Review
y# Attachment to Bug 11218 - Due notices broken / not generated (4.76 KB, patch)
2013-11-08 07:07 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 11218: Due notices not being generated (1.60 KB, patch)
2013-11-08 07:08 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration (3.10 KB, patch)
2013-11-08 07:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11218: Regression tests for Due notice generation (4.81 KB, patch)
2013-11-08 14:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11218: Due notices not being generated (1.66 KB, patch)
2013-11-08 14:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11218: QA follow-up - Fixing some whitespace and a variable declaration (1.13 KB, patch)
2013-11-08 14:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12118 - Unnecessary YUI assets included in offline circ manifest (1.60 KB, patch)
2014-04-24 12:55 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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)