From 7bdc49323e8a271231567aa9cf0075add5e9a98e Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Fri, 14 May 2021 15:39:03 +0100
Subject: [PATCH] Bug 22435: Update accountline-details page to show all
 history

The accountline-details page took a simplistic approach to displaying
the history of an accountline. This patch drops the now defunct
offset_type_description template block from it's respective include
files, and then updates the accountline-details template and controller
to show the full history of what's happend to the accountline passed.
This includes the creation, any increments/decrements (for fines), and
finally any offsets against the total (payments, cancellations, voids).

Test plan
1/ Create some credits and debits and apply them to each other in
various combinations.
2/ View the 'Details' page for some of the debit/credits
3/ Compare before patch and after for the Details page.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/Account/Line.pm                          |  13 ++
 .../prog/en/includes/accounts.inc             |  14 ---
 .../en/modules/members/accountline-details.tt | 119 ++++++++++--------
 .../bootstrap/en/includes/accounts.inc        |  14 ---
 members/accountline-details.pl                |  17 +--
 5 files changed, 92 insertions(+), 85 deletions(-)

diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm
index a05ab65c28..c0964fc006 100644
--- a/Koha/Account/Line.pm
+++ b/Koha/Account/Line.pm
@@ -57,6 +57,19 @@ sub patron {
     return Koha::Patron->_new_from_dbic( $rs );
 }
 
+=head3 manager
+
+Return the manager linked to this account line
+
+=cut
+
+sub manager {
+    my ( $self ) = @_;
+    my $rs = $self->_result->manager;
+    return unless $rs;
+    return Koha::Patron->_new_from_dbic( $rs );
+}
+
 =head3 item
 
 Return the item linked to this account line if exists
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc
index 6c5450b0de..e57195717c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc
@@ -56,17 +56,3 @@
         [%- CASE              -%]
     [%- END -%]
 [%- END -%]
-
-[%- BLOCK offset_type_description -%]
-    [%- SWITCH account_offset.type -%]
-        [%- CASE 'Credit Applied' -%]<span>Credit applied</span>
-        [%- CASE 'Payment'        -%]<span>Payment</span>
-        [%- CASE 'Manual Credit'  -%]<span>Manual credit</span>
-        [%- CASE 'Manual Debit'   -%]<span>Manual invoice</span>
-        [%- CASE 'Lost Item'      -%]<span>Lost item returned</span>
-        [%- CASE 'Writeoff'       -%]<span>Writeoff</span>
-        [%- CASE 'Void Payment'   -%]<span>Void payment</span>
-        [%- CASE 'CANCELLATION'   -%]<span>Cancelled charge</span>
-        [%- CASE                  -%][% account_offset.type | html %]
-    [%- END -%]
-[%- END -%]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/accountline-details.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/accountline-details.tt
index c8d50f0a5e..c6e9fb6285 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/accountline-details.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/accountline-details.tt
@@ -7,7 +7,7 @@
 [% SET footerjs = 1 %]
 [% PROCESS 'accounts.inc' %]
 [% INCLUDE 'doc-head-open.inc' %]
-<title>Details of fee &rsaquo; [% INCLUDE 'patron-title.inc' no_html = 1 %] &rsaquo; Patrons &rsaquo; Koha</title>
+<title>Details of [% IF type == 'credit' %]credit[% ELSE %]debit[% END %] &rsaquo; [% INCLUDE 'patron-title.inc' no_html = 1 %] &rsaquo; Patrons &rsaquo; Koha</title>
 [% INCLUDE 'doc-head-close.inc' %]
 </head>
 
@@ -24,24 +24,27 @@
             <a href="/cgi-bin/koha/members/members-home.pl">Patrons</a>
         </li>
 
-        [% IF type == 'credit' %]
+
+        [% IF accountline %]
+            [% IF accountline.credit_type_code %]
             <li>
                 <a href="/cgi-bin/koha/members/boraccount.pl?borrowernumber=[% patron.borrowernumber | uri %]">Account for [% INCLUDE 'patron-title.inc' %]</a>
             </li>
             <li>
                 <a href="#" aria-current="page">
-                    Details of payment
+                    Details of credit ([% accountline.id | html %])
                 </a>
             </li>
-        [% ELSIF type == 'debit' %]
+            [% ELSIF accountline.debit_type_code %]
             <li>
                 <a href="/cgi-bin/koha/members/boraccount.pl?borrowernumber=[% patron.borrowernumber | uri %]">Account for [% INCLUDE 'patron-title.inc' %]</a>
             </li>
             <li>
                 <a href="#" aria-current="page">
-                    Details of fee
+                    Details of debit ([% accountline.id | html %])
                 </a>
             </li>
+            [% END %]
         [% ELSE %]
             <li>
                 <a href="#" aria-current="page">
@@ -60,17 +63,17 @@
             [% INCLUDE 'members-toolbar.inc' borrowernumber=patron.borrowernumber %]
 
             [% IF accountline %]
-                [% IF type == 'credit' %]
-                    <h2>Details of payment</h2>
-                [% ELSIF type == 'debit' %]
-                    <h2>Details of fee</h2>
+                [% IF accountline.credit_type_code %]
+                    <h2>Details of credit</h2>
+                [% ELSIF accountline.debit_type_code %]
+                    <h2>Details of debit</h2>
                 [% END %]
 
                 <table id="table_account_fines">
                     <thead>
                         <tr>
                             <th>Date</th>
-                            <th>Description of charges</th>
+                            <th>Description</th>
                             <th>Barcode</th>
                             <th>Due date</th>
                             <th>Return date</th>
@@ -83,7 +86,7 @@
                     <tbody>
                         <tr>
                             <td>
-                                [% accountline.date |$KohaDates %]
+                                [% accountline.date | $KohaDates %]
                             </td>
                             <td>
                                 [%- PROCESS account_type_description account=accountline -%]
@@ -134,63 +137,81 @@
                     </tbody>
                 </table>
 
-
-                [% IF type == 'credit' %]
-                    <h3>Fees paid</h3>
-                [% ELSIF type == 'debit' %]
-                    <h3>Payments</h3>
-                [% END %]
-
+                <h3>History</h3>
                 <table class="accountline-offsets-table" id="accountline-debits-table">
                     <thead>
                         <tr>
-                            <th>Date created</th>
-                            <th>Date updated</th>
-                            <th>Amount</th>
-                            <th>Amount outstanding</th>
+                            <th rowspan="2">Date</th>
+                            <th colspan="2">Change</th>
+                            <th rowspan="2">Action</th>
+                            <th colspan="4">Related transaction</th>
+                        </tr>
+                        <tr>
+                            <th>Increase</th>
+                            <th>Decrease</th>
                             <th>Type</th>
+                            <th>Amount</th>
+                            <th>Librarian</th>
                             <th>Note</th>
-                            <th>Transacting librarian</th>
-                            <th>Date/time of change</th>
-                            <th>Amount of change</th>
-                            <th>Type of change</th>
-                            <th>&nbsp;</th>
                         </tr>
                     </thead>
 
                     <tbody>
                         [% FOREACH ao IN account_offsets %]
-                            [% IF type == 'credit' %]
-                                [% SET offset_accountline = ao.debit %]
-                            [% ELSIF type == 'debit' %]
-                                [% SET offset_accountline = ao.credit %]
-                            [% END %]
+                                [% IF ao.credit_id == accountline.accountlines_id %]
+                                    [% SET offset_accountline = ao.debit %]
+                                [% ELSIF ao.debit_id == accountline.accountlines_id %]
+                                    [% SET offset_accountline = ao.credit %]
+                                [% END %]
+
+                                [%- BLOCK ao_description -%]
+                                    [%- SWITCH ao.type -%]
+                                        [%- CASE 'CREATE'           -%]<span>Created</span>
+                                        [%- CASE 'OVERDUE_INCREASE' -%]<span>Increase</span>
+                                        [%- CASE 'OVERDUE_DECREASE' -%]<span>Decrease</span>
+                                        [%- CASE 'APPLY'            -%]<span>Applied</span>
+                                        [%- CASE 'VOID'             -%]<span>Reversed</span>
+                                    [%- END -%]
+                                [%- END -%]
 
-                            [% IF offset_accountline %]
                                 <tr>
-                                    <td>[% offset_accountline.date | $KohaDates %]</td>
-                                    <td>[% offset_accountline.timestamp | $KohaDates  with_hours => 1 %]</td>
-                                    <td>[% offset_accountline.amount | $Price %]</td>
-                                    <td>[% offset_accountline.amountoutstanding | $Price %]</td>
-                                    <td>[% PROCESS account_type_description account=offset_accountline %]</td>
+                                    <td>[% ao.created_on | $KohaDates with_hours => 1%]</td>
+                                    <td>[%- IF ao.amount > 0 -%][% ao.amount | $Price %][% END %]</td>
+                                    <td>[%- IF ao.amount < 0 -%][% ao.amount | $Price %][% END %]</td>
+                                    <td>
+                                        [% PROCESS ao_description ao=ao %]
+                                    </td>
+                                    [%- IF offset_accountline -%]
+                                    <td>
+                                        [% PROCESS account_type_description account=offset_accountline %] (<a href="accountline-details.pl?accountlines_id=[% offset_accountline.id | uri %]">[% offset_accountline.id | html %]</a>)
+                                    </td>
+                                    <td>
+                                        [% offset_accountline.amount | $Price %]
+                                    </td>
+                                    <td>
+                                        [% IF offset_accountline.manager_id %]
+                                        <a href="moremember.pl?borrowernumber=[% offset_accountline.manager_id | uri %]">
+                                            [% IF offset_accountline.manager.firstname %]
+                                                [% offset_accountline.manager.firstname | html %] [% offest_accountline.manager.surname | html %]
+                                            [% ELSE %]
+                                                [% offset_accountline.manager.surname | html %]
+                                            [% END %]
+                                        </a>
+                                        [% END %]
+                                    </td>
+                                    [%- ELSE -%]
+                                    <td></td>
+                                    <td></td>
+                                    <td></td>
+                                    [%- END -%]
                                     <td>[% offset_accountline.note | html %]</td>
-                                    <td>[% IF offset_accountline.manager_id %]<a href="moremember.pl?borrowernumber=[% offset_accountline.manager_id | uri %]">[% offset_accountline.manager_id | html %]</a>[% END %]</td>
-                                    <td>[% ao.created_on | $KohaDates  with_hours => 1 %]</td>
-                                    <td>[% ao.amount | $Price %]</td>
-                                    <td>[% PROCESS offset_type_description account_offset=ao %]</td>
-                                    <td><a href="accountline-details.pl?accountlines_id=[% offset_accountline.id | uri %]" class="btn btn-default btn-xs"><i class="fa fa-list"></i> Details</a></td>
                                 </tr>
-                            [% END %]
                         [% END %]
                     </tbody>
                 </table>
             [% ELSE %]
                 <div class="dialog message">
-                    [% IF type == 'credit' %]
-                        Payment not found
-                    [% ELSIF type == 'debit' %]
-                        Fee not found
-                    [% END %]
+                    Account not found
                 </div>
             [% END %]
 
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/accounts.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/accounts.inc
index 6c5450b0de..e57195717c 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/accounts.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/accounts.inc
@@ -56,17 +56,3 @@
         [%- CASE              -%]
     [%- END -%]
 [%- END -%]
-
-[%- BLOCK offset_type_description -%]
-    [%- SWITCH account_offset.type -%]
-        [%- CASE 'Credit Applied' -%]<span>Credit applied</span>
-        [%- CASE 'Payment'        -%]<span>Payment</span>
-        [%- CASE 'Manual Credit'  -%]<span>Manual credit</span>
-        [%- CASE 'Manual Debit'   -%]<span>Manual invoice</span>
-        [%- CASE 'Lost Item'      -%]<span>Lost item returned</span>
-        [%- CASE 'Writeoff'       -%]<span>Writeoff</span>
-        [%- CASE 'Void Payment'   -%]<span>Void payment</span>
-        [%- CASE 'CANCELLATION'   -%]<span>Cancelled charge</span>
-        [%- CASE                  -%][% account_offset.type | html %]
-    [%- END -%]
-[%- END -%]
diff --git a/members/accountline-details.pl b/members/accountline-details.pl
index 699993db76..b798ef9647 100755
--- a/members/accountline-details.pl
+++ b/members/accountline-details.pl
@@ -45,20 +45,21 @@ my $accountlines_id = $input->param('accountlines_id');
 my $accountline = Koha::Account::Lines->find($accountlines_id);
 
 if ($accountline) {
-    my $type = $accountline->amount < 0 ? 'credit' : 'debit';
-    my $column = $type eq 'credit' ? 'credit_id' : 'debit_id';
-
     my $account_offsets = Koha::Account::Offsets->search(
-        { $column  => $accountlines_id },
-        { order_by => 'created_on' },
+        [
+            {
+                credit_id => $accountline->accountlines_id
+            },
+            {
+                debit_id => $accountline->accountlines_id
+            }
+        ],
+        { order_by => 'created_on' }
     );
 
     $template->param(
-        type            => $type,
         accountline     => $accountline,
         account_offsets => $account_offsets,
-
-        finesview => 1,
     );
 
     my $patron = Koha::Patrons->find( $accountline->borrowernumber );
-- 
2.20.1