Bug 26536 - "Writeoff/Pay selected" deducts from old unpaid debts first
Summary: "Writeoff/Pay selected" deducts from old unpaid debts first
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: 20.05
Hardware: All All
: P5 - low critical (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
: 28645 (view as bug list)
Depends on: 26076
Blocks: 28645
  Show dependency treegraph
 
Reported: 2020-09-25 11:33 UTC by Vinod
Modified: 2022-12-12 21:24 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07


Attachments
Bug 26536: Fix incorrectly scoped variable (1.15 KB, patch)
2020-10-05 08:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26536: WRITEOFF not writeoff (1.47 KB, patch)
2020-10-05 08:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25636: Assignment fix (1.26 KB, patch)
2020-10-05 08:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26536: Fix incorrectly scoped variable (1.21 KB, patch)
2020-10-06 09:06 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 26536: WRITEOFF not writeoff (1.54 KB, patch)
2020-10-06 09:06 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 25636: Assignment fix (1.33 KB, patch)
2020-10-06 09:06 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 26536: Fix incorrectly scoped variable (1.27 KB, patch)
2020-10-14 23:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26536: WRITEOFF not writeoff (1.59 KB, patch)
2020-10-14 23:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25636: Assignment fix (1.39 KB, patch)
2020-10-14 23:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26536: Assignment fix (1.45 KB, patch)
2020-10-15 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
[20.05.x] Bug 26536: Fix incorrectly scoped variable (1.28 KB, patch)
2020-12-18 09:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
[20.05.x] Bug 26536: WRITEOFF not writeoff (1.71 KB, patch)
2020-12-18 09:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
[20.05.x] Bug 26536: Assignment fix (1.45 KB, patch)
2020-12-18 09:04 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 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 :)
Comment 25 Alexander Borkowski 2021-12-01 13:11:29 UTC
We do have this exact issue on 19.11 unfortunately. I guess a backport to 19.11 is unlikely at this stage in its lifecycle so I will just patch our local version based on the diffs provided here.
Comment 26 Victor Grousset/tuxayo 2021-12-03 02:45:01 UTC
> I guess a backport to 19.11 is unlikely at this stage in its lifecycle

Indeed 19.11.x is still supported but only for security fixes.

But the main issues is to be able to make a working backport. See comment 22
Comment 27 Alexander Borkowski 2021-12-03 09:20:37 UTC
Applying only attachments 114508 and 114509 went without problems and resolved the issue for our 19.11.23 installation. I think attachment 114510 [details] [review] which gave problems in comment 22 is not necessary for 19.11 as the return value of Koha::Account::pay changed only in attachment 100185 [details] [review] to bug 23501 which was not backported to 19.11.
Comment 28 Alexander Borkowski 2021-12-03 09:37:12 UTC
*** Bug 28645 has been marked as a duplicate of this bug. ***
Comment 29 Victor Grousset/tuxayo 2021-12-03 11:49:02 UTC
Thanks a lot :D glad you figured it out. That could help others still using 19.11