Attachment 108324 Details for Bug 19036 – Bug 19036: Add ability to enable credit number for only some credit types

[patch] Bug 19036: Add ability to enable credit number for only some credit types

Bug-19036-Add-ability-to-enable-credit-number-for-.patch (text/plain), 19.47 KB, created by Katrin Fischer on 2020-08-15 12:44:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2020-08-15 12:44:32 UTC

Size: 19.47 KB

patch

obsolete

>From 08cdc8302a4fa034021ad2de37acf94691248ff6 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Tue, 24 Mar 2020 10:14:03 +0100
>Subject: [PATCH] Bug 19036: Add ability to enable credit number for only some
> credit types
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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>
>---
> Koha/Account/Line.pm                               |  8 ++-
> admin/credit_types.pl                              | 12 +++--
> installer/data/mysql/atomicupdate/bug-19036.perl   | 11 +++-
> installer/data/mysql/kohastructure.sql             |  1 +
> .../prog/en/modules/admin/credit_types.tt          | 59 +++++++++++++++-------
> .../en/modules/admin/preferences/accounting.pref   |  1 +
> t/db_dependent/Koha/Account.t                      | 16 +++++-
> 7 files changed, 85 insertions(+), 23 deletions(-)
>
>diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm
>index 5272b34912..d4fbd6bd71 100644
>--- 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 75baff90bb..6b5da18071 100755
>--- 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 7a144bcd94..e92cf096b3 100644
>--- 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 2e3a42ac1c..27bb117b31 100644
>--- 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 11bcfab409..0169ea25b4 100644
>--- 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 5cbfe8d3fd..7f4ae68fd9 100644
>--- 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 9ebad52055..c6341c0cb2 100755
>--- 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;
> };
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 19036: 99301 | 99302 | 99303 | 99676 | 101549 | 101550 | 101896 | 101897 | 106278 | 106279 | 106280 | 106281 | 106767 | 107872 | 107875 | 107877 | 108323 | 108324 | 108325 | 108326 | 108327 | 108328 | 108329 | 108525 | 108587 | 108733