[OPEN-ILS-DEV] Lost item handling
Bill Ott
bott at grpl.org
Tue Jan 6 22:10:29 EST 2009
Oops, that diff was against some of my previous changes. This one
should make more sense.
Bill Ott wrote:
> Mike Rylander wrote:
>> In hopes of encouraging continuation, I offer a
>> DryTechnicalReview(tm). ;)
>>
>> <dry technical review>
>>
>> My only style comment is a request to use 'if' instead of 'unless' for
>> the multi-condition test at Circulate.pm:1925. The logic reversals
>> get ... complicated ... during debugging. ;) I've hurt myself with
>> this, and following other's code is even harder (you lack the original
>> logical context).
>>
>> It looks like we're loosing the fallback code in the original
>> Circulate.pm at lines 1927-1928 which set the copy status to
>> reshelving if there are no matched conditions. It looks like this
>> should be in its own 'else' block at the end of the new code, instead
>> of inside the 'elsif ($stat == OILS_COPY_STATUS_LOST)' branch.
>>
>> IMO the unvoiding of overdue billings should be separated out from
>> checkin_handle_lost_now_found, since it's wasted effort to do that
>> twice in the case of both voiding the lost and lost processing fees.
>>
>> </dry technical review>
>
> My vacation around the holidays put a real damper on my productivity,
> but I decided that I shouldn't leave this hanging until I completely
> forgot what I was doing. Tonight I plugged in a few changes,
> including adding a max time to accept the return (i.e. "1 year", "90
> days", etc.)
>
> In all it adds 5 different setting options, I don't really like what
> I've done with $max_return, and I haven't tested all the options yet,
> so there might be a glaring problem, but for what it's worth...
>
>
> Additions to Const.pm:
>
> econst OILS_SETTING_VOID_LOST_ON_CHECKIN => 'circ.void_lost_on_checkin';
> econst OILS_SETTING_MAX_ACCEPT_RETURN_OF_LOST =>
> 'circ.max_accept_return_of_lost';
> econst OILS_SETTING_VOID_LOST_PROCESS_FEE_ON_CHECKIN =>
> 'circ.void_lost_proc_fee_on_checkin';
> econst OILS_SETTING_RESTORE_OVERDUE_ON_LOST_RETURN =>
> 'circ.restore_overdue_on_lost_return';
> econst OILS_SETTING_LOST_IMMEDIATELY_AVAILABLE =>
> 'circ.lost_immediately_available';
>
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Circulate.pm.diff
Url: http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20090106/c4dbe49e/attachment-0001.txt
More information about the Open-ils-dev
mailing list