Bug 28664 - One should not be able to issue a refund against a VOID accountline
Summary: One should not be able to issue a refund against a VOID accountline
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Martin Renvoize (ashimema)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 27971
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-07-05 18:32 UTC by Martin Renvoize (ashimema)
Modified: 2024-09-18 03:22 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes VOID transactions for patron accounting entries so that the 'Issue refund' button is not available.
Version(s) released in:
24.11.00,24.05.02,23.11.07
Circulation function:


Attachments
Bug 28664: Prevent redunfs against void lines (2.48 KB, patch)
2021-07-05 18:37 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28664: Prevent refunds against void lines (2.51 KB, patch)
2021-11-11 12:09 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28664: Prevent refunds against void lines (2.56 KB, patch)
2021-11-11 15:52 UTC, David Nind
Details | Diff | Splinter Review
Bug 28664: Prevent refunds against void lines (2.67 KB, patch)
2024-06-27 16:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28664: (follow-up) Throw exception if debt if VOID (2.46 KB, patch)
2024-06-27 16:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28664: Prevent refunds against void lines (2.77 KB, patch)
2024-06-28 09:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28664: (follow-up) Throw exception if debt if VOID (2.56 KB, patch)
2024-06-28 09:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-07-05 18:32:13 UTC
When adding the VOID account line type in bug 27971 I neglected to update the logic for the display of the 'Issue refund' button on the patron transactions page.
Comment 1 Martin Renvoize (ashimema) 2021-07-05 18:37:05 UTC
Created attachment 122587 [details] [review]
Bug 28664: Prevent redunfs against void lines

With the introduction of double entry accounting for VOID actions, we
need to add an additional filter to the 'Issue refund' button appearance

Test plan
1/ Add a debt
2/ Pay the debt
3/ Void the payment
4/ Confirm that with the patch applied the 'Issue refund' button doesn
not appear on the 'Void' accountline.
Comment 2 David Nind 2021-11-10 19:56:49 UTC
Patch doesn't apply 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 28664: Prevent redunfs against void lines
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 28664: Prevent redunfs against void lines
Comment 3 Martin Renvoize (ashimema) 2021-11-11 12:09:04 UTC
Created attachment 127537 [details] [review]
Bug 28664: Prevent refunds against void lines

With the introduction of double entry accounting for VOID actions, we
need to add an additional filter to the 'Issue refund' button appearance

Test plan
1/ Add a debt
2/ Pay the debt
3/ Void the payment
4/ Confirm that with the patch applied the 'Issue refund' button doesn
not appear on the 'Void' accountline.
Comment 4 Martin Renvoize (ashimema) 2021-11-11 12:09:51 UTC
Always moving is our Koha!... :)

Rebased, thanks for looking David.
Comment 5 David Nind 2021-11-11 15:52:44 UTC
Created attachment 127548 [details] [review]
Bug 28664: Prevent refunds against void lines

With the introduction of double entry accounting for VOID actions, we
need to add an additional filter to the 'Issue refund' button appearance

Test plan
1/ Add a debt
2/ Pay the debt
3/ Void the payment
4/ Confirm that with the patch applied the 'Issue refund' button doesn
not appear on the 'Void' accountline.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Tomás Cohen Arazi (tcohen) 2021-11-11 20:44:12 UTC
The patch itself is simple and does the job. But I think we are a bit fragile here. Too much calculation in the controller/templates.

Can we add something like:

Koha::Account::Line->can_be_reduced
Koha::Account::Line->refund

and reuse it both in the templates and controllers? I would like the code to explode if tricked to try refunding a 'VOID' line.

I can help coding or writing the tests if required.

If this is being worked on somewhere else, please point me to the right direction.
Comment 7 Martin Renvoize (ashimema) 2024-06-27 15:41:59 UTC
I think this should really have gone in.. I don't really have the time spare capacity to build the new methods on top
Comment 8 Martin Renvoize (ashimema) 2024-06-27 16:23:04 UTC
Created attachment 168205 [details] [review]
Bug 28664: Prevent refunds against void lines

With the introduction of double entry accounting for VOID actions, we
need to add an additional filter to the 'Issue refund' button appearance

Test plan
1/ Add a debt
2/ Pay the debt
3/ Void the payment
4/ Confirm that with the patch applied the 'Issue refund' button doesn
not appear on the 'Void' accountline.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Martin Renvoize (ashimema) 2024-06-27 16:23:06 UTC
Created attachment 168206 [details] [review]
Bug 28664: (follow-up) Throw exception if debt if VOID

This patch adds an exception when an attempt is made to refund against a
VOID debit.

Test plan
1) Run the included unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize (ashimema) 2024-06-27 16:23:49 UTC
I've added an exception that follows how other exceptions for similar use cases are raised.

Hopefully this, with it's corresponding unit test, is enough to get us through QA
Comment 11 Marcel de Rooy 2024-06-28 09:36:46 UTC
Created attachment 168228 [details] [review]
Bug 28664: Prevent refunds against void lines

With the introduction of double entry accounting for VOID actions, we
need to add an additional filter to the 'Issue refund' button appearance

Test plan
1/ Add a debt
2/ Pay the debt
3/ Void the payment
4/ Confirm that with the patch applied the 'Issue refund' button doesn
not appear on the 'Void' accountline.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2024-06-28 09:36:48 UTC
Created attachment 168229 [details] [review]
Bug 28664: (follow-up) Throw exception if debt if VOID

This patch adds an exception when an attempt is made to refund against a
VOID debit.

Test plan
1) Run the included unit test

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Katrin Fischer 2024-06-28 11:50:09 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 14 Lucas Gass (lukeg) 2024-07-19 15:55:23 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 15 Fridolin Somers 2024-07-22 10:03:38 UTC
Pushed to 23.11.x for 23.11.07
Comment 16 Wainui Witika-Park 2024-09-18 03:22:07 UTC
Not backporting to 23.05.x unless requested