Bug 26536

Summary: "Writeoff/Pay selected" deducts from old unpaid debts first
Product: Koha Reporter: Vinod <mishravk79>
Component: Fines and feesAssignee: Martin Renvoize <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: andrew, george, jonathan.druart, kyle, lucas, martin.renvoize, mfuerst, sally.healey, victor
Version: 20.05   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26076
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07
Bug Depends on: 26076    
Bug Blocks:    
Attachments: Bug 26536: Fix incorrectly scoped variable
Bug 26536: WRITEOFF not writeoff
Bug 25636: Assignment fix
Bug 26536: Fix incorrectly scoped variable
Bug 26536: WRITEOFF not writeoff
Bug 25636: Assignment fix
Bug 26536: Fix incorrectly scoped variable
Bug 26536: WRITEOFF not writeoff
Bug 25636: Assignment fix
Bug 26536: Assignment fix
[20.05.x] Bug 26536: Fix incorrectly scoped variable
[20.05.x] Bug 26536: WRITEOFF not writeoff
[20.05.x] Bug 26536: Assignment fix

Description Vinod 2020-09-25 11:33:15 UTC
Hello

In version 20.05.04 Writeoff selected fine tab does not write off only selected instead it applied on oldest unpaid fine first.

Test Plan

1. Two returned book/s with unpaid fine
2. Try to write off lasted returned book with "Writeoff selected"
3. You will observe fine deducted from oldest unpaid book.

Vinod
Comment 1 Martin Renvoize 2020-10-02 15:25:56 UTC
Hmm.. I'm wondering whether this is a result of bug 26076.

Testing.
Comment 2 Martin Renvoize 2020-10-05 08:11:23 UTC
Created attachment 111209 [details] [review]
Bug 26536: Fix incorrectly scoped variable

During work on bug 26076 a variable scope mistake was made when
reverting the logic to use distinct queries for selected accountlines.

This patch restores the script scope of the varaible by removing the
superflous 'my' declaration.

Test plan
1/ Add a set of manual invoices
2/ Select a subset of those manual invoices to 'Writeoff selected'
3/ Confirm that only those selected invoices have been written off.
Comment 3 Martin Renvoize 2020-10-05 08:14:48 UTC
Created attachment 111210 [details] [review]
Bug 26536: WRITEOFF not writeoff

There is inconsistency in variable case between the controller script
and the template. This patch corrects those inconsistencies.
Comment 4 Martin Renvoize 2020-10-05 08:14:52 UTC
Created attachment 111211 [details] [review]
Bug 25636: Assignment fix

Whilst working on this bug I found a weird inconsistent variable assignment line
Comment 5 Martin Renvoize 2020-10-05 08:15:28 UTC
Only the first patch is actually required, the subsequent two patches are just cleanup I spotted whilst working on the bug
Comment 6 PTFS Europe Sandboxes 2020-10-06 09:06:46 UTC
Created attachment 111268 [details] [review]
Bug 26536: Fix incorrectly scoped variable

During work on bug 26076 a variable scope mistake was made when
reverting the logic to use distinct queries for selected accountlines.

This patch restores the script scope of the varaible by removing the
superflous 'my' declaration.

Test plan
1/ Add a set of manual invoices
2/ Select a subset of those manual invoices to 'Writeoff selected'
3/ Confirm that only those selected invoices have been written off.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 7 PTFS Europe Sandboxes 2020-10-06 09:06:52 UTC
Created attachment 111269 [details] [review]
Bug 26536: WRITEOFF not writeoff

There is inconsistency in variable case between the controller script
and the template. This patch corrects those inconsistencies.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 8 PTFS Europe Sandboxes 2020-10-06 09:06:56 UTC
Created attachment 111270 [details] [review]
Bug 25636: Assignment fix

Whilst working on this bug I found a weird inconsistent variable assignment line

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

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 9 Katrin Fischer 2020-10-14 23:21:54 UTC
Created attachment 111658 [details] [review]
Bug 26536: Fix incorrectly scoped variable

During work on bug 26076 a variable scope mistake was made when
reverting the logic to use distinct queries for selected accountlines.

This patch restores the script scope of the varaible by removing the
superflous 'my' declaration.

Test plan
1/ Add a set of manual invoices
2/ Select a subset of those manual invoices to 'Writeoff selected'
3/ Confirm that only those selected invoices have been written off.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2020-10-14 23:21:59 UTC
Created attachment 111659 [details] [review]
Bug 26536: WRITEOFF not writeoff

There is inconsistency in variable case between the controller script
and the template. This patch corrects those inconsistencies.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2020-10-14 23:22:05 UTC
Created attachment 111660 [details] [review]
Bug 25636: Assignment fix

Whilst working on this bug I found a weird inconsistent variable assignment line

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

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Jonathan Druart 2020-10-15 10:55:31 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 13 Jonathan Druart 2020-10-15 10:56:52 UTC
Created attachment 111701 [details] [review]
Bug 26536: Assignment fix

Whilst working on this bug I found a weird inconsistent variable assignment line

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

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

JD: Amended commit message: fix bug number
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Lucas Gass 2020-11-13 15:36:16 UTC
does not apply cleanly to 20.05.x, no backport
Comment 15 Katrin Fischer 2020-11-13 19:55:55 UTC
As this was initially reported against 20.05.04, could we have a rebase and backport?
Comment 16 Martha Fuerst 2020-12-17 19:47:33 UTC
Not being able to use Write Off Selected to select specific fines to write off defeats the purpose of even having the function to select fines for payment/write off.

It is essential to being able to clearly communicate to patrons why/for what materials they owe fines.

Losing this functionality has made for a lot of mess on the backend that our admin/business office is having to deal with. Please rebase and backport to 20.05.

Thanks!
Comment 17 George Williams (NEKLS) 2020-12-17 20:24:29 UTC
(In reply to Martha Fuerst from comment #16)
> Not being able to use Write Off Selected to select specific fines to write
> off defeats the purpose of even having the function to select fines for
> payment/write off.
> 
> It is essential to being able to clearly communicate to patrons why/for what
> materials they owe fines.
> 
> Losing this functionality has made for a lot of mess on the backend that our
> admin/business office is having to deal with. Please rebase and backport to
> 20.05.
> 
> Thanks!

+1
Comment 18 Martin Renvoize 2020-12-18 09:03:59 UTC
Created attachment 114508 [details] [review]
[20.05.x] Bug 26536: Fix incorrectly scoped variable

During work on bug 26076 a variable scope mistake was made when
reverting the logic to use distinct queries for selected accountlines.

This patch restores the script scope of the varaible by removing the
superflous 'my' declaration.

Test plan
1/ Add a set of manual invoices
2/ Select a subset of those manual invoices to 'Writeoff selected'
3/ Confirm that only those selected invoices have been written off.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Martin Renvoize 2020-12-18 09:04:02 UTC
Created attachment 114509 [details] [review]
[20.05.x] Bug 26536: WRITEOFF not writeoff

There is inconsistency in variable case between the controller script
and the template. This patch corrects those inconsistencies.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Martin Renvoize 2020-12-18 09:04:06 UTC
Created attachment 114510 [details] [review]
[20.05.x] Bug 26536: Assignment fix

Whilst working on this bug I found a weird inconsistent variable assignment line

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
JD: Amended commit message: fix bug number
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Andrew Fuerste-Henry 2020-12-18 14:20:29 UTC
Pushed to 20.05.x for 20.05.07
Comment 22 Victor Grousset/tuxayo 2020-12-18 21:32:08 UTC
Got a conflict when backporting from 20.05.x to 19.11.x

Tried to fix by selecting the 3rd section (Bug 26536: Assignment fix)

<<<<<<< HEAD
                $payment_id = $account->pay(
||||||| parent of 6d302b741c (Bug 26536: Assignment fix)
                $pay_result = $payment_id = $account->pay(
=======
                $pay_result = $account->pay(
>>>>>>> 6d302b741c (Bug 26536: Assignment fix)
                    {
                        amount       => $total_paid,
                        library_id   => $library_id,
                        note         => $note,
                        payment_type => $payment_type,
                        interface    => C4::Context->interface,
                        payment_type => $payment_type,
                        cash_register => $registerid
                    }
                );
                $payment_id = $pay_result->{payment_id};
            }

            print $input->redirect("/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber&payment_id=$payment_id&change_given=$change_given");
        }
    }




But when writing off, I got this error:
Could not compile /kohadevbox/koha/members/paycollect.pl: Global symbol "$pay_result" requires explicit package name

It's not clear either if 19.11.x is affected. I'm not sure I did the test plan right to confirm the issue.
Does anyone have hints?
Comment 23 Martha Fuerst 2020-12-21 12:35:31 UTC
It was not an issue for us in 19.11 at Huntsville-Madison County Public.
Comment 24 Victor Grousset/tuxayo 2020-12-21 12:37:14 UTC
Thanks for the confirmation :)