[Koha-bugs] [Bug 20912] Rental fees based on time period

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20912

--- Comment #27 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Kyle M Hall from comment #26)
> (In reply to Tomás Cohen Arazi from comment #25)
> > I have a few remarks:
> > 
> >          my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'},
> > $patron->borrowernumber );
> > +        my $itemtype = Koha::ItemTypes->find( $item->{itype} ); # GetItem
> > sets effective itemtype
> > +        $rentalCharge += $itemtype->calc_rental_charge_daily( { from =>
> > dt_from_string(), to => $duedate } );
> > 
> > This doesn't look right, shouldn't it be either a fixed charge or the daily
> > calculated one but not both?? If it was two different charges, we might
> > better have separate offsets for them, right?
> 
> Yes, a library should be allowed to have a flat fee plus a daily fee if they
> wish. This patch set already ensures they will have separate offsets.
> 
> 
> > I would remove any fee calculation from Koha::ItemType (same for using
> > userenv). It should only return the 'daily' charge and let some other class
> > handle the calculation. 
> 
> That's fine by me!
> 
> > I've been thinking of 
> > my $fee = Koha::Fee->new({ library => $library, patron => $patron, item =>
> > $item, datetime_due => $dt });
> > 
> > This is of course out of the scope of this bug, but with that in mind, I
> > concur on Martin's proposal of having GetIssuingCharges do it on this bug.
> > The only concern I have is offsets.
> 
> Integrating this into GetIssuingCharges seems like a step backwards. What if
> I created Koha::Fees and moved the method to it? What I imagine is basically
> 
> my @fees = Koha::Fee->new({ library => $library, patron => $patron, item =>
> > $item, datetime_due => $dt })->get_fees, where the return is a list of Koha::Fee objects. To limit the scope creep, I will only move calc_rental_charge_daily into Fees.pm, and leave the breaking up and moving of the rest of GetIssuingCharges as a later exercise.
> 
> What do you think?

I agree.

-- 
You are receiving this mail because:
You are watching all bug changes.