[Koha-bugs] [Bug 19489] Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

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

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

--- Comment #2 from Nick Clemens <nick at bywatersolutions.com> ---
Comment on attachment 81956
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81956
Bug 19489 - Development for fines and Fees Module - Detailed Description of
charges in Patron Account Tab/Fines Tab

Review of attachment 81956:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=19489&attachment=81956)
-----------------------------------------------------------------

Everything works nicely in testing, the columns appear as expected.

The method for checkouts itself should be an object method for
Koha::Account::Line

It would be nice too if there were some refactoring to simply pass the Line
objects and use the fields/method on the the template rather than formatting in
a hash in the scripts

::: members/accountline-details.pl
@@ +24,5 @@
>  use C4::Output;
>  use C4::Context;
>  use Koha::Patrons;
> +use Koha::Checkouts;
> +use Koha::Old::Checkouts;

Rather than importing these here we should implement 'checkout' as an object
method for the Koha::Account::Line

@@ +67,5 @@
>      my $patron = Koha::Patrons->find( $accountline->borrowernumber );
> +    my $issues;
> +    if ( $accountline->issue_id ) {
> +        $issues = Koha::Checkouts->find($accountline->issue_id );
> +        $issues = Koha::Old::Checkouts->find($accountline->issue_id ) unless $issues;

As mentioned above - this should use an object method from Koha::Account::Line

The plural is also confusing as there should on ly ever be one

$accountline->checkout;

::: members/boraccount.pl
@@ +112,5 @@
> +
> +    if ( $accountline->{issue_id} ) {
> +        my $issues = Koha::Checkouts->find($accountline->{issue_id} );
> +        $issues = Koha::Old::Checkouts->find($accountline->{issue_id} ) unless $issues;
> +        $accountline->{issue}        = $issues;

As before, this should use an object method

::: members/pay.pl
@@ +152,5 @@
>          }
> +        if ( $account_line->{issue_id} ) {
> +            my $issues = Koha::Checkouts->find($account_line->{issue_id} );
> +            $issues = Koha::Old::Checkouts->find($account_line->{issue_id} ) unless $issues;
> +            $account_line->{issue}        = $issues;

Use object method

::: members/printinvoice.pl
@@ +64,5 @@
> +
> +if ( $accountline->{issue_id} ) {
> +    my $issues = Koha::Checkouts->find($accountline->{issue_id} );
> +    $issues = Koha::Old::Checkouts->find($accountline->{issue_id} ) unless $issues;
> +    $accountline->{issue}        = $issues;

Use object method

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