Bug 16486

Summary: Display the TIME a fine was collected/written off
Product: Koha Reporter: Theodoros Theodoropoulos <theod>
Component: Fines and feesAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: aleisha, dominic.pichette, fridolin.somers, gmcharlt, jdemuth, jonathan.druart, katrin.fischer, kyle.m.hall, kyle, m.de.rooy, marjorie.barry-vila, martin.renvoize, nengard, nick, sally.healey, sandboxes, simon.pouchol, veron
Version: unspecifiedKeywords: Academy
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18790
Change sponsored?: Sponsored Patch complexity: Large patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch updates the information displayed in Koha for account lines. In accounts tables, the current 'Date' field is renamed 'Created', and a new column 'Updated' is added to display the last updated timestamp of the line. For new installations, accounts notices are also updated to include this information by default.
Version(s) released in:
21.05.00
Attachments: Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: Display the timestamp for account payments/writeoffs
[SIGNED OFF] Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: [FOLLOW-UP] Putting time in date column
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: [FOLLOW-UP] Putting time in date column
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: [FOLLOW-UP] Show original date and timestamp for fines
Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: [FOLLOW-UP] Putting time in date column
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: [FOLLOW-UP] Show original date and timestamp for fines
Bug 16486: Adding timestamp column to cash register statistics
Bug 16486: Display the timestamp for account payments/writeoffs
Bug 16486: [FOLLOW-UP] Putting time in date column
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: [FOLLOW-UP] Show original date and timestamp for fines
Bug 16486: Adding timestamp column to cash register statistics
Bug 16486: Adding timestamp column to cash register statistics
Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Bug 16486: [FOLLOW-UP] Show original date and timestamp for fines
Bug 16486: Adding timestamp column to cash register statistics
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated
Bug 16486: Display the timestamp when an accountline is updated

Description Theodoros Theodoropoulos 2016-05-10 11:28:07 UTC
In many cases it would be very useful to be able to know the time a fine was collected/written off (in addition to the date).

So ideally, the TIME could be shown next to the date in the Fines->Account tab.
(This would make sense in almost all cases except maybe in Accruing fines).

Alternatively, a text could be added to the description column. For example, "Payment, thanks" could become "Payment on 15/09/2015 14:30, thanks", "Writeoff" could become "Written off on 15/09/2015 14:30", etc
Comment 1 Aleisha Amohia 2017-04-18 23:49:03 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2017-04-20 13:08:19 UTC
I like this change but shouldn't the date formatting be done in the template using '$KohaDates with_hours => 1' ?
Comment 3 Marc VĂ©ron 2017-04-21 07:33:52 UTC
(In reply to Owen Leonard from comment #2)
> I like this change but shouldn't the date formatting be done in the template
> using '$KohaDates with_hours => 1' ?

+1
Comment 4 Aleisha Amohia 2017-04-26 03:51:05 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2017-05-02 21:52:40 UTC Comment hidden (obsolete)
Comment 6 Theodoros Theodoropoulos 2017-05-03 06:49:52 UTC
Thank you all for your work on this request.
I can verify that the provided patch works as expected!
[FYI, for Koha 3.20 the patch needs one line less, because 'Hold waiting too long' is not available]

Having said that, unless you print slips with the text from the description, I think it would be more generic to simply change the date column to show hours/minutes in ALL fine lines.
Following Owen's suggestion, it would be something like this:

-  <td><span title="[% account.date %]">[% account.date |$KohaDates %]</span></td>
+  <td><span title="[% account.timestamp %]">[% account.timestamp |$KohaDates with_hours => 1 %]</span></td>

What do you think?
Comment 7 Jonathan Druart 2017-05-03 18:51:13 UTC
Would not it make sense to add it everywhere?

% git grep ', thanks' **/*.tt|cut -d':' -f1|uniq
koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/printfeercpt.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/printinvoice.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-account.tt

It seems that occurrences in pay.tt are not relevant, we do not display payments in this table.
Comment 8 Marcel de Rooy 2017-09-01 09:11:14 UTC
(In reply to Jonathan Druart from comment #7)
> Would not it make sense to add it everywhere?
> 
> % git grep ', thanks' **/*.tt|cut -d':' -f1|uniq
> koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
> koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
> koha-tmpl/intranet-tmpl/prog/en/modules/members/printfeercpt.tt
> koha-tmpl/intranet-tmpl/prog/en/modules/members/printinvoice.tt
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-account.tt
> 
> It seems that occurrences in pay.tt are not relevant, we do not display
> payments in this table.

No response on this question.
Changing status to reflect need for feedback.
Comment 9 Aleisha Amohia 2017-09-04 23:43:44 UTC Comment hidden (obsolete)
Comment 10 Aleisha Amohia 2017-09-05 02:48:41 UTC Comment hidden (obsolete)
Comment 11 Dominic Pichette 2017-09-29 19:50:57 UTC
I had this error trying to access the account tab in fines:

Global symbol "$input" requires explicit package name (did you forget to declare "my $input"?) at /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl line 41.
syntax error at /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl line 44, near "1cc607141dd67a2373996a1cf5116b1334354420

"
Global symbol "$input" requires explicit package name (did you forget to declare "my $input"?) at /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl line 56.
Execution of /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl aborted due to compilation errors.
Comment 12 Jonathan Druart 2017-09-29 20:18:23 UTC
(In reply to Dominic Pichette from comment #11)
> I had this error trying to access the account tab in fines:

Hi Dominic,
When you find an issue, you can switch the status to Failed QA.
Thanks for testing!
Comment 13 Aleisha Amohia 2017-10-03 01:52:05 UTC Comment hidden (obsolete)
Comment 14 Aleisha Amohia 2017-10-03 01:55:51 UTC Comment hidden (obsolete)
Comment 15 Aleisha Amohia 2017-10-03 01:57:58 UTC
Found a conflict in the first patch so fixed it.

(In reply to Dominic Pichette from comment #11)
> I had this error trying to access the account tab in fines:
> 
> Global symbol "$input" requires explicit package name (did you forget to
> declare "my $input"?) at
> /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl line 41.
> syntax error at /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl
> line 44, near "1cc607141dd67a2373996a1cf5116b1334354420
> 
> "
> Global symbol "$input" requires explicit package name (did you forget to
> declare "my $input"?) at
> /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl line 56.
> Execution of /inlibro/git/koha-master-dev-inlibro/members/boraccount.pl
> aborted due to compilation errors.

Thank you for testing but I was unable to reproduce these errors, everything still works as expected for me. Can you please test again and make sure your master is up to date?
Comment 16 Simon Pouchol 2017-10-19 12:28:42 UTC
Hey,
I tried testing, and got this error message upon trying to apply the patch :

fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 16486: [FOLLOW-UP] Putting time in date column

Unsure what it means, whether it's my setup or the patches not working. (my master is up to date)
Comment 17 Marcel de Rooy 2017-10-19 12:31:00 UTC
Are the patches still in the right order ?
Comment 18 Aleisha Amohia 2017-10-20 00:15:01 UTC
(In reply to Marcel de Rooy from comment #17)
> Are the patches still in the right order ?

oops no they are not - i will fix this
Comment 19 Aleisha Amohia 2017-10-20 00:16:49 UTC Comment hidden (obsolete)
Comment 20 Aleisha Amohia 2017-10-20 00:16:55 UTC Comment hidden (obsolete)
Comment 21 Aleisha Amohia 2017-10-20 00:17:00 UTC Comment hidden (obsolete)
Comment 22 Simon Pouchol 2017-10-20 09:50:08 UTC Comment hidden (obsolete)
Comment 23 Nick Clemens 2017-10-26 16:14:49 UTC
(In reply to Aleisha Amohia from comment #20)
> Created attachment 68303 [details] [review] [review]
> Bug 16486: [FOLLOW-UP] Putting time in date column
> 
> In reply to Comment 6, I have moved the time into a column separate from
> the description. Unfortunately the timestamp has the date and the time,
> so I figured it was easier to just have them in the same column and
> replace the original date column.

I think the date column has different info than the timestamp and shouldn't be replaced for all instances - for things like lost fees or account fees date is when it was created - if a partial payment is applied that updates timestamp but not date

We should probably have both columns available, or show both info:

Original date:
2017-07-07
Updated:
2017-10-21 12:12:38

or

2017-10-21 12:12:38 (2017-07-07)
Comment 24 Aleisha Amohia 2018-01-16 22:45:10 UTC Comment hidden (obsolete)
Comment 25 Aleisha Amohia 2018-01-16 23:11:05 UTC Comment hidden (obsolete)
Comment 26 Aleisha Amohia 2018-01-16 23:12:35 UTC Comment hidden (obsolete)
Comment 27 Aleisha Amohia 2018-01-16 23:14:10 UTC Comment hidden (obsolete)
Comment 28 Aleisha Amohia 2018-01-16 23:15:11 UTC Comment hidden (obsolete)
Comment 29 Marjorie Barry-Vila 2018-01-17 14:19:40 UTC
(In reply to Aleisha Amohia from comment #28)
> Created attachment 70583 [details] [review] [review]
> Bug 16486: [FOLLOW-UP] Show original date and timestamp for fines
> 
> This patch has two columns in fines tables, one showing the original
> date and one showing the timestamp (when the patch was last updated)
> 
> Fixed merge conflicts

Hi Aleisha,

I tested on a sandbox and it works very well.
But I was wondering if the column Updated should not appear also in the report Cash register statistics (reports/cash_register_stats.pl)?


Regards,
Marjorie
Comment 30 Aleisha Amohia 2018-01-17 23:10:25 UTC Comment hidden (obsolete)
Comment 31 Biblibre Sandboxes 2018-01-18 13:26:11 UTC
Patch tested with a sandbox, by Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca>
Comment 32 Biblibre Sandboxes 2018-01-18 13:27:15 UTC Comment hidden (obsolete)
Comment 33 Biblibre Sandboxes 2018-01-18 13:27:19 UTC Comment hidden (obsolete)
Comment 34 Biblibre Sandboxes 2018-01-18 13:27:23 UTC Comment hidden (obsolete)
Comment 35 Biblibre Sandboxes 2018-01-18 13:27:27 UTC Comment hidden (obsolete)
Comment 36 Biblibre Sandboxes 2018-01-18 13:27:30 UTC Comment hidden (obsolete)
Comment 37 Maksim Sen 2018-02-16 21:58:34 UTC Comment hidden (obsolete)
Comment 38 Katrin Fischer 2018-04-10 06:12:55 UTC
I am sorry, Aleisha, I know this has been sitting for a bit. 
Can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16486: Display the timestamp for account payments/writeoffs
Applying: Bug 16486: [FOLLOW-UP] Putting time in date column
Applying: Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/printfeercpt.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/printinvoice.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-account.tt
M	members/printfeercpt.pl
M	members/printinvoice.pl
Falling back to patching base and 3-way merge...
Auto-merging members/printinvoice.pl
CONFLICT (content): Merge conflict in members/printinvoice.pl
Auto-merging members/printfeercpt.pl
CONFLICT (content): Merge conflict in members/printfeercpt.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-account.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/printinvoice.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/printinvoice.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/printfeercpt.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 16486: [FOLLOW-UP] Adding timestamp to date in other places
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16486-FOLLOW-UP-Adding-timestamp-to-date-in-ot-9RSFdA.patch
Comment 39 Aleisha Amohia 2018-04-12 04:42:10 UTC Comment hidden (obsolete)
Comment 40 Aleisha Amohia 2018-04-12 04:50:04 UTC Comment hidden (obsolete)
Comment 41 Aleisha Amohia 2018-04-12 04:52:15 UTC Comment hidden (obsolete)
Comment 42 Katrin Fischer 2018-04-12 07:43:28 UTC
If the rebase was not very severe, it's ok if you put this back to 'signed off'
Comment 43 Katrin Fischer 2018-04-18 22:16:50 UTC
Hi Aleisha,

QA script passes with the exception of the commit message descriptions. Could you please fix with the next iteraton? (https://wiki.koha-community.org/wiki/Commit_messages)


1) borraccount.tt
+          <th class="title-string">Original date</th>
+          <th class="title-string">Updated</th>

Maybe make this "Created" or "Creation date" and "Updated". Not sure original date will be understood, thinking of translation too.

Why the change from Writeoff to Written off?

The last table row (total due) is not correctly formatted with the patch (empty last cell).

2) printfeercpt.tt

Table layout is broken by the patch here too. Same for terminology as above.

3) cash_register_stats.tt

Table layout is broken here too (last total row).

Stopping here. Please consider if changing the description would make sense and recheck the other changed templates for layout problems too.
Comment 44 Katrin Fischer 2019-03-14 23:46:44 UTC
*** Bug 2601 has been marked as a duplicate of this bug. ***
Comment 45 Aleisha Amohia 2020-04-23 05:18:37 UTC Comment hidden (obsolete)
Comment 46 PTFS Europe Sandboxes 2020-04-29 13:23:10 UTC Comment hidden (obsolete)
Comment 47 Katrin Fischer 2020-05-02 02:52:31 UTC
Comment on attachment 103938 [details] [review]
Bug 16486: Display the timestamp when an accountline is updated

Review of attachment 103938 [details] [review]:
-----------------------------------------------------------------

Some first notes here:

1) Database updates

$dbh->do(q{DELETE FROM letter WHERE code = 'ACCOUNT_DEBIT'});
This means we'd be destroying any customized templates the libraries have created themselves and also will force them back to English if they have translated them. I think we should remove these and leave the templates for existing installations as they are. We could add a sample template including the new information to the notices sample page on the wiki maybe

2) Needs a little rebase - es-ES is now using the .yml files to create translations.

3) Cash register changes work well, but the column should also be added to the CSV export to make them match.
Comment 48 Aleisha Amohia 2021-02-24 04:02:06 UTC Comment hidden (obsolete)
Comment 49 Aleisha Amohia 2021-02-24 04:03:22 UTC
Created attachment 117233 [details] [review]
Bug 16486: Display the timestamp when an accountline is updated

This patch updates the Date field to be 'Created' and adds a new column
'Updated' which shows the timestamp of the accountline.

To test:
1) Apply patch, restart services
2) Test the following staff client pages:
- Reports -> Cash register
- Patron Accounting tab -> Transactions
- Create a manual invoice. Click 'Print' on the transactions tab to
generate Invoice slip
- Make a payment. Click 'Print' on the transactions tab to generate
Receipt slip
3) Test the OPAC:
- your charges

Sponsored-by: Catalyst IT

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 50 Owen Leonard 2021-02-25 16:28:22 UTC
Created attachment 117348 [details] [review]
Bug 16486: Display the timestamp when an accountline is updated

This patch updates the Date field to be 'Created' and adds a new column
'Updated' which shows the timestamp of the accountline.

To test:
1) Apply patch, restart services
2) Test the following staff client pages:
- Reports -> Cash register
- Patron Accounting tab -> Transactions
- Create a manual invoice. Click 'Print' on the transactions tab to
generate Invoice slip
- Make a payment. Click 'Print' on the transactions tab to generate
Receipt slip
3) Test the OPAC:
- your charges

Sponsored-by: Catalyst IT

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 51 Martin Renvoize 2021-04-27 14:58:17 UTC
Sorry Aliesha, I didn't spot this one in the tree... else I'd have tried to fold it into the now pushed bug 26734.. it was only upon going to update the wiki pages at Katrins request I saw a link to this bug.

Would you like me to have a go at rebasing for you or are you OK to take a look.. it appears that it stopped applying prior to my patch but I feel a bit guilty having made it even harder.
Comment 52 Aleisha Amohia 2021-04-27 21:49:57 UTC
(In reply to Martin Renvoize from comment #51)
> Sorry Aliesha, I didn't spot this one in the tree... else I'd have tried to
> fold it into the now pushed bug 26734.. it was only upon going to update the
> wiki pages at Katrins request I saw a link to this bug.
> 
> Would you like me to have a go at rebasing for you or are you OK to take a
> look.. it appears that it stopped applying prior to my patch but I feel a
> bit guilty having made it even harder.

No worries. Yes please if you can try rebasing that would be awesome, I'm not sure when I'll have time to revisit this.
Comment 53 Aleisha Amohia 2021-05-05 05:13:28 UTC
Created attachment 120479 [details] [review]
Bug 16486: Display the timestamp when an accountline is updated

This patch updates the Date field to be 'Created' and adds a new column
'Updated' which shows the timestamp of the accountline.

To test:
1) Apply patch, restart services
2) Test the following staff client pages:
- Reports -> Cash register
- Patron Accounting tab -> Transactions
- Create a manual invoice. Click 'Print' on the transactions tab to
generate Invoice slip
- Make a payment. Click 'Print' on the transactions tab to generate
Receipt slip
3) Test the OPAC:
- your charges

Sponsored-by: Catalyst IT

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 54 Martin Renvoize 2021-05-05 10:31:08 UTC
Created attachment 120485 [details] [review]
Bug 16486: Display the timestamp when an accountline is updated

This patch updates the Date field to be 'Created' and adds a new column
'Updated' which shows the timestamp of the accountline.

To test:
1) Apply patch, restart services
2) Test the following staff client pages:
- Reports -> Cash register
- Patron Accounting tab -> Transactions
- Create a manual invoice. Click 'Print' on the transactions tab to
generate Invoice slip
- Make a payment. Click 'Print' on the transactions tab to generate
Receipt slip
3) Test the OPAC:
- your charges

Sponsored-by: Catalyst IT
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2021-05-05 10:35:28 UTC
This works as described and causes no regressions I could see.. as such I think it warrants a PQA.

I do however slightly question the logic in some of the notices... I find it a little strange adding the updated time to the ACCOUNT_CREDIT and ACCOUNT_DEBIT slips.. both of those, as they stand, only show the one credit or debit line.. and the created and updated times should match in all cases I believe.  That said.. there's ongoing work to refine those notices and make them more useful by iterating the offsets to get the linked accountlines so I think this isn't a bad first step overall.
Comment 56 Jonathan Druart 2021-05-07 12:41:00 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 57 Fridolin Somers 2021-05-11 14:07:34 UTC
Enhancement not pushed to 20.11.x