Bug 24603 - Allow to cancel charges in patron accounting
Summary: Allow to cancel charges in patron accounting
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Martin Renvoize
URL:
Keywords: Manual
Depends on:
Blocks: 17596 26697
  Show dependency treegraph
 
Reported: 2020-02-06 14:20 UTC by Julian Maurice
Modified: 2020-10-16 15:27 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24603: Allow to cancel charges in patron accounting (12.06 KB, patch)
2020-02-06 14:21 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 24603: Allow to cancel charges in patron accounting (12.14 KB, patch)
2020-02-07 10:43 UTC, David Nind
Details | Diff | Splinter Review
Bug 24603: Allow to cancel charges in patron accounting (11.70 KB, patch)
2020-03-27 15:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 24603: (follow-up) Update to double entry accounting (18.90 KB, patch)
2020-07-10 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24603: Allow to cancel charges in patron accounting (11.67 KB, patch)
2020-10-05 10:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24603: (follow-up) Update to double entry accounting (18.91 KB, patch)
2020-10-05 10:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24603: Allow to cancel charges in patron accounting (11.75 KB, patch)
2020-10-05 19:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 24603: (follow-up) Update to double entry accounting (19.00 KB, patch)
2020-10-05 19:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 24603: Allow to cancel charges in patron accounting (11.87 KB, patch)
2020-10-15 20:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24603: (follow-up) Update to double entry accounting (19.18 KB, patch)
2020-10-15 20:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24603: Fix number of unit tests (776 bytes, patch)
2020-10-15 20:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24603: (QA follow-up) Make the new credit type translatable and shorten description a bit (2.32 KB, patch)
2020-10-15 20:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24603: (QA follow-up) Add translation to OPAC (2.11 KB, patch)
2020-10-16 15:26 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2020-02-06 14:20:31 UTC
There is already a button to void a payment. It should be possible to cancel a charge too.
Comment 1 Julian Maurice 2020-02-06 14:21:01 UTC
Created attachment 98534 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t
Comment 2 David Nind 2020-02-07 10:43:00 UTC
Created attachment 98551 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Julian Maurice 2020-03-27 15:05:53 UTC
Created attachment 102005 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Martin Renvoize 2020-07-06 14:34:50 UTC
Looking here...
Comment 5 Martin Renvoize 2020-07-10 11:00:44 UTC
The logic here doesn't feel right to me I'm afraid.

I've worked hard in recent releases to improve the consistency of our accounts records.. taking the 'double-entry accounting' approach.

In such an approach accountlines, once set, should not be altered and instead, all alterations/updates should be recorded as additional lines and offset against the original.  The exception to that rule is where the final amount is not yet set when the line is initially created (the only case so far is fines where we have accruing fines which are updated via the 'adjust' method).

I was intending to eventually update the void method to prevent it from updating the original payment line as losing that detail in the line itself can be a red flag when accountants come to audit the system.

Could we perhaps either update VOID to allow voiding of charges and apply a similar treatment here.. or utilise the 'reduce' method I added recently?
Comment 6 Martin Renvoize 2020-07-10 16:08:42 UTC
Created attachment 106772 [details] [review]
Bug 24603: (follow-up) Update to double entry accounting

This patch updates the logic to create a cancellation accountline and
apply it to the charge line so we correction record the transaction in
terms of double entry accounting standards.

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. A cancellation line should be associated with the original charge.
5. Create another manual invoice
6. Pay it (partially or fully)
7. Notice that the 'Cancel charge' button is not available
8. Void the payment
9. 'Cancel charge' button is available again. Click on it and verify
   that it still works
10. prove t/db_dependent/Koha/Account/Lines.t
Comment 7 Martin Renvoize 2020-07-10 16:51:36 UTC
Setting back to NSO to get Julians take on my follow-up
Comment 8 Julian Maurice 2020-10-05 09:22:34 UTC
Thanks for the patch Martin, but I cannot apply it ("could not build fake ancestor" error).
Can you rebase it please ?
Comment 9 Martin Renvoize 2020-10-05 10:41:50 UTC
Created attachment 111217 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize 2020-10-05 10:41:59 UTC
Created attachment 111218 [details] [review]
Bug 24603: (follow-up) Update to double entry accounting

This patch updates the logic to create a cancellation accountline and
apply it to the charge line so we correction record the transaction in
terms of double entry accounting standards.

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. A cancellation line should be associated with the original charge.
5. Create another manual invoice
6. Pay it (partially or fully)
7. Notice that the 'Cancel charge' button is not available
8. Void the payment
9. 'Cancel charge' button is available again. Click on it and verify
   that it still works
10. prove t/db_dependent/Koha/Account/Lines.t
Comment 11 Martin Renvoize 2020-10-05 10:43:17 UTC
I've rebased, but just in case there's also a branch on gitlab: https://gitlab.com/mrenvoize/Koha/-/commits/bug_24603
Comment 12 David Nind 2020-10-05 19:31:17 UTC
Created attachment 111255 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2020-10-05 19:31:22 UTC
Created attachment 111256 [details] [review]
Bug 24603: (follow-up) Update to double entry accounting

This patch updates the logic to create a cancellation accountline and
apply it to the charge line so we correction record the transaction in
terms of double entry accounting standards.

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. A cancellation line should be associated with the original charge.
5. Create another manual invoice
6. Pay it (partially or fully)
7. Notice that the 'Cancel charge' button is not available
8. Void the payment
9. 'Cancel charge' button is available again. Click on it and verify
   that it still works
10. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 David Nind 2020-10-05 19:34:53 UTC
This might be an edge case, but if you:
- Partially pay a charge (pay $5 off a $15 charge)
- Pay the rest of the charge (pay $10 off a $15 charge)
- Void one of the payments for the charge
- Then you cannot cancel the charge
- This option comes back when you void the other partial payment

My notes from testing;
- updatedatabase required
Comment 15 Martin Renvoize 2020-10-06 07:08:18 UTC
(In reply to David Nind from comment #14)
> This might be an edge case, but if you:
> - Partially pay a charge (pay $5 off a $15 charge)
> - Pay the rest of the charge (pay $10 off a $15 charge)
> - Void one of the payments for the charge
> - Then you cannot cancel the charge
> - This option comes back when you void the other partial payment
> 
> My notes from testing;
> - updatedatabase required

I believe this is the expected behaviour. You can only cancel a charge if it has not been paid off (partially or fully). So the action of voiding all payments against it would make it act as though that was the case and as such it should be cancellable.
Comment 16 Martin Renvoize 2020-10-06 07:08:26 UTC
Thanks for testing :)
Comment 17 David Nind 2020-10-06 08:08:33 UTC
Thanks Martin!
Comment 18 Katrin Fischer 2020-10-14 23:00:08 UTC
I am sorry, but this currently no longer applies:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 24603: Allow to cancel charges in patron accounting
Applying: Bug 24603: (follow-up) Update to double entry accounting
error: 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 24603: (follow-up) Update to double entry accounting
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-24603-follow-up-Update-to-double-entry-account-FBn5S8.patch

I'd also love an explanation on how void, cancel and other options differ! (little confused there)
Comment 19 Martin Renvoize 2020-10-15 08:24:36 UTC
Grr, that sha1 issue is a nightmare to solve.. I'll stick it on a branch for you instead, that easier: https://gitlab.com/mrenvoize/Koha/-/tree/bug_24603
Comment 20 Martin Renvoize 2020-10-15 08:50:41 UTC
(In reply to Katrin Fischer from comment #18)
> 
> I'd also love an explanation on how void, cancel and other options differ!
> (little confused there)

OK, I must admit, I did wonder this myself and had a conversation with Julian about it.. must have been IRC as I don't see it in the comments above.

So..

Void: This can only be applied to 'payments' and it will reverse the whole payment process and affect the payment line amounts itself. If the payment has been applied against any debts, then those debts will have their amounts reset to the value they had before the payment was applied to them.  The final payment line will read 'VOID' as it's status and will have an amount and amountoutstanding of '0'.. It's basically meant for "correcting mistakes". (I don't like this method as at present it's the only one that doesn't respect double-entry accounting as it changes the accountline in question directly without adding a distinct 'VOID' line to the system to offset it.)

Cancel: This can only be applied to 'charges' and only charges that have not yet had any payment/credit applied against them.

Other Options (May as well list them all):
Forgive - Is automatically applied to 'overdue charges' depending on settings; for example, if the item is marked as lost and lost charge is added instead.
Discount - Can only be applied to 'charges', records a discount line that is applied against the outstanding amount on the charge. Used to give a patron an agree discount on the list price of the charge. (Currently this is manually added)
Pay - Can only be applied to 'charges', used to pay either the whole or part of the outstanding debt
Writeoff - Can only be applied to 'charges', records a 'writeoff' line that is applied against the list price of the charge. In British libraries at least, this signifies when they've 'Given up' on getting the money from the patron.
Payout - Can only be applied to 'credits', where the patron has excess credit on their account for one reason or another (Perhaps they paid a fine in full before a backdated return was added). Allows the library to pay the credit back out to the patron.
Refund - Can be applied to a debt that has been paid off already. This is used to grant a patron credit/payout a cash amount when the library has decided they have wrongly charged a patron for some reason and the patron has already paid.
Comment 21 Katrin Fischer 2020-10-15 10:22:08 UTC
Hi Martin, thx! Adding the manual keyword here as I tihnk your explanations would be great to have in there.
Comment 22 Katrin Fischer 2020-10-15 20:52:31 UTC
Created attachment 111794 [details] [review]
Bug 24603: Allow to cancel charges in patron accounting

There is already a button to void a payment. It should be possible to
cancel a charge too.

This patch adds a button in patron's accounting section (Transactions
tab) that allow to cancel charges.
Charges that have been fully or partially paid cannot be cancelled.

It also fixes Koha::Account::Line::is_credit by looking at
credit_type_code instead of amount (amount can be 0 for voided payments)

It also fixes the tests for Koha::Account::Line::void when database does
not contain the borrowernumber 51 (the default in
t::lib::Mocks::mock_userenv)

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. Create another manual invoice
5. Pay it (partially or fully)
6. Notice that the 'Cancel charge' button is not available
7. Void the payment
8. 'Cancel charge' button is available again. Click on it and verify
   that it still works
9. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=24063
Comment 23 Katrin Fischer 2020-10-15 20:52:37 UTC
Created attachment 111795 [details] [review]
Bug 24603: (follow-up) Update to double entry accounting

This patch updates the logic to create a cancellation accountline and
apply it to the charge line so we correction record the transaction in
terms of double entry accounting standards.

Test plan:
1. Go to a patron's accounting section
2. Create a manual invoice
3. In Transactions tab, you should see a 'Cancel charge' button. Click
   on it. It should now be marked as cancelled
4. A cancellation line should be associated with the original charge.
5. Create another manual invoice
6. Pay it (partially or fully)
7. Notice that the 'Cancel charge' button is not available
8. Void the payment
9. 'Cancel charge' button is available again. Click on it and verify
   that it still works
10. prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=24063
Comment 24 Katrin Fischer 2020-10-15 20:52:43 UTC
Created attachment 111796 [details] [review]
Bug 24603: Fix number of unit tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=24063
Comment 25 Katrin Fischer 2020-10-15 20:52:49 UTC
Created attachment 111797 [details] [review]
Bug 24603: (QA follow-up) Make the new credit type translatable and shorten description a bit

Adding the new offset type and credit type to the include so it can be translated.

Also suggest shortening the db description a bit.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=24063
Comment 26 Martin Renvoize 2020-10-16 15:26:24 UTC
Created attachment 111914 [details] [review]
Bug 24603: (QA follow-up) Add translation to OPAC

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>