Bug 15666 - Automatic_renewals.pl fails when OverduesBlockRenewing and RestrictionBlockRenewing are set to "block"
Summary: Automatic_renewals.pl fails when OverduesBlockRenewing and RestrictionBlockRe...
Status: RESOLVED DUPLICATE of bug 14395
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-26 10:49 UTC by Claudio Costales
Modified: 2016-08-29 00:05 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Automatic_renewals failed (139.98 KB, application/pdf)
2016-01-27 10:30 UTC, Claudio Costales
Details
No renewal before test (283.73 KB, application/pdf)
2016-02-05 18:36 UTC, Claudio Costales
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Claudio Costales 2016-01-26 10:49:16 UTC
1) Now, when automatic_renewals.pl tries to renew due items the sysprefs vars OverduesBlockRenewing and RestrictionBlockRenewing don't allow renewing. That's right for these sysprefs vars but not for renewing.

2) Automatic_renewals.pl should work like a user should: must renew his issues the date they due.

3) Thus automatic_renewals.pl should always avoid loans expire. So automatic_renewals.pl must consider only issues which due date is equal to the date automatic_renewals.pl is executing. 

4) I think that CanBookBeRenewed in Circulation.pm must be updated but do not how...

5) In automatic_renewals.pl it is needed to change the mysql command to this one:

SELECT borrowernumber, itemnumber, branchcode FROM issues WHERE auto_renew = 1 AND DATE(date_due) = DATE(NOW())



------------------
I will attach a sample soon...
Comment 1 Katrin Fischer 2016-01-26 12:32:49 UTC
Hi Bondiurbano, 

> 1) Now, when automatic_renewals.pl tries to renew due items the sysprefs
> vars OverduesBlockRenewing and RestrictionBlockRenewing don't allow
> renewing. That's right for these sysprefs vars but not for renewing.

Do they only block when renewal would be blocked in the OPAC too or did the new feature break the auto renewals?

> 
> 2) Automatic_renewals.pl should work like a user should: must renew his
> issues the date they due.
> 
> 3) Thus automatic_renewals.pl should always avoid loans expire. So
> automatic_renewals.pl must consider only issues which due date is equal to
> the date automatic_renewals.pl is executing. 

I think 2) and 3) are not how this feature is supposed to work. 
The automatic renewal cronjob takes the "No renewal before" value from the circulation conditions into account. So you can say: Try to renew for the first time X days before the due date. This means that you can also time a reminder notice to the same day: 
Try to renew 5 days before the due date - if renewal fails, the advance_notices.pl job can run after and send a reminder. If it was always the day of the due date - you couldn't send a reminder and things would go overdue immediately instead 

> 4) I think that CanBookBeRenewed in Circulation.pm must be updated but do
> not how...

Can you explain?

> 5) In automatic_renewals.pl it is needed to change the mysql command to this
> one:
> 
> SELECT borrowernumber, itemnumber, branchcode FROM issues WHERE auto_renew =
> 1 AND DATE(date_due) = DATE(NOW())

I disagree - see above.
Comment 2 Claudio Costales 2016-01-26 13:13:50 UTC
(In reply to Katrin Fischer from comment #1)
> Hi Bondiurbano, 
> 
> > 1) Now, when automatic_renewals.pl tries to renew due items the sysprefs
> > vars OverduesBlockRenewing and RestrictionBlockRenewing don't allow
> > renewing. That's right for these sysprefs vars but not for renewing.
> 
> Do they only block when renewal would be blocked in the OPAC too or did the
> new feature break the auto renewals?

The problem occurs when OverduesBlockRenewing is set to "block". This syspref var is a recent enhancement. I agree how it works. But now they is also blocking automatic renewals and this is not correct. I think that when OverduesBlockRenewing was added as a control var in syspref nobody of us took into account this.

> 
> > 
> > 2) Automatic_renewals.pl should work like a user should: must renew his
> > issues the date they due.
> > 
> > 3) Thus automatic_renewals.pl should always avoid loans expire. So
> > automatic_renewals.pl must consider only issues which due date is equal to
> > the date automatic_renewals.pl is executing. 
> 
> I think 2) and 3) are not how this feature is supposed to work. 
> The automatic renewal cronjob takes the "No renewal before" value from the
> circulation conditions into account. So you can say: Try to renew for the
> first time X days before the due date. This means that you can also time a
> reminder notice to the same day: 
> Try to renew 5 days before the due date - if renewal fails, the
> advance_notices.pl job can run after and send a reminder. If it was always
> the day of the due date - you couldn't send a reminder and things would go
> overdue immediately instead 

"No renewal before" is an optional setting so it could be absent in the rule. Automatic_renewals.pl must be executed BEFORE the item date_due expires. Otherwise it will be a due item and so OverduesBlockRenewing will block it. This is my point. Apart from "No renewal before" settings, automatic_renewals.pl works as a "replacement" of a manual action by a user or Staff. I'm testing now. ASAP I will attach a sample that hope helps better than my poor english...

> 
> > 4) I think that CanBookBeRenewed in Circulation.pm must be updated but do
> > not how...
> 
> Can you explain?
> 
> > 5) In automatic_renewals.pl it is needed to change the mysql command to this
> > one:
> > 
> > SELECT borrowernumber, itemnumber, branchcode FROM issues WHERE auto_renew =
> > 1 AND DATE(date_due) = DATE(NOW())
> 
> I disagree - see above.
Comment 3 Katrin Fischer 2016-01-26 13:23:33 UTC
It's true that it should try before the item is due - usually. But for example this could happen: if there is a hold on the item, it will prevent renewal, the item will go overdue... the hold might be cancelled. Then you will have a renewable item that is also overdue. The Settings in the system prefrences should apply then to decide if it can be renewed or not.

I think if the "No renewal before" is empty, we could agree that it should try on the due date - but witht he parameter set the behaviour should be changed.
Comment 4 Claudio Costales 2016-01-27 10:30:01 UTC
Created attachment 47328 [details]
Automatic_renewals failed

Test
Comment 5 Claudio Costales 2016-01-27 10:32:08 UTC
(In reply to Katrin Fischer from comment #3)
> It's true that it should try before the item is due - usually. But for
> example this could happen: if there is a hold on the item, it will prevent
> renewal, the item will go overdue... the hold might be cancelled. Then you
> will have a renewable item that is also overdue. The Settings in the system
> prefrences should apply then to decide if it can be renewed or not.

What you are describing may happen with manual renewals. Please see the test-case I have attached: except this case I think all works fine.


> 
> I think if the "No renewal before" is empty, we could agree that it should
> try on the due date - but witht he parameter set the behaviour should be
> changed.
Comment 6 Katrin Fischer 2016-01-27 11:52:36 UTC
Looking at your example - I think what happened is:

The item was checked out on the 26th with due date 26th, but after the auto_renewal job was run for that day.

On the 27th in the morning the job was run again, but the item was overdue by then - so renewal was blocked.

That makes sense to me to be honest. I am not sure how to avoid that.

> I think if the "No renewal before" is empty, we could agree that it should
> try on the due date - but witht he parameter set the behaviour should be
> changed.

NOT be changed... :)
Comment 7 Claudio Costales 2016-01-29 14:28:05 UTC
(In reply to Katrin Fischer from comment #6)
> Looking at your example - I think what happened is:
> 
> The item was checked out on the 26th with due date 26th, but after the
> auto_renewal job was run for that day.
> 
> On the 27th in the morning the job was run again, but the item was overdue
> by then - so renewal was blocked.
> 
> That makes sense to me to be honest. I am not sure how to avoid that.
> 
> > I think if the "No renewal before" is empty, we could agree that it should
> > try on the due date - but witht he parameter set the behaviour should be
> > changed.
> 
> NOT be changed... :)

Katrin, I have finished my tests. My conclusion is that automatic_renewals.pl works fine if "No renewals before" is informed in the issuing rules, i.e., "No renewals before" is mandatory for that automatic_renewals.pl works fine. Otherwise (my case) it doesn't update the due date of the loan.
I will update my internal procedures to adopt this approach. Thanks for you patience.
Comment 8 Holger Meißner 2016-02-02 09:22:55 UTC
(In reply to bondiurbano from comment #2)
> "No renewal before" is an optional setting so it could be absent in the
> rule.

Yes, but bug 14101 introduced a fallback. If the setting is absent (undef) then automatic renewal works just like "No renewal before" was zero. Do you get different results with undef and zero??

> Automatic_renewals.pl must be executed BEFORE the item date_due
> expires.
I think this cannot be done with a cron job running just once or a few times per day. Due date might be any time of the day, especially with hourly loans.

> Otherwise it will be a due item and so OverduesBlockRenewing will
> block it.
Yes, but I suppose that is exactly what should happen, if the patron has both automatic and manual loans and some of the manual loans are overdue?

But I see the problem! It should renew if ONLY automatic renewals are overdue! So maybe that's the solution. Check if all overdue loans are automatic, then override the block?

I need to think about it some more, but does think this makes sense? :)
Comment 9 Claudio Costales 2016-02-02 12:08:57 UTC
(In reply to Holger Meißner from comment #8)
> (In reply to bondiurbano from comment #2)
> > "No renewal before" is an optional setting so it could be absent in the
> > rule.
> 
> Yes, but bug 14101 introduced a fallback. If the setting is absent (undef)
> then automatic renewal works just like "No renewal before" was zero. Do you
> get different results with undef and zero??
> 
> > Automatic_renewals.pl must be executed BEFORE the item date_due
> > expires.
> I think this cannot be done with a cron job running just once or a few times
> per day. Due date might be any time of the day, especially with hourly loans.
> 
> > Otherwise it will be a due item and so OverduesBlockRenewing will
> > block it.
> Yes, but I suppose that is exactly what should happen, if the patron has
> both automatic and manual loans and some of the manual loans are overdue?
> 
> But I see the problem! It should renew if ONLY automatic renewals are
> overdue! So maybe that's the solution. Check if all overdue loans are
> automatic, then override the block?
> 
> I need to think about it some more, but does think this makes sense? :)

Hi Holger! I will check the fallback. Give a couple of days...I think that the fallback definition is right. But I will check. Meanwhile you can see my attached PDF.
Comment 10 Claudio Costales 2016-02-05 18:35:30 UTC
(In reply to Holger Meißner from comment #8)
> (In reply to bondiurbano from comment #2)
> > "No renewal before" is an optional setting so it could be absent in the
> > rule.
> 
> Yes, but bug 14101 introduced a fallback. If the setting is absent (undef)
> then automatic renewal works just like "No renewal before" was zero. Do you
> get different results with undef and zero??
> 
> > Automatic_renewals.pl must be executed BEFORE the item date_due
> > expires.
> I think this cannot be done with a cron job running just once or a few times
> per day. Due date might be any time of the day, especially with hourly loans.
> 
> > Otherwise it will be a due item and so OverduesBlockRenewing will
> > block it.
> Yes, but I suppose that is exactly what should happen, if the patron has
> both automatic and manual loans and some of the manual loans are overdue?
> 
> But I see the problem! It should renew if ONLY automatic renewals are
> overdue! So maybe that's the solution. Check if all overdue loans are
> automatic, then override the block?
> 
> I need to think about it some more, but does think this makes sense? :)


Holger, I've attached a PDF file with 3 days test. Hope this helps.
Comment 11 Claudio Costales 2016-02-05 18:36:57 UTC
Created attachment 47715 [details]
No renewal before test
Comment 12 Claudio Costales 2016-02-12 11:50:04 UTC
.
Comment 13 Holger Meißner 2016-02-12 14:43:42 UTC
Mh. I studied the second PDF. But I still have to take a closer look at those blocking sysprefs.

A quick question: Did you apply bug 14395 before testing? What was your setting for syspref NoRenewalBeforePrecision?
Comment 14 Claudio Costales 2016-02-12 14:49:59 UTC
(In reply to Holger Meißner from comment #13)
> Mh. I studied the second PDF. But I still have to take a closer look at
> those blocking sysprefs.
> 
> A quick question: Did you apply bug 14395 before testing? What was your
> setting for syspref NoRenewalBeforePrecision?

Hi Holger! I did not apply bug 14395 before testing. If you agree I can do that this weekend.
Comment 15 Holger Meißner 2016-02-12 15:25:40 UTC
Hi bondiurbano, please take your time. Don't work on the weekend.

When you do test again, please include when exactly the cronjob is scheduled, or if you run automatic_renewals.pl manually.
Comment 16 Claudio Costales 2016-02-13 12:25:57 UTC
(In reply to Holger Meißner from comment #15)
> Hi bondiurbano, please take your time. Don't work on the weekend.
> 
> When you do test again, please include when exactly the cronjob is
> scheduled, or if you run automatic_renewals.pl manually.

The test was done in a production environment, in a "demo" installation of my own. Crons run daily at 4:00am.

I will follow your suggestion and will not work on this weekend. But yerterday I've done a quick test on a local virtual machine, applying bug 14395 and the results were the same: UNDEF and ZERO settings didn't work.

A last comment: pay attention that according to my test, UNDEF and ZERO settings do not work neither in a MANUAL schema, not only for automatic renewals.
Comment 17 Claudio Costales 2016-02-17 19:17:01 UTC
Holger,

I have made a second test (including bug 14101) on my local virtual machine  and the problem persists. 

- Release 3.22.3 + bug 14101
- Automatic renewal cronjob was executed manually by me
- Same parameters in issuing rules that were set in the first test 

Tell me how can I help from now on please.
Comment 18 Claudio Costales 2016-02-17 19:19:45 UTC
(In reply to bondiurbano from comment #17)
> Holger,
> 
> I have made a second test (including bug 14101) on my local virtual machine 
> and the problem persists. 
> 
> - Release 3.22.3 + bug 14101
> - Automatic renewal cronjob was executed manually by me
> - Same parameters in issuing rules that were set in the first test 
> 
> Tell me how can I help from now on please.



Sorry, 14101 (pushed to master) and 14395 (passed QA) bugs were included.
Comment 19 Claudio Costales 2016-02-24 16:08:44 UTC
(In reply to Holger Meißner from comment #15)
> Hi bondiurbano, please take your time. Don't work on the weekend.
> 
> When you do test again, please include when exactly the cronjob is
> scheduled, or if you run automatic_renewals.pl manually.

Hi Holger!

I do not want to bother you but this problem is generating confussion in most customers. Apart from automatic renewals, the standard procedure is working bad, i.e., the legend "No renewals before dd/mm/yyy" is correct (the date informed is OK) but Koha does not enable the renewal. For example, if date in the legend is 24/2 and today is 24/2 Koha does not enable the renewal to be set manually (neither in intranet nor OPAC). No matter what value of days is informed in the issuing rule, Koha never enables the renewals in this case.

This erroneous behavoir occurs since last december, when bug 14101 was send to stable. Look at my comment 15 point 3 in bug 14101.
Comment 20 Claudio Costales 2016-08-29 00:05:55 UTC

*** This bug has been marked as a duplicate of bug 14395 ***