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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm
index 5272b34..d4fbd6b 100644 (file)
--- a/Koha/Account/Line.pm
+++ b/Koha/Account/Line.pm
@@ -886,7 +886,13 @@ sub store {
     my ($self) = @_;
 
     my $AutoCreditNumber = C4::Context->preference('AutoCreditNumber');
-    if ($AutoCreditNumber && !$self->in_storage && $self->is_credit && !$self->credit_number) {
+    my $credit_number_enabled = $self->is_credit && $self->credit_type->credit_number_enabled;
+
+    if ($AutoCreditNumber && $credit_number_enabled && !$self->in_storage) {
+        if (defined $self->credit_number) {
+            Koha::Exceptions::Account->throw('AutoCreditNumber is enabled but credit_number is already defined');
+        }
+
         my $rs = Koha::Database->new->schema->resultset($self->_type);
 
         if ($AutoCreditNumber eq 'incremental') {

diff --git a/admin/credit_types.pl b/admin/credit_types.pl
index 75baff9..6b5da18 100755 (executable)
--- a/admin/credit_types.pl
+++ b/admin/credit_types.pl
@@ -77,17 +77,23 @@ if ( $op eq 'add_form' ) {
 elsif ( $op eq 'add_validate' ) {
     my $description           = $input->param('description');
     my $can_be_added_manually = $input->param('can_be_added_manually') || 0;
+    my $credit_number_enabled = $input->param('credit_number_enabled') || 0;
     my @branches = grep { $_ ne q{} } $input->multi_param('branches');
 
     if ( not defined $credit_type ) {
         $credit_type = Koha::Account::CreditType->new( { code => $code } );
     }
-    $credit_type->description($description);
-    $credit_type->can_be_added_manually($can_be_added_manually);
+    unless ($credit_type->is_system) {
+        $credit_type->description($description);
+        $credit_type->can_be_added_manually($can_be_added_manually);
+    }
+    $credit_type->credit_number_enabled($credit_number_enabled);
 
     try {
         $credit_type->store;
-        $credit_type->replace_library_limits( \@branches );
+        unless ($credit_type->is_system) {
+            $credit_type->replace_library_limits( \@branches );
+        }
         push @messages, { type => 'message', code => 'success_on_saving' };
     }
     catch {

diff --git a/installer/data/mysql/atomicupdate/bug-19036.perl b/installer/data/mysql/atomicupdate/bug-19036.perl
index 7a144bc..e92cf09 100644 (file)
--- a/installer/data/mysql/atomicupdate/bug-19036.perl
+++ b/installer/data/mysql/atomicupdate/bug-19036.perl
@@ -4,8 +4,17 @@ if (CheckVersion($DBversion)) {
         $dbh->do('ALTER TABLE accountlines ADD COLUMN credit_number VARCHAR(20) NULL DEFAULT NULL COMMENT "autogenerated number for credits" AFTER debit_type_code');
     }
 
+    unless (column_exists('account_credit_types', 'credit_number_enabled')) {
+        $dbh->do(q{
+            ALTER TABLE account_credit_types
+            ADD COLUMN credit_number_enabled TINYINT(1) NOT NULL DEFAULT 0
+                COMMENT "Is autogeneration of credit number enabled for this credit type"
+                AFTER can_be_added_manually
+        });
+    }
+
     $dbh->do('INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES(?, ?, ?, ?, ?)', undef, 'AutoCreditNumber', '', '', 'Automatically generate a number for account credits', 'Choice');
 
     SetVersion($DBversion);
-    print "Upgrade to $DBversion done (Bug 19036 - Add column accountlines.credit_number)\n";
+    print "Upgrade to $DBversion done (Bug 19036 - Add accountlines.credit_number, account_credit_types.credit_number_enabled and syspref AutoCreditNumber)\n";
 }

diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index 2e3a42a..27bb117 100644 (file)
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -2663,6 +2663,7 @@ CREATE TABLE `account_credit_types` (
   `code` varchar(80) NOT NULL,
   `description` varchar(200) DEFAULT NULL,
   `can_be_added_manually` tinyint(4) NOT NULL DEFAULT 1,
+  `credit_number_enabled` TINYINT(1) NOT NULL DEFAULT 0 COMMENT "Is autogeneration of credit number enabled for this credit type",
   `is_system` tinyint(1) NOT NULL DEFAULT 0,
   `archived` tinyint(1) NOT NULL DEFAULT 0, -- boolean flag to denote if this till is archived or not
   PRIMARY KEY (`code`)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt
index 11bcfab..0169ea2 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/credit_types.tt
@@ -75,29 +75,48 @@
                                 </li>
                                 <li>
                                     <label for="description" class="required">Description: </label>
-                                    <input type="text" name="description" id="description" required="required" class="required" size="80" maxlength="100" value="[% credit_type.description | html %]" /> <span class="required">Required</span>
+                                    [% IF credit_type && credit_type.is_system %]
+                                        <span>[% credit_type.description | html %]</span>
+                                    [% ELSE %]
+                                        <input type="text" name="description" id="description" required="required" class="required" size="80" maxlength="100" value="[% credit_type.description | html %]" /> <span class="required">Required</span>
+                                    [% END %]
                                 </li>
                                 <li>
                                     <label for="can_be_added_manually">Can be manually added ? </label>
-                                    [% IF credit_type.can_be_added_manually %]
+                                    [% IF credit_type && credit_type.is_system %]
+                                        [% IF credit_type.can_be_added_manually %]Yes[% ELSE %]No[% END %]
+                                    [% ELSIF credit_type.can_be_added_manually %]
                                         <input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" checked="checked" value="1" />
                                     [% ELSE %]
                                         <input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" value="1" />
                                     [% END %]
                                 </li>
                                 <li>
+                                    <label for="credit_number_enabled">Enable credit number</label>
+                                    [% IF credit_type.credit_number_enabled %]
+                                        <input type="checkbox" name="credit_number_enabled" id="credit_number_enabled" checked="checked" value="1" />
+                                    [% ELSE %]
+                                        <input type="checkbox" name="credit_number_enabled" id="credit_number_enabled" value="1" />
+                                    [% END %]
+                                    <span>Enable automatic generation of credit number (see <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=AutoCreditNumber">AutoCreditNumber</a>)</span>
+                                </li>
+                                <li>
                                     <label for="branches">Libraries limitation: </label>
-                                    <select id="branches" name="branches" multiple size="10">
-                                        <option value="">All libraries</option>
-                                        [% FOREACH branch IN branches_loop %]
-                                        [% IF ( branch.selected ) %]
-                                        <option selected="selected" value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                                        [% ELSE %]
-                                        <option value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                                        [% END %]
-                                        [% END %]
-                                    </select>
-                                    <span>Select 'All libraries' if this credit type should be available at all libraries. Otherwise select libraries you want to associate credit type with.</span>
+                                    [% IF credit_type && credit_type.is_system %]
+                                        No library limitation
+                                    [% ELSE %]
+                                        <select id="branches" name="branches" multiple size="10">
+                                            <option value="">All libraries</option>
+                                            [% FOREACH branch IN branches_loop %]
+                                            [% IF ( branch.selected ) %]
+                                            <option selected="selected" value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
+                                            [% ELSE %]
+                                            <option value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
+                                            [% END %]
+                                            [% END %]
+                                        </select>
+                                        <span>Select 'All libraries' if this credit type should be available at all libraries. Otherwise select libraries you want to associate credit type with.</span>
+                                    [% END %]
                                 </li>
                             </ol>
                         </fieldset>
@@ -123,6 +142,7 @@
                                 <th>Code</th>
                                 <th>Description</th>
                                 <th>Available for</th>
+                                <th>Credit number enabled</th>
                                 <th>Library limitations</th>
                                 <th>Actions</th>
                             </thead>
@@ -134,6 +154,7 @@
                                     <td>[% credit_type.code | html %]</td>
                                     <td>[% credit_type.description | html %]</td>
                                     <td>[% IF credit_type.can_be_added_manually %]Manual credit[% END %]</td>
+                                    <td>[% IF credit_type.credit_number_enabled %]Yes[% ELSE %]No[% END %]</td>
                                     <td>
                                         [% IF credit_type.library_limits.count > 0 %]
                                             [% library_limits_str = "" %]
@@ -152,11 +173,15 @@
                                         [% END %]
                                     </td>
                                     <td class="actions">
+                                        [% IF !credit_type.archived %]
+                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=add_form&amp;code=[% credit_type.code | uri %]&type=credit"><i class="fa fa-pencil"></i> Edit</a>
+                                        [% END %]
                                         [% IF !credit_type.is_system && !credit_type.archived %]
-                                        <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=add_form&amp;code=[% credit_type.code | uri %]&type=credit"><i class="fa fa-pencil"></i> Edit</a>
-                                        <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=archive&amp;code=[% credit_type.code | uri %]"><i class="fa fa-archive"></i> Archive</a>
-                                        [% ELSIF credit_type.archived %]
-                                        <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=unarchive&amp;code=[% credit_type.code | uri %]"><i class="fa fa-undo"></i> Restore</a>
+                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=archive&amp;code=[% credit_type.code | uri %]"><i class="fa fa-archive"></i> Archive</a>
+                                        [% END %]
+
+                                        [% IF !credit_type.is_system && credit_type.archived %]
+                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/credit_types.pl?op=unarchive&amp;code=[% credit_type.code | uri %]"><i class="fa fa-undo"></i> Restore</a>
                                         [% END %]
                                     </td>
                                 </tr>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref
index 5cbfe8d..7f4ae68 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref
@@ -40,3 +40,4 @@ Accounting:
                 annual: 'Automatically generate credit numbers in the form <year>-0001'
                 branchyyyymmincr: 'Automatically generate credit numbers in the form <branchcode>yyyymm0001'
                 incremental: 'Automatically generate credit numbers in the form 1, 2, 3'
+            - Automatic generation also has to be enabled for each credit type (<a href="/cgi-bin/koha/admin/credit_types.pl">Configure credit types</a>)

diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t
index 9ebad52..c6341c0 100755 (executable)
--- a/t/db_dependent/Koha/Account.t
+++ b/t/db_dependent/Koha/Account.t
@@ -26,6 +26,7 @@ use Test::Exception;
 use DateTime;
 
 use Koha::Account;
+use Koha::Account::CreditTypes;
 use Koha::Account::Lines;
 use Koha::Account::Offsets;
 use Koha::DateUtils qw( dt_from_string );
@@ -1051,7 +1052,7 @@ subtest 'Koha::Account::Line::apply() handles lost items' => sub {
 };
 
 subtest 'Koha::Account::pay() generates credit number' => sub {
-    plan tests => 34;
+    plan tests => 37;
 
     $schema->storage->txn_begin;
 
@@ -1069,6 +1070,10 @@ subtest 'Koha::Account::pay() generates credit number' => sub {
     my $month = $now->month;
     my ($accountlines_id, $accountline);
 
+    my $credit_type = Koha::Account::CreditTypes->find('PAYMENT');
+    $credit_type->credit_number_enabled(1);
+    $credit_type->store();
+
     t::lib::Mocks::mock_preference('AutoCreditNumber', '');
     $accountlines_id = $account->pay({ amount => '1.00', library_id => $library->id })->{payment_id};
     $accountline = Koha::Account::Lines->find($accountlines_id);
@@ -1080,6 +1085,9 @@ subtest 'Koha::Account::pay() generates credit number' => sub {
         $accountline = Koha::Account::Lines->find($accountlines_id);
         is($accountline->credit_number, $i);
     }
+    $accountlines_id = $account->pay({ type => 'WRITEOFF', amount => '1.00', library_id => $library->id })->{payment_id};
+    $accountline = Koha::Account::Lines->find($accountlines_id);
+    is($accountline->credit_number, undef);
 
     t::lib::Mocks::mock_preference('AutoCreditNumber', 'annual');
     for my $i (1..11) {
@@ -1087,6 +1095,9 @@ subtest 'Koha::Account::pay() generates credit number' => sub {
         $accountline = Koha::Account::Lines->find($accountlines_id);
         is($accountline->credit_number, sprintf('%s-%04d', $year, $i));
     }
+    $accountlines_id = $account->pay({ type => 'WRITEOFF', amount => '1.00', library_id => $library->id })->{payment_id};
+    $accountline = Koha::Account::Lines->find($accountlines_id);
+    is($accountline->credit_number, undef);
 
     t::lib::Mocks::mock_preference('AutoCreditNumber', 'branchyyyymmincr');
     for my $i (1..11) {
@@ -1094,6 +1105,9 @@ subtest 'Koha::Account::pay() generates credit number' => sub {
         $accountline = Koha::Account::Lines->find($accountlines_id);
         is($accountline->credit_number, sprintf('%s%d%02d%04d', $library->id, $year, $month, $i));
     }
+    $accountlines_id = $account->pay({ type => 'WRITEOFF', amount => '1.00', library_id => $library->id })->{payment_id};
+    $accountline = Koha::Account::Lines->find($accountlines_id);
+    is($accountline->credit_number, undef);
 
     $schema->storage->txn_rollback;
 };