Bug 22511 - Koha::Account::Line->void loses the original type of the credit
Summary: Koha::Account::Line->void loses the original type of the credit
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 18790 22564
Blocks: 22200
  Show dependency treegraph
 
Reported: 2019-03-14 10:12 UTC by Martin Renvoize
Modified: 2020-01-06 20:14 UTC (History)
7 users (show)

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


Attachments
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-03-15 14:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.90 KB, patch)
2019-03-15 14:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.90 KB, patch)
2019-03-22 12:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-03-22 12:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.65 KB, patch)
2019-03-22 12:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.90 KB, patch)
2019-03-22 16:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-03-22 16:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.65 KB, patch)
2019-03-22 16:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.90 KB, patch)
2019-03-27 16:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-03-27 16:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.65 KB, patch)
2019-03-27 16:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.80 KB, patch)
2019-03-27 16:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.99 KB, patch)
2019-03-29 15:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-03-29 15:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.39 KB, patch)
2019-03-29 15:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.80 KB, patch)
2019-03-29 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.99 KB, patch)
2019-04-09 16:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-04-09 16:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.39 KB, patch)
2019-04-09 16:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.80 KB, patch)
2019-04-09 16:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (7.06 KB, patch)
2019-04-09 17:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: Update tests (7.06 KB, patch)
2019-04-09 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.46 KB, patch)
2019-04-09 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.46 KB, patch)
2019-04-09 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.87 KB, patch)
2019-04-09 17:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: (follow-up) Add DB Update (1.86 KB, patch)
2019-04-10 09:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Add DB Update (1.93 KB, patch)
2019-04-10 10:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22511: Update tests (6.99 KB, patch)
2019-04-11 18:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-04-11 18:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.74 KB, patch)
2019-04-11 18:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.80 KB, patch)
2019-04-11 18:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Add DB Update (1.86 KB, patch)
2019-04-11 18:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (6.99 KB, patch)
2019-04-12 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.39 KB, patch)
2019-04-12 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.40 KB, patch)
2019-04-12 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.80 KB, patch)
2019-04-12 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (follow-up) Add DB Update (1.86 KB, patch)
2019-04-12 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Update tests (7.06 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: Update void method to use status (1.44 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: Update UI to use accountlines.status (5.51 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: (follow-up) Fix test plans (1.86 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: (follow-up) Add DB Update (1.92 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: (QA follow-up) Fix update database (1.26 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: (QA follow-up) Refactor status description in templates (6.62 KB, patch)
2019-04-25 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22511: (QA follow-up) Fix update database (1.33 KB, patch)
2019-04-25 08:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: (QA follow-up) Refactor status description in templates (6.67 KB, patch)
2019-04-25 08:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22511: Fix type role->roll (794 bytes, patch)
2019-04-25 11:10 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-03-14 10:12:06 UTC
The `void` routine in Koha::Account::Lines does restrict one to only being able to void a 'credit' accountline, however it does not distinguish between accounttypes.  We do however, change the accounttype as part of the void routine to VOID and as such we may be loosing data, and thus audit trail, here.

Credits can take the form of 'Payment', 'Writeoff', 'Lost Item Return', 'Fine Forgiven', 'Forgiven' and finally 'Credit'.
Comment 1 Martin Renvoize 2019-03-14 10:29:26 UTC
Actually.. we should be able to get to this original data via the offsets, walking backwards to find the first 'creation' offset... we don't however do that as yet and as such always display "Payment, voided" to the borrower for all such transactions.. this could be confusing to the end user?
Comment 2 Kyle M Hall 2019-03-14 19:44:18 UTC
I actually consider this a feature. It's always possible that something like a writeoff was done accidentally and needs to be voided. The rest of the credit types could be argued, but I don't since great utility in preventing them from being voided.
Comment 3 Martin Renvoize 2019-03-15 08:24:19 UTC
I don't think I gave this a good title.. I'll change it.

I didn't really mean that's it's a bad thing that one can void all the different types of credit, but more that it's a bad thing that you loose the information as to what type of credit you have voided.

In my opinion 'void' is a valid 'state' of any credit (and for that matter, I don't see why one shouldn't be able to void a debit too). 

See bug 22512 for more explanation, I may mark this bug as a duplicate at some point, or use to to encapsulate the corresponding work for the void part of adding states.
Comment 4 Kyle M Hall 2019-03-15 10:17:38 UTC
(In reply to Martin Renvoize from comment #3)
> I don't think I gave this a good title.. I'll change it.
> 
> I didn't really mean that's it's a bad thing that one can void all the
> different types of credit, but more that it's a bad thing that you loose the
> information as to what type of credit you have voided.
> 
> In my opinion 'void' is a valid 'state' of any credit (and for that matter,
> I don't see why one shouldn't be able to void a debit too). 
> 
> See bug 22512 for more explanation, I may mark this bug as a duplicate at
> some point, or use to to encapsulate the corresponding work for the void
> part of adding states.

Yes, I agree completely. It would make more sense to have a 'voided' column, or a 'status' column that could have the state 'voided'.

Are there other states we should consider for payments? A 'state' column could be re-used for 22512 which cuts now on the number of additional columns needed. What do you think?
Comment 5 Martin Renvoize 2019-03-15 14:20:01 UTC
Created attachment 86674 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-03-15 14:20:05 UTC
Created attachment 86675 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-03-15 14:21:52 UTC
I'm not also wondering about the logic of updating both 'amount' and 'amountoutstanding' here.. shouldn't we perhaps keep the 'amount' as the original value so one can see not only 'What' was voided but also 'How much'.
Comment 8 Kyle M Hall 2019-03-15 17:53:14 UTC
(In reply to Martin Renvoize from comment #7)
> I'm not also wondering about the logic of updating both 'amount' and
> 'amountoutstanding' here.. shouldn't we perhaps keep the 'amount' as the
> original value so one can see not only 'What' was voided but also 'How much'.

The logic behind the current behavior is to avoid the need to specifically remove VOIDs from reports when summing amounts. That data should be available the account offsets table.

That being said, I'm not opposed to a change in this behavior.
Comment 9 Kyle M Hall 2019-03-18 11:45:58 UTC
This needs a bit of UI logic so new voided payments and old VOID types look the same in the UI. In addition, it would make sends to update accoulines with an accounttype of VOID to also have a status of VOID.

Since future voided payments will retain the original accountype, it would be reasonable for Koha to show the original account type *and* show that it was voided.
Comment 10 Martin Renvoize 2019-03-20 13:39:30 UTC
Totally agree.. I wasn't fully ready for testing yet.. my bad for setting it to NSO.. I meant to leave it as NEW until I'd done more here.
Comment 11 Kyle M Hall 2019-03-20 13:47:14 UTC
(In reply to Martin Renvoize from comment #10)
> Totally agree.. I wasn't fully ready for testing yet.. my bad for setting it
> to NSO.. I meant to leave it as NEW until I'd done more here.

NP! :)
Comment 12 Martin Renvoize 2019-03-22 12:04:02 UTC
Created attachment 86903 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2019-03-22 12:04:05 UTC
Created attachment 86904 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2019-03-22 12:04:09 UTC
Created attachment 86905 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2019-03-22 16:45:37 UTC
Created attachment 86915 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2019-03-22 16:45:40 UTC
Created attachment 86916 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2019-03-22 16:45:43 UTC
Created attachment 86917 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2019-03-27 16:18:15 UTC
Created attachment 87088 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2019-03-27 16:18:18 UTC
Created attachment 87089 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2019-03-27 16:18:21 UTC
Created attachment 87090 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2019-03-27 16:18:25 UTC
Created attachment 87091 [details] [review]
Bug 22511: (follow-up) Fix test plans

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2019-03-29 15:05:50 UTC
Created attachment 87221 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2019-03-29 15:05:53 UTC
Created attachment 87222 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2019-03-29 15:05:56 UTC
Created attachment 87223 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2019-03-29 15:06:00 UTC
Created attachment 87224 [details] [review]
Bug 22511: (follow-up) Fix test plans

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2019-04-09 16:27:02 UTC
Created attachment 87632 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2019-04-09 16:27:05 UTC
Created attachment 87633 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2019-04-09 16:27:08 UTC
Created attachment 87634 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2019-04-09 16:27:12 UTC
Created attachment 87635 [details] [review]
Bug 22511: (follow-up) Fix test plans

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize 2019-04-09 16:27:34 UTC
Freshly re-based and ready for SO
Comment 31 Kyle M Hall 2019-04-09 17:21:40 UTC
Created attachment 87650 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2019-04-09 17:24:00 UTC
Created attachment 87651 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2019-04-09 17:24:12 UTC
Created attachment 87652 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2019-04-09 17:24:15 UTC
Created attachment 87653 [details] [review]
Bug 22511: Update UI to use accountlines.status

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2019-04-09 17:24:18 UTC
Created attachment 87654 [details] [review]
Bug 22511: (follow-up) Fix test plans

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Martin Renvoize 2019-04-10 09:07:52 UTC
Created attachment 87704 [details] [review]
Bug 22511: (follow-up) Add DB Update

This update takes old VOID accountlines and attempts to restore the
original accounttypes using the offsets and set the status to 'VOID'

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Kyle M Hall 2019-04-10 10:58:45 UTC
Created attachment 87706 [details] [review]
Bug 22511: (follow-up) Add DB Update

This update takes old VOID accountlines and attempts to restore the
original accounttypes using the offsets and set the status to 'VOID'

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Martin Renvoize 2019-04-11 18:43:01 UTC
Created attachment 87796 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2019-04-11 18:43:04 UTC
Created attachment 87797 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2019-04-11 18:43:08 UTC
Created attachment 87798 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2019-04-11 18:43:11 UTC
Created attachment 87799 [details] [review]
Bug 22511: (follow-up) Fix test plans

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2019-04-11 18:43:16 UTC
Created attachment 87800 [details] [review]
Bug 22511: (follow-up) Add DB Update

This update takes old VOID accountlines and attempts to restore the
original accounttypes using the offsets and set the status to 'VOID'

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize 2019-04-11 18:43:42 UTC
Freshly rebased
Comment 44 Martin Renvoize 2019-04-12 08:54:32 UTC
Created attachment 87869 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2019-04-12 08:54:36 UTC
Created attachment 87870 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2019-04-12 08:54:39 UTC
Created attachment 87871 [details] [review]
Bug 22511: Update UI to use accountlines.status

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2019-04-12 08:54:42 UTC
Created attachment 87872 [details] [review]
Bug 22511: (follow-up) Fix test plans

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2019-04-12 08:54:46 UTC
Created attachment 87873 [details] [review]
Bug 22511: (follow-up) Add DB Update

This update takes old VOID accountlines and attempts to restore the
original accounttypes using the offsets and set the status to 'VOID'

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2019-04-12 08:56:44 UTC
Rebased again.. things are moving fast here at the moment :)
Comment 50 Josef Moravec 2019-04-25 08:37:19 UTC
Created attachment 88707 [details] [review]
Bug 22511: Update tests

Updated tests to check for 'status' change and fixed 'accounttype' and
moved from t/db_dependent/Accounts.t to the more appropriate
t/db_dependent/Koha/Account/Lines.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 51 Josef Moravec 2019-04-25 08:37:23 UTC
Created attachment 88708 [details] [review]
Bug 22511: Update void method to use status

We added a 'status' field to 'accountlines' in bug 22512 to allow for
more fined grained control over 'types' and 'states'.

This patch updates the void method to utilise this new field so we may
keep the original credit type for later auditing should it be required.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 52 Josef Moravec 2019-04-25 08:37:27 UTC
Created attachment 88709 [details] [review]
Bug 22511: Update UI to use accountlines.status

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 53 Josef Moravec 2019-04-25 08:37:30 UTC
Created attachment 88710 [details] [review]
Bug 22511: (follow-up) Fix test plans

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 54 Josef Moravec 2019-04-25 08:37:34 UTC
Created attachment 88711 [details] [review]
Bug 22511: (follow-up) Add DB Update

This update takes old VOID accountlines and attempts to restore the
original accounttypes using the offsets and set the status to 'VOID'

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 55 Josef Moravec 2019-04-25 08:37:37 UTC
Created attachment 88712 [details] [review]
Bug 22511: (QA follow-up) Fix update database

Test plan:
Run db update (and have some voided accountlines)
--> without patch it fails with error message similar to:
    Can't use string ("Payment") as a HASH ref while "strict refs" in use at
    (eval 1393) line 19.
--> with patch it runs as expected

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 56 Josef Moravec 2019-04-25 08:37:41 UTC
Created attachment 88713 [details] [review]
Bug 22511: (QA follow-up) Refactor status description in templates

Test plan:

Before this patch, some accounttypes does not show status (for example
        voided payment)

After this patch, the status is always generated - defaults to empty
string

This behaviour should be same in intranet and opac

In OPAC templates there was even bug (used variable account instead of
        ACCOUNt_LINE), so the status was not shown at all

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 57 Josef Moravec 2019-04-25 08:39:16 UTC
Martin, it would be nice if you could sign-off my follow-ups ;)
Comment 58 Martin Renvoize 2019-04-25 08:44:08 UTC
Created attachment 88714 [details] [review]
Bug 22511: (QA follow-up) Fix update database

Test plan:
Run db update (and have some voided accountlines)
--> without patch it fails with error message similar to:
    Can't use string ("Payment") as a HASH ref while "strict refs" in use at
    (eval 1393) line 19.
--> with patch it runs as expected

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2019-04-25 08:44:11 UTC
Created attachment 88715 [details] [review]
Bug 22511: (QA follow-up) Refactor status description in templates

Test plan:

Before this patch, some accounttypes does not show status (for example
        voided payment)

After this patch, the status is always generated - defaults to empty
string

This behaviour should be same in intranet and opac

In OPAC templates there was even bug (used variable account instead of
        ACCOUNt_LINE), so the status was not shown at all

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Martin Renvoize 2019-04-25 08:45:00 UTC
Thanks for the followups, nice bit of tidying and makes great sense.
Comment 61 Josef Moravec 2019-04-25 08:45:38 UTC
(In reply to Martin Renvoize from comment #59)

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

Thanks Martin!
Comment 62 Nick Clemens 2019-04-25 11:10:45 UTC
Created attachment 88748 [details] [review]
Bug 22511: Fix type role->roll
Comment 63 Nick Clemens 2019-04-25 11:11:19 UTC
(In reply to Nick Clemens from comment #62)
> Created attachment 88748 [details] [review] [review]
> Bug 22511: Fix type role->roll

Will fix List item->Lost item in the DB update on pushing
Comment 64 Josef Moravec 2019-04-25 11:23:58 UTC
(In reply to Nick Clemens from comment #63)
> (In reply to Nick Clemens from comment #62)
> > Created attachment 88748 [details] [review] [review] [review]
> > Bug 22511: Fix type role->roll
> 
> Will fix List item->Lost item in the DB update on pushing

Silly me, I oversee it... thanks Nick
Comment 65 Nick Clemens 2019-04-25 11:56:34 UTC
Awesome work all!

Pushed to master for 19.05
Comment 66 Martin Renvoize 2019-04-26 15:27:02 UTC
Enhancement will not be backported to 18.11.x series.