[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