Bug 19036

Summary: Number payment receipts / payment slips
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: Fines and feesAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: new feature    
Priority: P5 - low CC: andrewfh, black23, david, gmcharlt, indradg, jonathan.druart, josef.moravec, julian.maurice, kyle.m.hall, lucas, martin.renvoize, oleonard, sally.healey, sujan.lib
Version: unspecifiedKeywords: Manual
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10708
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This new feature lets you sequentially number number payment slips/receipts when using cash registers (it replaces the internal 'accountlines_id' used as the transaction identifier on the cash register summary page). This feature is disabled by default for all credit types. Enable it by: 1. Setting the new system preference AutoCreditNumber to one of the available options. 2. Enabling for each credit type. Available options are: - Do not automatically generate credit numbers - Automatically generate credit numbers in the form <year>-0001 - Automatically generate credit numbers in the form <branchcode>yyyymm0001 - Automatically generate credit numbers in the form 1, 2, 3'
Version(s) released in:
20.11.00
Bug Depends on: 14826    
Bug Blocks: 13985, 28318    
Attachments: Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to enable credit number for only some credit types
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to enable credit number for only some credit types
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to enable credit number for only some credit types
Bug 19036: (QA follow-up) Use Koha::DateUtils
Bug 19036: DBIC Schema Updates
Bug 19036: (QA follow-up) Fix test plan
Bug 19036: (QA follow-up) Fix 'out-by-one' error on table
Bug 19036: (QA follow-up) Add credit_number handling to POS
Bug 19036: (QA follow-up) Add credit_number handling to POS
Bug 19036: Add ability to auto generate a number for account credits
Bug 19036: Add ability to enable credit number for only some credit types
Bug 19036: (QA follow-up) Use Koha::DateUtils
Bug 19036: DBIC Schema Updates
Bug 19036: (QA follow-up) Fix test plan
Bug 19036: (QA follow-up) Fix 'out-by-one' error on table
Bug 19036: (QA follow-up) Add credit_number handling to POS
Bug 19036: Add missing test
Bug 19036: (follow-up) Improve test output
Bug 19036: (follow-up) Test credits should be credits

Description Katrin Fischer 2017-08-04 10:35:03 UTC
In some areas it's required to sequentially number payment slips / receipts. 

At the moment this is not easily possible in Koha, as we don't have a sequential internal number for the payments, but will hopefully be possible in some point in the future with the work on the Accounts module.
Comment 1 Katrin Fischer 2018-02-28 06:37:07 UTC
*** Bug 20309 has been marked as a duplicate of this bug. ***
Comment 2 Martin Renvoize 2019-07-18 16:15:04 UTC
Do these actually need to be sequential?

At this point we could use the accountlineid for the 'payment' type accountline that's associated with the slip.
Comment 3 Katrin Fischer 2019-10-02 13:20:38 UTC
As I understood this is a legal requirement in some countries, possibly relating to anti-corruption laws, where they have to keep a tight track of all the monetary transactions. I believe the number has to be sequential to avoid problems (like getting asked about missing receipts for missing numbers).
Comment 4 Katrin Fischer 2019-10-02 13:22:48 UTC
Ah, also accountlines_id is on the wrong level I think - we want to number the payments - a payment might include changes to multiple accountlines.
Comment 5 Julian Maurice 2020-02-20 08:48:41 UTC Comment hidden (obsolete)
Comment 6 Julian Maurice 2020-02-20 09:02:59 UTC Comment hidden (obsolete)
Comment 7 Julian Maurice 2020-02-20 09:06:19 UTC Comment hidden (obsolete)
Comment 8 Michal Denar 2020-02-25 20:16:09 UTC
Hi Julian,
I've tested bug. All works great, but I'm not able to add account credit number on print slip. It generate just empty space. But maybe it's my mistake. Can you double-check this part?

Thank you.
Comment 9 Julian Maurice 2020-02-26 06:52:57 UTC
(In reply to Michal Denar from comment #8)
> Hi Julian,
> I've tested bug. All works great, but I'm not able to add account credit
> number on print slip. It generate just empty space. But maybe it's my
> mistake. Can you double-check this part?
> 
> Thank you.

Hi Michal,

You need to edit the ACCOUNT_CREDIT letter ("Print" message type), search for a FOREACH loop like this:

  [% FOREACH account IN accounts %]

And add [% account.credit_number %] inside that loop
Comment 10 Michal Denar 2020-02-26 11:46:11 UTC
Hi Julian,
it's clear. I'll try to test it again soon.
Thank You.
Comment 11 Michal Denar 2020-02-26 19:50:58 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2020-02-28 16:09:38 UTC
Comment on attachment 99676 [details] [review]
Bug 19036: Add ability to auto generate a number for account credits

Review of attachment 99676 [details] [review]:
-----------------------------------------------------------------

Great work Julian, I really wasn't sure what the criteria were for this yet so I'm really pleased to see you've taken it on.

Just a few comments and questions as outlined:

::: Koha/Account/Line.pm
@@ +771,5 @@
> +sub store {
> +    my ($self) = @_;
> +
> +    my $AutoCreditNumber = C4::Context->preference('AutoCreditNumber');
> +    if ($AutoCreditNumber && !$self->in_storage && $self->is_credit && !$self->credit_number) {

Hmm, I slightly wonder if it's a good idea to allow passing credit_number and allowing it to override the AudoCreditNumber setting.. to me if it's set then we should throw an Exception if someone is trying to pass in their own?

Not a failing case, but wanted to raise as a question.

The second question here.. should this apply to ALL credit types, or just a subset of those that actually deal with taking money as apposed to writeoffs and other types of giving credit (like LOST_ITEM_RETURN)?

@@ +773,5 @@
> +
> +    my $AutoCreditNumber = C4::Context->preference('AutoCreditNumber');
> +    if ($AutoCreditNumber && !$self->in_storage && $self->is_credit && !$self->credit_number) {
> +        my $rs = Koha::Database->new->schema->resultset($self->_type);
> +

It feels like this whole block should be inside a transaction.. you're doing a lookup followed by a store using calculated data.. what if another db connection adds a payment line at the same time?
Comment 13 Martin Renvoize 2020-02-28 19:06:27 UTC
Note to self.. I'd like to add support for cash_register_id + timstamp as an option for numbering.. I'll post that as a followup once we've worked through my other comments ;)
Comment 14 Julian Maurice 2020-03-03 09:59:10 UTC
> Hmm, I slightly wonder if it's a good idea to allow passing
> credit_number and allowing it to override the AudoCreditNumber
> setting.. to me if it's set then we should throw an Exception if
> someone is trying to pass in their own?

Since credit_number can only be automatically generated, it does not make sense to check if it's defined or not, so I can remove this check and always autogenerate a value.
I can also throw an exception.
To me, both solutions are acceptable.

I'll let you pick the one you prefer ;)

> The second question here.. should this apply to ALL credit types, or
> just a subset of those that actually deal with taking money as apposed
> to writeoffs and other types of giving credit (like LOST_ITEM_RETURN)?

I don't know, but the answer is probably... it depends :)
Should I add a "Auto generate credit number" parameter in credit types ?
Comment 15 Martin Renvoize 2020-03-03 10:10:28 UTC
(In reply to Julian Maurice from comment #14)
> > Hmm, I slightly wonder if it's a good idea to allow passing
> > credit_number and allowing it to override the AudoCreditNumber
> > setting.. to me if it's set then we should throw an Exception if
> > someone is trying to pass in their own?
> 
> Since credit_number can only be automatically generated, it does not make
> sense to check if it's defined or not, so I can remove this check and always
> autogenerate a value.
> I can also throw an exception.
> To me, both solutions are acceptable.
> 
> I'll let you pick the one you prefer ;)

I like throwing exceptions as it highlights errors when future devs make mistakes.. but at the same time they have a tiny performance impact by calling additional checks so I'm happy without if you'd rather just stop checking for it and add a note in the POD of the store method to highlight we will be ignoring it if passed. Your call, I'll accept either :)

> 
> > The second question here.. should this apply to ALL credit types, or
> > just a subset of those that actually deal with taking money as apposed
> > to write-offs and other types of giving credit (like LOST_ITEM_RETURN)?
> 
> I don't know, but the answer is probably... it depends :)
> Should I add a "Auto generate credit number" parameter in credit types ?

That's not a bad idea at all, and is probably reasonably trivial to add if you're happy to do so, and will keep everbody happy :)
Comment 16 Martin Renvoize 2020-03-23 12:00:54 UTC
Failing QA until the above is responded to.
Comment 17 Julian Maurice 2020-03-23 12:22:27 UTC
I will add the "Auto generate credit number" parameter, and the exception (probably better to have an error rather than a silent overwrite).
Changing status to ASSIGNED
Comment 18 Julian Maurice 2020-03-23 15:11:01 UTC
(In reply to Julian Maurice from comment #17)
> I will add the "Auto generate credit number" parameter

In fact, it's not that simple. "system" credit types cannot be modified :/
Should I add the possibility to edit them but only allow this parameter to be modifiable ? Or should I find another way (another syspref for example) ?
Comment 19 Julian Maurice 2020-03-24 12:06:40 UTC
Created attachment 101549 [details] [review]
Bug 19036: Add ability to auto generate a number for account credits

In some areas it's required to sequentially number payment slips /
receipts.

This patch adds a database column accountlines.credit_number and a
system preference AutoCreditNumber to control how this number will be
generated.  The following options are available:

- Do not automatically generate credit numbers.
  This is the current behaviour and the default syspref value.

- Automatically generate credit numbers in the form <year>-0001 (annual)

- Automatically generate credit numbers in the form
  <branchcode>yyyymm0001 (branchyyyymmincr)
  where <branchcode> is the branch where the user (staff member) is
  logged in

- Automatically generate credit numbers in the form 1, 2, 3
  (incremental)

It also adds a column (hidden by default) in the table under
Transactions tab to display this number.

Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. Change syspref 'AutoCreditNumber' to 'incremental'
4. Create more manual credits, and verify that the numbers generated are
   1, 2, 3, ...
5. Change syspref 'AutoCreditNumber' to 'annual'
6. Create more manual credits, and verify that the numbers generated are
   '2020-0001', '2020-0002', ...
7. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
8. Create more manual credits, and verify that the numbers generated are
   'BRANCHA2020020001', 'BRANCHA2020020002', ... (assuming you are
   connected to library BRANCHA, and it's February 2020)
9. Set library to another one, say BRANCHB
10. Create more manual credits, and verify that the numbers generated are
    'BRANCHB2020020001', 'BRANCHB2020020002', ...
11. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
12. prove t/db_dependent/Koha/Account.t

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 20 Julian Maurice 2020-03-24 12:06:45 UTC
Created attachment 101550 [details] [review]
Bug 19036: Add ability to enable credit number for only some credit types

This feature is disabled by default for all credit types. To enable it
you have to set the syspref AutoCreditNumber to the value of your choice
and then enable the feature for each credit type you want.
System credit types can be modified too (but only this particular field
can be modified)

Also, throw an exception when the feature is enabled and a value for
credit_number has already been set

Updated test plan:
Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. In Admin » Credit types:
  a. edit the FORGIVEN type and enable credit number generation
  b. create a new type A, check "can be manually added" and "enable
     credit number"
  c. create a new type B, check "can be manually added". Do NOT enable
     credit number
4. Change syspref 'AutoCreditNumber' to 'incremental'
5. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
6. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are 1, 2, 3, ...
7. Change syspref 'AutoCreditNumber' to 'annual'
8. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
9. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are '2020-0001', '2020-0002', ...
10. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
11. Create more manual credits with types CREDIT and B, and verify that
    the numbers are not generated
12. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHA2020020001',
    'BRANCHA2020020002', ... (assuming you are connected to library
    BRANCHA, and it's February 2020)
13. Set library to another one, say BRANCHB
14. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHB2020020001',
    'BRANCHB2020020002', ...
15. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
16. prove t/db_dependent/Koha/Account.t
Comment 21 Martin Renvoize 2020-03-26 17:03:22 UTC
Created attachment 101896 [details] [review]
Bug 19036: Add ability to auto generate a number for account credits

In some areas it's required to sequentially number payment slips /
receipts.

This patch adds a database column accountlines.credit_number and a
system preference AutoCreditNumber to control how this number will be
generated.  The following options are available:

- Do not automatically generate credit numbers.
  This is the current behaviour and the default syspref value.

- Automatically generate credit numbers in the form <year>-0001 (annual)

- Automatically generate credit numbers in the form
  <branchcode>yyyymm0001 (branchyyyymmincr)
  where <branchcode> is the branch where the user (staff member) is
  logged in

- Automatically generate credit numbers in the form 1, 2, 3
  (incremental)

It also adds a column (hidden by default) in the table under
Transactions tab to display this number.

Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. Change syspref 'AutoCreditNumber' to 'incremental'
4. Create more manual credits, and verify that the numbers generated are
   1, 2, 3, ...
5. Change syspref 'AutoCreditNumber' to 'annual'
6. Create more manual credits, and verify that the numbers generated are
   '2020-0001', '2020-0002', ...
7. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
8. Create more manual credits, and verify that the numbers generated are
   'BRANCHA2020020001', 'BRANCHA2020020002', ... (assuming you are
   connected to library BRANCHA, and it's February 2020)
9. Set library to another one, say BRANCHB
10. Create more manual credits, and verify that the numbers generated are
    'BRANCHB2020020001', 'BRANCHB2020020002', ...
11. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
12. prove t/db_dependent/Koha/Account.t

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2020-03-26 17:03:26 UTC
Created attachment 101897 [details] [review]
Bug 19036: Add ability to enable credit number for only some credit types

This feature is disabled by default for all credit types. To enable it
you have to set the syspref AutoCreditNumber to the value of your choice
and then enable the feature for each credit type you want.
System credit types can be modified too (but only this particular field
can be modified)

Also, throw an exception when the feature is enabled and a value for
credit_number has already been set

Updated test plan:
Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. In Admin » Credit types:
  a. edit the FORGIVEN type and enable credit number generation
  b. create a new type A, check "can be manually added" and "enable
     credit number"
  c. create a new type B, check "can be manually added". Do NOT enable
     credit number
4. Change syspref 'AutoCreditNumber' to 'incremental'
5. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
6. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are 1, 2, 3, ...
7. Change syspref 'AutoCreditNumber' to 'annual'
8. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
9. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are '2020-0001', '2020-0002', ...
10. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
11. Create more manual credits with types CREDIT and B, and verify that
    the numbers are not generated
12. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHA2020020001',
    'BRANCHA2020020002', ... (assuming you are connected to library
    BRANCHA, and it's February 2020)
13. Set library to another one, say BRANCHB
14. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHB2020020001',
    'BRANCHB2020020002', ...
15. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
16. prove t/db_dependent/Koha/Account.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2020-03-26 17:03:47 UTC
This works really nicely, thanks Julian!

Signing off
Comment 24 Katrin Fischer 2020-06-22 23:15:37 UTC
There are some failures in QA tools that need to be taken care of. I will try to come back to this for more testing after they are fixed:

 FAIL	t/db_dependent/Koha/Account.t
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 1068)

 FAIL	installer/data/mysql/kohastructure.sql
   FAIL	  tinyint_has_boolean_flag
		The new column account_credit_types.credit_number_enabled is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/AccountCreditType.pm), see the SQL12 coding guideline

 FAIL	Koha/Account/Line.pm
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 895)
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 910)
Comment 25 Martin Renvoize 2020-06-25 09:44:56 UTC
Created attachment 106278 [details] [review]
Bug 19036: Add ability to auto generate a number for account credits

In some areas it's required to sequentially number payment slips /
receipts.

This patch adds a database column accountlines.credit_number and a
system preference AutoCreditNumber to control how this number will be
generated.  The following options are available:

- Do not automatically generate credit numbers.
  This is the current behaviour and the default syspref value.

- Automatically generate credit numbers in the form <year>-0001 (annual)

- Automatically generate credit numbers in the form
  <branchcode>yyyymm0001 (branchyyyymmincr)
  where <branchcode> is the branch where the user (staff member) is
  logged in

- Automatically generate credit numbers in the form 1, 2, 3
  (incremental)

It also adds a column (hidden by default) in the table under
Transactions tab to display this number.

Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. Change syspref 'AutoCreditNumber' to 'incremental'
4. Create more manual credits, and verify that the numbers generated are
   1, 2, 3, ...
5. Change syspref 'AutoCreditNumber' to 'annual'
6. Create more manual credits, and verify that the numbers generated are
   '2020-0001', '2020-0002', ...
7. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
8. Create more manual credits, and verify that the numbers generated are
   'BRANCHA2020020001', 'BRANCHA2020020002', ... (assuming you are
   connected to library BRANCHA, and it's February 2020)
9. Set library to another one, say BRANCHB
10. Create more manual credits, and verify that the numbers generated are
    'BRANCHB2020020001', 'BRANCHB2020020002', ...
11. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
12. prove t/db_dependent/Koha/Account.t

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2020-06-25 09:45:00 UTC
Created attachment 106279 [details] [review]
Bug 19036: Add ability to enable credit number for only some credit types

This feature is disabled by default for all credit types. To enable it
you have to set the syspref AutoCreditNumber to the value of your choice
and then enable the feature for each credit type you want.
System credit types can be modified too (but only this particular field
can be modified)

Also, throw an exception when the feature is enabled and a value for
credit_number has already been set

Updated test plan:
Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. In Admin » Credit types:
  a. edit the FORGIVEN type and enable credit number generation
  b. create a new type A, check "can be manually added" and "enable
     credit number"
  c. create a new type B, check "can be manually added". Do NOT enable
     credit number
4. Change syspref 'AutoCreditNumber' to 'incremental'
5. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
6. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are 1, 2, 3, ...
7. Change syspref 'AutoCreditNumber' to 'annual'
8. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
9. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are '2020-0001', '2020-0002', ...
10. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
11. Create more manual credits with types CREDIT and B, and verify that
    the numbers are not generated
12. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHA2020020001',
    'BRANCHA2020020002', ... (assuming you are connected to library
    BRANCHA, and it's February 2020)
13. Set library to another one, say BRANCHB
14. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHB2020020001',
    'BRANCHB2020020002', ...
15. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
16. prove t/db_dependent/Koha/Account.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2020-06-25 09:45:03 UTC
Created attachment 106280 [details] [review]
Bug 19036: (QA follow-up) Use Koha::DateUtils

This patch replaced raw DateTime->now calls with dt_from_string from
Koha::DateUtils
Comment 28 Martin Renvoize 2020-06-25 09:45:07 UTC
Created attachment 106281 [details] [review]
Bug 19036: DBIC Schema Updates

Dumped schema updates + manual addition of boolean flags
Comment 29 Martin Renvoize 2020-06-25 09:45:46 UTC
Patchset rescued.. I've added the required QA followups and rebased on master :)
Comment 30 Katrin Fischer 2020-07-04 23:48:50 UTC
The test file this also changes is currently failing for me on master:

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Koha/Account.t
t/db_dependent/Koha/Account.t .. 1/12 Use of uninitialized value in numeric lt (<) at /home/vagrant/kohaclone/Koha/Account/Line.pm line 745.
t/db_dependent/Koha/Account.t .. 3/12 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`accountlines`, CONSTRAINT `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`, `branchcode`, `date`, `debit_type_code`, `description`, `interface`, `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`) VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )" with ParamValues: 0=5, 1=5, 2=199, 3='dgj1I5', 4='failure', 5='type validation failure', 6='commandline', 7=undef, 8=undef, 9=199, 10='this should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm line 166
t/db_dependent/Koha/Account.t .. 13/12 # Looks like you planned 12 tests but ran 13.
t/db_dependent/Koha/Account.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
All 12 subtests passed 

Test Summary Report
-------------------
t/db_dependent/Koha/Account.t (Wstat: 65280 Tests: 13 Failed: 1)
  Failed test:  13
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 12 tests but ran 13.
Files=1, Tests=13,  8 wallclock secs ( 0.05 usr  0.02 sys +  6.85 cusr  0.92 csys =  7.84 CPU)
Result: FAIL

Moving this to BLOCKED until this is resolved to then continue here.
Comment 31 Martin Renvoize 2020-07-10 10:33:07 UTC
Created attachment 106767 [details] [review]
Bug 19036: (QA follow-up) Fix test plan
Comment 32 Martin Renvoize 2020-07-10 10:35:53 UTC
(In reply to Katrin Fischer from comment #30)
> The test file this also changes is currently failing for me on master:
> 
> kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove
> t/db_dependent/Koha/Account.t
> t/db_dependent/Koha/Account.t .. 1/12 Use of uninitialized value in numeric
> lt (<) at /home/vagrant/kohaclone/Koha/Account/Line.pm line 745.
> t/db_dependent/Koha/Account.t .. 3/12
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Cannot add or update a child row: a foreign key constraint
> fails (`koha_kohadev`.`accountlines`, CONSTRAINT
> `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES
> `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT
> INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`,
> `branchcode`, `date`, `debit_type_code`, `description`, `interface`,
> `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`)
> VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )"
> with ParamValues: 0=5, 1=5, 2=199, 3='dgj1I5', 4='failure', 5='type
> validation failure', 6='commandline', 7=undef, 8=undef, 9=199, 10='this
> should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm
> line 166
> t/db_dependent/Koha/Account.t .. 13/12 # Looks like you planned 12 tests but
> ran 13.
> t/db_dependent/Koha/Account.t .. Dubious, test returned 255 (wstat 65280,
> 0xff00)
> All 12 subtests passed 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Koha/Account.t (Wstat: 65280 Tests: 13 Failed: 1)
>   Failed test:  13
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 12 tests but ran 13.
> Files=1, Tests=13,  8 wallclock secs ( 0.05 usr  0.02 sys +  6.85 cusr  0.92
> csys =  7.84 CPU)
> Result: FAIL
> 
> Moving this to BLOCKED until this is resolved to then continue here.

I think you missed the DB update... The test passes both on master and with this bug for me (though the test plan was out by one, which I've corrected with a follow-up).
Comment 33 Katrin Fischer 2020-07-30 22:41:31 UTC
Hm, it wasn't the missed db update, because as I wrote... I have the same output of master - but it claims all tests successful. Are we provoking the db error intentionally?

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Koha/Account.t
t/db_dependent/Koha/Account.t .. 3/12 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`accountlines`, CONSTRAINT `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`, `branchcode`, `date`, `debit_type_code`, `description`, `interface`, `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`) VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )" with ParamValues: 0=5, 1=5, 2=93, 3='Ro3LQ1AIV', 4='failure', 5='type validation failure', 6='commandline', 7=undef, 8=undef, 9=93, 10='this should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm line 166
t/db_dependent/Koha/Account.t .. ok     
All tests successful.
Comment 34 Katrin Fischer 2020-07-30 23:05:06 UTC
1) Transactions table

http://localhost:8081/cgi-bin/koha/members/boraccount.pl?borrowernumber=51
Tab: Transactions

The last table row with the total is off by one (display issue).

2) Terminology: credit number versus receipt number versus ?

I wonder if "credit number" is the right term. 
I think for debits we'd probably use invoice number... and for debits I am thinking of receipt number. 

What do the native speakers say?


Everything else works as expected as far as I can tell - I like!

1) should be an easy fix, 2 is a bit more difficult. CC'ing some other librarians in hope for help :)
Comment 35 David Nind 2020-07-31 06:05:33 UTC
My thoughts on terminology, not having worked through the test plan (from an accounting perspective):

- A debit transaction: invoice number for charges and purchases
- A credit transaction: receipt number for payments, credit note for return of goods

For the purposes of having an auto incremented number in a sequence for auditing, then perhaps calling it a transaction number may be better.

This could be used for different debit and credit types. 

On statements or transaction records, having some way to use the preferred term depending on your country, accounting rules or preferences for the transaction type may be useful.
Comment 36 Martin Renvoize 2020-08-06 10:24:44 UTC
(In reply to Katrin Fischer from comment #33)
> Hm, it wasn't the missed db update, because as I wrote... I have the same
> output of master - but it claims all tests successful. Are we provoking the
> db error intentionally?
> 
> kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ git checkout master
> Switched to branch 'master'
> Your branch is up-to-date with 'origin/master'.
> kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove
> t/db_dependent/Koha/Account.t
> t/db_dependent/Koha/Account.t .. 3/12
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Cannot add or update a child row: a foreign key constraint
> fails (`koha_kohadev`.`accountlines`, CONSTRAINT
> `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES
> `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT
> INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`,
> `branchcode`, `date`, `debit_type_code`, `description`, `interface`,
> `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`)
> VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )"
> with ParamValues: 0=5, 1=5, 2=93, 3='Ro3LQ1AIV', 4='failure', 5='type
> validation failure', 6='commandline', 7=undef, 8=undef, 9=93, 10='this
> should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm
> line 166
> t/db_dependent/Koha/Account.t .. ok     
> All tests successful.

That is NOT a failure.. it's an expected case.. we are testing for the DB constraint.. there's isn't an easy way to hide this noise and it's already there on lots of tests.
Comment 37 Martin Renvoize 2020-08-06 10:35:25 UTC
Created attachment 107872 [details] [review]
Bug 19036: (QA follow-up) Fix 'out-by-one' error on table

This patch fixes the total row where the column count was out by one.
Comment 38 Martin Renvoize 2020-08-06 10:49:10 UTC
Fixed the table issue in the attached followup.

As for terminology.. this identifier is only added to 'credits' to the 'debits' terminology discussion it mute at this time.  As for the terminology of 'credit number'.. I think it's fine at the DB level as is.. I'm less sure about how it displays in the UI.  Currently, we're only allowing one 'style' of incrementing number for a set of credit types.. I could imagine there being two distinct sets of running numbers.. one for 'payment reciepts' and a second for 'credit notes', but we don't allow for that here. 

As such, I'd be tempted to go much more generic and just call the column 'Identifier' or 'Unique identifier' or something like that in the UI?
Comment 39 Martin Renvoize 2020-08-06 10:51:12 UTC
Should we be hiding the 'credit number' column by default in fact... we don't enable the feature by default so why expose the empty field in the table?
Comment 40 Martin Renvoize 2020-08-06 10:55:59 UTC
I also think the credit number should be added to the 'register details' page that lists transactions too.
Comment 41 Jonathan Druart 2020-08-06 11:05:01 UTC
(In reply to Martin Renvoize from comment #36)
> (In reply to Katrin Fischer from comment #33)
> > Hm, it wasn't the missed db update, because as I wrote... I have the same
> > output of master - but it claims all tests successful. Are we provoking the
> > db error intentionally?
> > 
> > kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ git checkout master
> > Switched to branch 'master'
> > Your branch is up-to-date with 'origin/master'.
> > kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove
> > t/db_dependent/Koha/Account.t
> > t/db_dependent/Koha/Account.t .. 3/12
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Cannot add or update a child row: a foreign key constraint
> > fails (`koha_kohadev`.`accountlines`, CONSTRAINT
> > `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES
> > `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT
> > INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`,
> > `branchcode`, `date`, `debit_type_code`, `description`, `interface`,
> > `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`)
> > VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )"
> > with ParamValues: 0=5, 1=5, 2=93, 3='Ro3LQ1AIV', 4='failure', 5='type
> > validation failure', 6='commandline', 7=undef, 8=undef, 9=93, 10='this
> > should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm
> > line 166
> > t/db_dependent/Koha/Account.t .. ok     
> > All tests successful.
> 
> That is NOT a failure.. it's an expected case.. we are testing for the DB
> constraint.. there's isn't an easy way to hide this noise and it's already
> there on lots of tests.

I will deal with that on bug 26157.
Comment 42 Martin Renvoize 2020-08-06 11:31:02 UTC
Created attachment 107875 [details] [review]
Bug 19036: (QA follow-up) Add credit_number handling to POS

This patch adds the display of 'credit_number' as a replacement for the
internal 'accountlines_id' as the transaction identifier on the
register summary page when using cash registers.
Comment 43 Martin Renvoize 2020-08-06 11:32:33 UTC
(In reply to Jonathan Druart from comment #41)
> (In reply to Martin Renvoize from comment #36)
> > (In reply to Katrin Fischer from comment #33)
> > > Hm, it wasn't the missed db update, because as I wrote... I have the same
> > > output of master - but it claims all tests successful. Are we provoking the
> > > db error intentionally?
> > > 
> > > kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ git checkout master
> > > Switched to branch 'master'
> > > Your branch is up-to-date with 'origin/master'.
> > > kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove
> > > t/db_dependent/Koha/Account.t
> > > t/db_dependent/Koha/Account.t .. 3/12
> > > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > > execute failed: Cannot add or update a child row: a foreign key constraint
> > > fails (`koha_kohadev`.`accountlines`, CONSTRAINT
> > > `accountlines_ibfk_debit_type` FOREIGN KEY (`debit_type_code`) REFERENCES
> > > `account_debit_types` (`code`) ON UPDATE CASCADE) [for Statement "INSERT
> > > INTO `accountlines` ( `amount`, `amountoutstanding`, `borrowernumber`,
> > > `branchcode`, `date`, `debit_type_code`, `description`, `interface`,
> > > `issue_id`, `itemnumber`, `manager_id`, `note`, `payment_type`, `timestamp`)
> > > VALUES ( ?, ?, ?, ?, NOW(), ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp )"
> > > with ParamValues: 0=5, 1=5, 2=93, 3='Ro3LQ1AIV', 4='failure', 5='type
> > > validation failure', 6='commandline', 7=undef, 8=undef, 9=93, 10='this
> > > should fail anyway', 11=undef] at /home/vagrant/kohaclone/Koha/Object.pm
> > > line 166
> > > t/db_dependent/Koha/Account.t .. ok     
> > > All tests successful.
> > 
> > That is NOT a failure.. it's an expected case.. we are testing for the DB
> > constraint.. there's isn't an easy way to hide this noise and it's already
> > there on lots of tests.
> 
> I will deal with that on bug 26157.

Awesome!  Thanks Jonathan
Comment 44 Martin Renvoize 2020-08-06 11:53:57 UTC
Created attachment 107877 [details] [review]
Bug 19036: (QA follow-up) Add credit_number handling to POS

This patch adds the display of 'credit_number' as a replacement for the
internal 'accountlines_id' as the transaction identifier on the
register summary page when using cash registers.

Test plan
1/ Enable 'UseCashRegisters'
2/ Enable 'EnablePointOfSale'
3/ Enable at least one 'Debit type' to allow 'sale'
4/ Add at least one 'Cash register'
5/ Enable 'AutoCreditNumber'
6/ 'Enable automatic generation of credit number' for the 'Purchase'
credit type
7/ Pay for an item via the Point of Sale page
8/ Navigate to the 'register' page to display the purchase details
9/ Confirm that the 'Transaction' is listed with a number of the form
specified in the 'AutoCreditNumber' preference
10/ Refund an item from this screen
11/ Note that the 'accountlines_id' is used for this refund as we did
not specify the 'Refund' credit type to use the 'AutoCreditNumber'
system
12/ Signoff
Comment 45 Katrin Fischer 2020-08-15 12:44:23 UTC
Created attachment 108323 [details] [review]
Bug 19036: Add ability to auto generate a number for account credits

In some areas it's required to sequentially number payment slips /
receipts.

This patch adds a database column accountlines.credit_number and a
system preference AutoCreditNumber to control how this number will be
generated.  The following options are available:

- Do not automatically generate credit numbers.
  This is the current behaviour and the default syspref value.

- Automatically generate credit numbers in the form <year>-0001 (annual)

- Automatically generate credit numbers in the form
  <branchcode>yyyymm0001 (branchyyyymmincr)
  where <branchcode> is the branch where the user (staff member) is
  logged in

- Automatically generate credit numbers in the form 1, 2, 3
  (incremental)

It also adds a column (hidden by default) in the table under
Transactions tab to display this number.

Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. Change syspref 'AutoCreditNumber' to 'incremental'
4. Create more manual credits, and verify that the numbers generated are
   1, 2, 3, ...
5. Change syspref 'AutoCreditNumber' to 'annual'
6. Create more manual credits, and verify that the numbers generated are
   '2020-0001', '2020-0002', ...
7. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
8. Create more manual credits, and verify that the numbers generated are
   'BRANCHA2020020001', 'BRANCHA2020020002', ... (assuming you are
   connected to library BRANCHA, and it's February 2020)
9. Set library to another one, say BRANCHB
10. Create more manual credits, and verify that the numbers generated are
    'BRANCHB2020020001', 'BRANCHB2020020002', ...
11. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
12. prove t/db_dependent/Koha/Account.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 46 Katrin Fischer 2020-08-15 12:44:32 UTC
Created attachment 108324 [details] [review]
Bug 19036: Add ability to enable credit number for only some credit types

This feature is disabled by default for all credit types. To enable it
you have to set the syspref AutoCreditNumber to the value of your choice
and then enable the feature for each credit type you want.
System credit types can be modified too (but only this particular field
can be modified)

Also, throw an exception when the feature is enabled and a value for
credit_number has already been set

Updated test plan:
Test plan:
0. Apply patch, run updatedatabase and update_dbix_class_files
1. Go to Admin » Column settings, and uncheck the 'hidden' box for
   column credit_number in table account-fines. It will be easier for
   testing
2. Create a manual credit for a borrower. Verify in Transactions tab
   that this credit has no number generated
3. In Admin » Credit types:
  a. edit the FORGIVEN type and enable credit number generation
  b. create a new type A, check "can be manually added" and "enable
     credit number"
  c. create a new type B, check "can be manually added". Do NOT enable
     credit number
4. Change syspref 'AutoCreditNumber' to 'incremental'
5. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
6. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are 1, 2, 3, ...
7. Change syspref 'AutoCreditNumber' to 'annual'
8. Create more manual credits with types CREDIT and B, and verify that
   the numbers are not generated
9. Create more manual credits with types FORGIVEN and A, and verify that
   the numbers generated are '2020-0001', '2020-0002', ...
10. Change syspref to 'AutoCreditNumber' to 'branchyyyymmincr'
11. Create more manual credits with types CREDIT and B, and verify that
    the numbers are not generated
12. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHA2020020001',
    'BRANCHA2020020002', ... (assuming you are connected to library
    BRANCHA, and it's February 2020)
13. Set library to another one, say BRANCHB
14. Create more manual credits with types FORGIVEN and A, and verify
    that the numbers generated are 'BRANCHB2020020001',
    'BRANCHB2020020002', ...
15. Edit the letter ACCOUNT_CREDIT, and add [% account.credit_number %]
    somewhere. Go back to Transactions tab and click on 'Print' for one
    line that has a credit number. Make sure the number is there.
16. prove t/db_dependent/Koha/Account.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 47 Katrin Fischer 2020-08-15 12:44:38 UTC
Created attachment 108325 [details] [review]
Bug 19036: (QA follow-up) Use Koha::DateUtils

This patch replaced raw DateTime->now calls with dt_from_string from
Koha::DateUtils

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 48 Katrin Fischer 2020-08-15 12:44:44 UTC
Created attachment 108326 [details] [review]
Bug 19036: DBIC Schema Updates

Dumped schema updates + manual addition of boolean flags

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 49 Katrin Fischer 2020-08-15 12:44:51 UTC
Created attachment 108327 [details] [review]
Bug 19036: (QA follow-up) Fix test plan

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 50 Katrin Fischer 2020-08-15 12:44:59 UTC
Created attachment 108328 [details] [review]
Bug 19036: (QA follow-up) Fix 'out-by-one' error on table

This patch fixes the total row where the column count was out by one.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 51 Katrin Fischer 2020-08-15 12:45:06 UTC
Created attachment 108329 [details] [review]
Bug 19036: (QA follow-up) Add credit_number handling to POS

This patch adds the display of 'credit_number' as a replacement for the
internal 'accountlines_id' as the transaction identifier on the
register summary page when using cash registers.

Test plan
1/ Enable 'UseCashRegisters'
2/ Enable 'EnablePointOfSale'
3/ Enable at least one 'Debit type' to allow 'sale'
4/ Add at least one 'Cash register'
5/ Enable 'AutoCreditNumber'
6/ 'Enable automatic generation of credit number' for the 'Purchase'
credit type
7/ Pay for an item via the Point of Sale page
8/ Navigate to the 'register' page to display the purchase details
9/ Confirm that the 'Transaction' is listed with a number of the form
specified in the 'AutoCreditNumber' preference
10/ Refund an item from this screen
11/ Note that the 'accountlines_id' is used for this refund as we did
not specify the 'Refund' credit type to use the 'AutoCreditNumber'
system
12/ Signoff

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 52 Katrin Fischer 2020-08-15 12:47:16 UTC
I a little torn still about the terminology, but not enough to block this. I have retested the table layout and the added POS functionality.
Comment 53 Jonathan Druart 2020-08-18 15:38:11 UTC
Created attachment 108525 [details] [review]
Bug 19036: Add missing test
Comment 54 Jonathan Druart 2020-08-18 15:38:41 UTC
I've added a missing test but it does not pass. Could you have a look?
Comment 55 Martin Renvoize 2020-08-19 11:20:19 UTC
Created attachment 108587 [details] [review]
Bug 19036: (follow-up) Improve test output

This patch adds proper detail to the test output for the newly
introduced AutoCreditNumber feature and fixes the final test by adding
the 'interface' and 'amount' parameters to the Koha::Account::Line being
created so that we reach the Exception we are trying to catch and don't
die early.
Comment 56 Martin Renvoize 2020-08-19 11:21:48 UTC
Fixed the test.. there were two minor issues with the construction meaning that we would not reach the exception (missing interface and amount params in the call to new).

I've also added some more verbose output to the prior tests to clarify their intent at runtime.
Comment 57 Jonathan Druart 2020-08-20 10:34:23 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 58 Martin Renvoize 2020-08-20 11:54:56 UTC
Created attachment 108733 [details] [review]
Bug 19036: (follow-up) Test credits should be credits

A series of accountlines added to mock credits were being added with a
debit_type_code instead of a valid credit_type_code and so were being
mis-identified by the new credit numbering code trigger in the store
routine.

This patch updates the test to correctly identify the credits as credits

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2020-08-20 11:55:35 UTC
The attached followup should fix the related failing test on master.
Comment 60 Jonathan Druart 2020-08-21 10:34:26 UTC
Follow-up pushed to master!
Comment 61 Lucas Gass 2020-09-04 17:16:06 UTC
enhancement will not be backported to 20.05.x
Comment 62 Lucas Gass 2020-09-04 17:20:18 UTC
enhancement will not be backported to 20.05.x