Bug 17702

Summary: Create configuration for account credit types
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: Fines and feesAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: 1joynelson, black23, chris.kirby, gmcharlt, julian.maurice, katrin.fischer, kyle.m.hall, lisettepalouse+koha, martin.renvoize, rbit, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This adds a new configuration page for credit types to the administration module. It shows all internal credit types used by Koha and allows to configure additional credit types. Additional credit types might be used for anonymous transactions with the Point of sale feature.
Version(s) released in:
20.05.00
Bug Depends on: 23805    
Bug Blocks: 23894    
Attachments: Bug 17702: Account types configuration - db changes
Bug 17702: Account types configuration - Manual invoice and credit
Bug 17702: Account types configuration - Admin page
Bug 17702: Add tests
Bug 17702: Account types configuration - db changes
Bug 17702: Account types configuration - Manual invoice and credit
Bug 17702: Account types configuration - Admin page
Bug 17702: Add tests
Bug 17702: Add UI to manage account credit types
Bug 17702: Add UI to manage account credit types
Bug 17702: (QA follow-up) Add new field to kohastructure
Bug 17702: Add UI to manage account credit types
Bug 17702: (QA follow-up) Add new field to kohastructure
Bug 17702: (RM follow-up) Remove use of I18N tt
Bug 17702: (RM follow-up) Remove use of I18N tt

Description Josef Moravec 2016-11-30 11:06:31 UTC
As using the MANUAL_INV for defining fines is not ideal, I tried another approach by defining the two new db tables for credit types and debit types with own administration page. 

Patch will be uploaded soon
Comment 1 Josef Moravec 2016-12-01 12:54:19 UTC
Created attachment 57856 [details] [review]
Bug 17702: Account types configuration - db changes

Test plan:
1) Read the diff and confirm that it does make sense
2) Comment what does not make sense ;)
Comment 2 Josef Moravec 2016-12-01 12:54:26 UTC
Created attachment 57857 [details] [review]
Bug 17702: Account types configuration - Manual invoice and credit
Comment 3 Josef Moravec 2016-12-01 12:54:34 UTC
Created attachment 57858 [details] [review]
Bug 17702: Account types configuration - Admin page

Test plan:
1) Go to admin home, note there is new Account types page in Patrons and
circulation section
2) Go to any other admin page and confirm there is link to Account types
in admin menu as welll
3) Go to Account types page
4) You should see two datatables, one for debit types and one for
credit types, ensure the datatables are working corectly
5) Try to create, edit and delete some debit types, note, that some of
them can't be deleted - they are neede for Koha internally
6) Do the same with credit types
7) Try to add some default amount to internal debits - like Acccount
management fee, New card, ...
8) Go to Patron fines page and play with account type select, when you
change thy account type, the description and amount should change as you
set it on the admin page
9) Try to add some fees and ensure they are inserted corectly
10) Pay the inserted fees
11) Try to add manual credit and ensure it is added corectly
Comment 4 Josef Moravec 2016-12-01 12:55:59 UTC
I attached my current work, but stay with ASSIGNED status till I write tests ;)
Comment 5 Josef Moravec 2016-12-09 12:36:09 UTC
Created attachment 58066 [details] [review]
Bug 17702: Add tests

Test plan:
  prove t/db_dependent/Koha/AccountTypes.t
Comment 6 Chris Kirby 2016-12-10 14:44:31 UTC
Hi Josef,
When I tried to add a new debit type, I got the following error:
DBIx::Class::Storage::DBI::_dbh_execute(): Table 'koha_mykoha.account_debit_types' doesn't exist at /home/ckirby/koha/Koha/Objects.pm line 83
Comment 7 Josef Moravec 2016-12-10 22:04:48 UTC
(In reply to Chris Kirby from comment #6)
> Hi Josef,
> When I tried to add a new debit type, I got the following error:
> DBIx::Class::Storage::DBI::_dbh_execute(): Table
> 'koha_mykoha.account_debit_types' doesn't exist at
> /home/ckirby/koha/Koha/Objects.pm line 83

Did you upgrade your db schema? I forget to mention it in test plan, sorry.
Comment 8 Chris Kirby 2016-12-11 01:21:49 UTC
(In reply to comment #7), 
Hi Josef, thanks for the tip to upgrade the db schema.

When one creates a "manual invoice" for a patron using one of the created debit types, both the debit type code and the description are entered into the "Description of charges." However, for the pre-existing debit types, the debit type description is entered twice in the "description of charges".
Is there any reason for this difference?

When one creates a credit type, there is no field for default amount. Is there any reason for leaving a default amount out of the credit type but keeping it in the debit?
Comment 9 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-12 13:44:46 UTC
(In reply to Josef Moravec from comment #1)
> Created attachment 57856 [details] [review] [review]
> Bug 17702: Account types configuration - db changes
> 
> Test plan:
> 1) Read the diff and confirm that it does make sense
> 2) Comment what does not make sense ;)

According to db changes, is it possible to extend type_code from only 5 chars to (let's say) 10 chars? It would be very useful for some libraries having a longer list of payments not to be so cryptic. For instance, one of them (Czech only - sorry):
mvs	50|MVS
tA4	3|Tisk a kopie A4
t2sA4	4|Tisk a kopie A4 oboustranně
toA4	6|Tisk a kopie A4 + obrázek
tbA4	15|Celobarevná A4
t20A4	2|Tisk a kopie A4 nad 20 kusů
s20A4	3|Tisk a kopie A4 oboustranně nad 20 kusů
b20A4	10|Tisk a kopie A4 barevně nad 20 kusů
tA3	5|Tisk a kopie A3
t2sA3	7|Tisk a kopie A3 oboustranně
toA3	10|Tisk a kopie A3 + obrázek
tbA3	25|Celobarevná A3
t20A3	3|Tisk a kopie A3 nad 20 kusů
s20A3	5|Tisk a kopie A3 oboustranně nad 20 kusů
b20A3	20|Tisk a kopie A3 barevně nad 20 kusů
sken	7|skenování
eursl	3|eurosložka
papsl	5|papírová složka
taskm	3|taška malá
taskv	5|taška velká
prodj	?|prodej
plack	?|placky
eshop	?|e-shop
poukaz	110|Poukaz na registraci
REZER	10|Rezervace (ručně)
pspec	2|Papír speciál
Comment 10 Josef Moravec 2016-12-13 11:54:21 UTC
(In reply to Chris Kirby from comment #8)
> (In reply to comment #7), 
> Hi Josef, thanks for the tip to upgrade the db schema.
> 
> When one creates a "manual invoice" for a patron using one of the created
> debit types, both the debit type code and the description are entered into
> the "Description of charges." However, for the pre-existing debit types, the
> debit type description is entered twice in the "description of charges".
> Is there any reason for this difference?

It's a bug, I am going to work on this a bit more...


> When one creates a credit type, there is no field for default amount. Is
> there any reason for leaving a default amount out of the credit type but
> keeping it in the debit?

Yes, the reason is, the when you are creating manual invoice, you often know the price of some kind of things, but you can't pretend which amount are people going to pay, or so...
Comment 11 Josef Moravec 2016-12-13 11:55:16 UTC
(In reply to Radek Šiman (R-Bit Technology, s.r.o.) from comment #9)
> (In reply to Josef Moravec from comment #1)
> > Created attachment 57856 [details] [review] [review] [review]
> > Bug 17702: Account types configuration - db changes
> > 
> > Test plan:
> > 1) Read the diff and confirm that it does make sense
> > 2) Comment what does not make sense ;)
> 
> According to db changes, is it possible to extend type_code from only 5
> chars to (let's say) 10 chars? It would be very useful for some libraries
> having a longer list of payments not to be so cryptic. For instance, one of
> them (Czech only - sorry):

I have no problem with this.
Comment 12 Martin Renvoize 2019-02-28 13:58:01 UTC
Created attachment 85874 [details] [review]
Bug 17702: Account types configuration - db changes

Test plan:
1) Read the diff and confirm that it does make sense
2) Comment what does not make sense ;)
Comment 13 Martin Renvoize 2019-02-28 13:58:05 UTC
Created attachment 85875 [details] [review]
Bug 17702: Account types configuration - Manual invoice and credit
Comment 14 Martin Renvoize 2019-02-28 13:58:08 UTC
Created attachment 85876 [details] [review]
Bug 17702: Account types configuration - Admin page

Test plan:
1) Go to admin home, note there is new Account types page in Patrons and
circulation section
2) Go to any other admin page and confirm there is link to Account types
in admin menu as welll
3) Go to Account types page
4) You should see two datatables, one for debit types and one for
credit types, ensure the datatables are working corectly
5) Try to create, edit and delete some debit types, note, that some of
them can't be deleted - they are neede for Koha internally
6) Do the same with credit types
7) Try to add some default amount to internal debits - like Acccount
management fee, New card, ...
8) Go to Patron fines page and play with account type select, when you
change thy account type, the description and amount should change as you
set it on the admin page
9) Try to add some fees and ensure they are inserted corectly
10) Pay the inserted fees
11) Try to add manual credit and ensure it is added corectly
Comment 15 Martin Renvoize 2019-02-28 13:58:12 UTC
Created attachment 85877 [details] [review]
Bug 17702: Add tests

Test plan:
  prove t/db_dependent/Koha/AccountTypes.t
Comment 16 Michal Denar 2019-04-24 13:46:22 UTC
Hi Martin,
it's ready for testing? 

Thank You.
Comment 17 Michal Denar 2019-04-24 13:49:23 UTC
I tried test this patch but there're some problem with depencies.

Applying: Bug 22511: Update tests
Applying: Bug 22511: Update void method to use status
Applying: Bug 22511: Update UI to use accountlines.status
error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/includes/account-table.inc).
error: could not build fake ancestor
Patch failed at 0001 Bug 22511: Update UI to use accountlines.status
Comment 18 Martin Renvoize 2019-04-24 18:14:58 UTC
Sorry, this one isn't ready at all for testing yet I'm afraid. I've been working through the tree of bugs above it and may will split this one up a little further too once a few more of those foundations are closer to being in.

Don't worry, I've certainly not forgotten this one though, we have lots of customers very eager to see it too.
Comment 19 Michal Denar 2019-04-24 18:56:42 UTC
Hi Martin,
we want this feature too. So, I'm ready for testing. Just set status if follow-up will be ready.

Thank you.
Comment 20 Martin Renvoize 2019-04-25 10:15:32 UTC
Probably the best way to get this one moving again at this point is for someone to follow the dependencies down from bug 22200. I think I've added reasonable test plans to each of the bugs now so they should be reasonably clear.

I intend to get back onto this bug in earnest very shortly, but I have my doubts that it'll make it into 19.05 at this stage.
Comment 21 Martin Renvoize 2019-10-24 21:17:56 UTC
The combination of bug 23049 and bug 23805 resolves 90% of this enhancement report.  What is missing is the UI for account credits.

I chose not to implement the UI for account credit definitions as part of bug 23805 as at the time of writing all credit types are hard coded in Koha.

I do wonder if we still require the 'PAYMENT_TYPE' authorized value and corresponding accountlines field however. It feels like that should be merged into account_credit_types and then a UI exposed much like the account_debit_type management page introduced in but 23049.

As such, I'm leaving this bug open awaiting those next steps.
Comment 22 Julian Maurice 2020-02-07 13:09:26 UTC
Created attachment 98564 [details] [review]
Bug 17702: Add UI to manage account credit types

It is the same as for debit types.

It adds a new column `archived` in table `account_credit_types` that has
the same purpose than `account_debit_types.archived`

Test plan:
0. Apply patch && run updatedatabase && update_dbix_class_files
1. Go to Admin » Credit types
2. Add a new credit type, give it a code and a description and check
   'Can be manually added'
3. Go to a patron's accounting section, 'Create manual credit' tab
4. Verify that the new credit type appears
5. Return to Admin » Credit types and archive the credit type
6. Verify that the new credit type is not available anymore in 'Create
   manual credit'
7. Restore the credit type and verify that it is available again
8. Create a manual credit with the new credit type
9. Go to Reports » Cash register and make sure you can find the
   transaction by filtering on transaction type
10. Edit the new credit type and set some library limitations, make sure
    that the credit type doesn't appear if you're connected to a library
    you didn't selected, and that it appears if you're connected to a
    library you selected.

Note to QA team:
The change in Koha/Account.pm, I added it because otherwise Koha died
when adding a manual credit with a custom type.
In that case, offset type will default to 'Manual Credit'. I'm not sure
if that is the best thing to do. I'm open to suggestions :)
Comment 23 Julian Maurice 2020-02-07 13:13:47 UTC
Not sure if it should still be BLOCKED, or if it is the right bug for this patch. Martin, can you take a look please ?
Comment 24 Martin Renvoize 2020-02-10 09:59:08 UTC
Thanks for taking this on Julian, 

I'll just reverse the dependencies and we can unblock this one, it's great to see a new face taking an interest in this work.

I'll give it a test later today :)
Comment 25 Martin Renvoize 2020-02-10 14:36:19 UTC
Created attachment 98659 [details] [review]
Bug 17702: Add UI to manage account credit types

It is the same as for debit types.

It adds a new column `archived` in table `account_credit_types` that has
the same purpose than `account_debit_types.archived`

Test plan:
0. Apply patch && run updatedatabase && update_dbix_class_files
1. Go to Admin » Credit types
2. Add a new credit type, give it a code and a description and check
   'Can be manually added'
3. Go to a patron's accounting section, 'Create manual credit' tab
4. Verify that the new credit type appears
5. Return to Admin » Credit types and archive the credit type
6. Verify that the new credit type is not available anymore in 'Create
   manual credit'
7. Restore the credit type and verify that it is available again
8. Create a manual credit with the new credit type
9. Go to Reports » Cash register and make sure you can find the
   transaction by filtering on transaction type
10. Edit the new credit type and set some library limitations, make sure
    that the credit type doesn't appear if you're connected to a library
    you didn't selected, and that it appears if you're connected to a
    library you selected.

Note to QA team:
The change in Koha/Account.pm, I added it because otherwise Koha died
when adding a manual credit with a custom type.
In that case, offset type will default to 'Manual Credit'. I'm not sure
if that is the best thing to do. I'm open to suggestions :)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2020-02-10 14:36:23 UTC
Created attachment 98660 [details] [review]
Bug 17702: (QA follow-up) Add new field to kohastructure

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2020-02-10 14:37:22 UTC
Thanks for taking this on Julian, it's all looking good to me, signing off.
Comment 28 Kyle M Hall 2020-02-20 12:09:28 UTC
Created attachment 99306 [details] [review]
Bug 17702: Add UI to manage account credit types

It is the same as for debit types.

It adds a new column `archived` in table `account_credit_types` that has
the same purpose than `account_debit_types.archived`

Test plan:
0. Apply patch && run updatedatabase && update_dbix_class_files
1. Go to Admin » Credit types
2. Add a new credit type, give it a code and a description and check
   'Can be manually added'
3. Go to a patron's accounting section, 'Create manual credit' tab
4. Verify that the new credit type appears
5. Return to Admin » Credit types and archive the credit type
6. Verify that the new credit type is not available anymore in 'Create
   manual credit'
7. Restore the credit type and verify that it is available again
8. Create a manual credit with the new credit type
9. Go to Reports » Cash register and make sure you can find the
   transaction by filtering on transaction type
10. Edit the new credit type and set some library limitations, make sure
    that the credit type doesn't appear if you're connected to a library
    you didn't selected, and that it appears if you're connected to a
    library you selected.

Note to QA team:
The change in Koha/Account.pm, I added it because otherwise Koha died
when adding a manual credit with a custom type.
In that case, offset type will default to 'Manual Credit'. I'm not sure
if that is the best thing to do. I'm open to suggestions :)

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall 2020-02-20 12:09:44 UTC
Created attachment 99307 [details] [review]
Bug 17702: (QA follow-up) Add new field to kohastructure

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Martin Renvoize 2020-02-20 14:51:48 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 31 Martin Renvoize 2020-02-20 16:54:00 UTC
Created attachment 99322 [details] [review]
Bug 17702: (RM follow-up) Remove use of I18N tt

This patch removes the use of the I18N TT plugin in introduced for all
strings in the new template. We're not yet ready to use this plugin for
all strings as we need to further investigate performance for such a
move and perhaps add template compilation.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 32 Julian Maurice 2020-02-21 08:15:25 UTC
I did investigate performance at the time of bug 15395. See https://gitlab.com/jajm/time-i18n.
Comment 33 Lisette Scheer 2020-02-21 21:34:51 UTC Comment hidden (obsolete)
Comment 34 Lisette Scheer 2020-02-21 21:38:14 UTC
Apologies for the incorrect patch attachment.
Comment 35 Martin Renvoize 2020-02-22 08:50:57 UTC
(In reply to Martin Renvoize from comment #31)
> Created attachment 99322 [details] [review] [review]
> Bug 17702: (RM follow-up) Remove use of I18N tt
> 
> This patch removes the use of the I18N TT plugin in introduced for all
> strings in the new template. We're not yet ready to use this plugin for
> all strings as we need to further investigate performance for such a
> move and perhaps add template compilation.
> 
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

I somehow missed that performance comparison.. I'll have a look, thanks for bringing it up..

Jonathan is looking to work on a mass replacement of existing strings to move us over to this standard as a mass change and also update the QA scripts and xt test for filters to prevent the false failures we were seeing on this patch.

Thanks Julian..
Comment 36 Joy Nelson 2020-03-13 00:51:21 UTC
enhancement not backported to 19.11.x