Bug 9362

Can you please add a test plan for this?

Current code looks good to me.

Current code is buggy but the proposed patch does not fix correctly the issue.

I did the following test : 

- 1 book to check in 2 days
- 2 books to check in in the past

before applying the patch : 

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
getting upcoming due issues at ../misc/cronjobs/advance_notices.pl line 203.
found 1 issues at ../misc/cronjobs/advance_notices.pl line 205.

I changed the value of "-m" : 0, 1, 2, 3, 4
=> always 1 issue found (the book to check in in 2 days)


after applying the patch

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
found 0 issues
for m = 0, 1, 2 => 0 issues

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=3
found 1 issues
for m = 3,4,5 => 1 issues (the book to check in in 2 days)


Do you consider it is the expected behavior?
If it is so, I can sign off.

Mathieu

Reply to Comment 6 : 
Yes I think it is the expected behavior.

It seems logical, and can probably explain some problem we encountered in Rennes 2 :some patrons complain they receive unwanted predue notice. We thought they were wrong, but maybe they are not...

I sign off.

M. Saby

Created attachment 19599 [details] [review]
Bug 9362 - Wrong query in GetUpcomingDueIssues

C4:Circulation:GetUpcomingDueIssues is used in the advance_notices.pl script.
This script waits for a "maxdays" parameter, which is used in the following request.

We have in GetUpcomingDueIssues the following query:
SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail
FROM issues
LEFT JOIN items USING (itemnumber)
LEFT OUTER JOIN branches USING (branchcode)
WhERE returndate is NULL
AND ( TO_DAYS( NOW() )-TO_DAYS( date_due ) ) < ?

The last line should be with date_due before NOW. The date_due is supposed to be "bigger" (farther) than the NOW date.
Also, this request gives the upcoming due issues, but also the overdues.

This patch corrects this by using HAVING in query.

Test plan :
- Create an issue with a date due in the paste
- Create an issue with a date due in two days
- Launch advance notices with due date in max 2 days : perl misc/cronjobs/advance_notices.pl -c -n -v -m=2
=> You get a warn "found 0 issues"
- Launch advance notices with due date in max 3 days : perl misc/cronjobs/advance_notices.pl -c -n -v -m=3
=> You get a warn "found 1 issues"

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
I did the following test :

- 1 book to check in 2 days
- 2 books to check in in the past

before applying the patch :

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
getting upcoming due issues at ../misc/cronjobs/advance_notices.pl line 203.
found 1 issues at ../misc/cronjobs/advance_notices.pl line 205.

I changed the value of "-m" : 0, 1, 2, 3, 4
=> always 1 issue found (the book to check in in 2 days)

after applying the patch :

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
found 0 issues
for m = 0, 1, 2 => 0 issues

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=3
found 1 issues
for m = 3,4,5 => 1 issues (the book to check in in 2 days)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, works as advertised.

Pushed to master.  Thanks, Fridolyn!

This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks Fridolyn!

This patch is also needed for 3.8.x and 3.10.x versions. Thanks

Pushed to 3.10.x, will be in 3.10.11

Pushed to 3.8.x, will be in 3.8.18

As the advance_notice.pl script also procudes the DUE notices, the assumption that issues due today should not be selected is not correct.