Why didn't I think of checking git for that line. Thanks for taking blame there ;) Let me explain what I'm working on, and see if it makes sense. I'm looking at having circulation.pl issue an error if the ceilingDueDate is in the past. My thought is that this is best done in CanBookBeIssued. On a side note I'm planning to move ReturnBeforeExpiry and ceilingDueDate into CalcDateDue() so that the renewal due date catches those also. So my plan is to reintroduce the date due check to CanBookBeIssued for the case of the date due not being given using a call to CalcDateDue. How's that sound? On Mon, 2009-04-27 at 17:42 -0400, Galen Charlton wrote:

Hi,

On Mon, Apr 27, 2009 at 5:26 PM, Michael Hafen <mdhafen@tech.washk12.org> wrote:

...

The Due Date check is commented out in CanBookBeIssued. I'm planning uncomment and expand it. Before I do that I want to know why it's commented out. I seem to recall it being a problem for Offline Circ, but looking at process_koc.pl CanBookBeIssued isn't even called.

git blame is your friend here. As far as I can work out, the line was commented out with the expectation that any code that calls CanBookBeIssued() would have already verified that the due date, in this case set via a global due date or a due date specified by the operator, is valid. Note that in this context, the $datedue parameter passed to CanBookBeIssued() is allowed to be undef, since most of the time you'll be having AddIssue() calculate the due date.

Regards,

Galen -- Michael Hafen Systems Analyst and Programmer Washington County School District Utah, USA

for Koha checkout http://development.washk12.org/gitweb/ or git://development.washk12.org/koha

Yeah, that's what I'm seeing too. Either have CanBookBeIssued return a due date, or have AddIssue and AddRenewal return an error code. Looking at the template issuingimpossible{ INVALID_DATE} is expected to be the offending date, but that's shoddy error response in this case. It doesn't indicate the source of the invalid date. What I've got now ( beta code ) is just that. CanBookBeIssued() sets up and calls CalcDateDue(), and stuffs the invalid date in issuingimpossible for the template if it fails. Having AddIssue and AddRenewal return errors isn't the best option in my opinion. In part because it means changing process_koc.pl too. Also because to me these subs are to perform action, and their CanBookBe* siblings are for the checking. I'd rather have the CanBookBe* pair return the expected ( or even real ) due date or give more descriptive errors. If circulation wasn't the used so much I'd say go as is. All I've done is added the calls to GetLoanLength and CalcDateDue. There's been a lot of work to tune circulation though, so I should be trying to optimize this. I should add the due date to the return list from CanBookBeIssued. It's already passed into AddIssue. Well, talked myself into it. I'll work on having CanBookBeIssued return the due date then. How's that sound? On Mon, 2009-04-27 at 18:12 -0400, Galen Charlton wrote:

Hi,

On Mon, Apr 27, 2009 at 5:56 PM, Michael Hafen <mdhafen@tech.washk12.org> wrote:

...

Let me explain what I'm working on, and see if it makes sense. I'm looking at having circulation.pl issue an error if the ceilingDueDate is in the past. My thought is that this is best done in CanBookBeIssued.

On a side note I'm planning to move ReturnBeforeExpiry and ceilingDueDate into CalcDateDue() so that the renewal due date catches those also.

So my plan is to reintroduce the date due check to CanBookBeIssued for the case of the date due not being given using a call to CalcDateDue. How's that sound?

I think there will have to be a bit more reorganization of the code. CalcDateDue() is currently called by AddIssue() and AddRenewal(), but the client code that calls CanBookBeIssued() currently doesn't always have access to the projected due date.

I think it would be a good idea to have one function call that calculates the due date, taking into account all of the parameters and sysprefs that can affect it. But once you do that, you have a choice between taking the calls to CalcDateDue() out of AddIssue() and AddRenewal() or adding an error return value to those too functions.

Regards,

Galen -- Michael Hafen Systems Analyst and Programmer Washington County School District Utah, USA

for Koha checkout http://development.washk12.org/gitweb/ or git://development.washk12.org/koha