[Koha-bugs] [Bug 20997] Add Koha::Account::Line::apply

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

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

--- Comment #24 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Jonathan Druart from comment #23)
> Just minor (not blocker), reading the code:
> 
> 1. In Koha::Account::Line->apply
> +    my $offset_type = $params->{offset_type} // 'credit_applied';
> 
> Should not we raise an exception if the offset_type is not known?

This method applies a credit to debits. The idea was to make it simple to use,
with documented defaults. There are two use cases in which I'm sure offset_type
will not be 'credit_applied':
- Payments: if we rewrite ->pay using this, then the offset type would be
'payment' to keep the current behaviour.
- Write-offs: if we are writing off a debit, then 'writeoff' should be used.

IThe work collecting the existing hardcoded options is already done so should
be easy. If you consider it a blocker, I'll do it as a follow-up. Otherwise, I
will happily do it on a separate bug.

> I would also list the possible values in the POD (or at list tell it comes
> from 
> account_offset_types)

I agree.

> 2. I think we should follow the pattern "Credit Applied", instead of
> switching to snake_case "credit_applied" only for this new one.

Well, I wanted to deal with switching to snake_case all of them on a separate
bug, and I expected Katrin to fail this one for this case issue. I can fix it
as you like if you're QAing it.

> 3. is_debit, is_credit - what about amount == 0?

I only added is_credit he, blame Josef. Seriously, an account line with amount
= 0 only happens when a credit is void. And void credits are not credits as we
handle them. If we need methods for finding void credits specifically we could
add them, but that'd be another bug, when there's a use case for that.

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