Attachment 93025 Details for Bug 23321 – Bug 23321: Allow setting of branch default

[patch] Bug 23321: Allow setting of branch default

Bug-23321-Allow-setting-of-branch-default.patch (text/plain), 15.93 KB, created by Tomás Cohen Arazi (tcohen) on 2019-09-20 19:01:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2019-09-20 19:01:58 UTC

Size: 15.93 KB

patch

obsolete

>From 908aa7a7cfd7da1ef7a1c22ae3cb23f04375f673 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Tue, 17 Sep 2019 12:24:18 +0100
>Subject: [PATCH] Bug 23321: Allow setting of branch default
>
>This adds the ability to set a cash register as the default selected
>option when making payments at a particular branch.
>
>1) Note the addition of a 'Make branch default' button in each row of
>   the table under 'Manage cash registers'
>2) Click the button and note that the button has changed.
>3) Click on an alternative cash register for the same branch and note
>   that the default has been switched to the new register.
>4) Click the 'unset' button on the default branch and note that there is
>   no longer a default register for this branch.
>5) Signoff
>
>Sponsored-by: PTFS Europe
>Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Cash/Register.pm                         | 69 ++++++++++++++++-
> Koha/Exceptions/Object.pm                     |  8 ++
> admin/cash_registers.pl                       | 31 +++++++-
> .../intranet-tmpl/prog/css/src/_tables.scss   |  8 +-
> .../prog/en/modules/admin/cash_registers.tt   | 74 +++++++++++++++----
> t/db_dependent/Koha/Cash/Register.t           | 61 ++++++++++++++-
> 6 files changed, 227 insertions(+), 24 deletions(-)
>
>diff --git a/Koha/Cash/Register.pm b/Koha/Cash/Register.pm
>index 15f8a49e23..b3fe0f5413 100644
>--- a/Koha/Cash/Register.pm
>+++ b/Koha/Cash/Register.pm
>@@ -42,10 +42,75 @@ Return the library linked to this cash register
> =cut
> 
> sub library {
>-    my ( $self ) = @_;
>+    my ($self) = @_;
>     my $rs = $self->_result->branch;
>     return unless $rs;
>-    return Koha::Library->_new_from_dbic( $rs );
>+    return Koha::Library->_new_from_dbic($rs);
>+}
>+
>+=head3 store
>+
>+Local store method to prevent direct manipulation of the 'branch_default' field
>+
>+=cut
>+
>+sub store {
>+    my ($self) = @_;
>+    $self->_result->result_source->schema->txn_do(
>+        sub {
>+            if ( $self->_result->is_column_changed('branch_default') ) {
>+                Koha::Exceptions::Object::ReadOnlyProperty->throw(
>+                    property => 'branch_default' );
>+            }
>+            else {
>+                if ($self->_result->is_column_changed('branch') && $self->branch_default) {
>+                }
>+                $self = $self->SUPER::store;
>+            }
>+        }
>+    );
>+    return $self;
>+}
>+
>+=head3 make_default
>+
>+Set the current cash register as the branch default
>+
>+=cut
>+
>+sub make_default {
>+    my ($self) = @_;
>+
>+    $self->_result->result_source->schema->txn_do(
>+        sub {
>+            my $registers =
>+              Koha::Cash::Registers->search( { branch => $self->branch } );
>+            $registers->update( { branch_default => 0 } );
>+            $self->set( { branch_default => 1 } );
>+            $self->SUPER::store;
>+        }
>+    );
>+
>+    return $self;
>+}
>+
>+=head3 drop_default
>+
>+Drop the current cash register as the branch default
>+
>+=cut
>+
>+sub drop_default {
>+    my ($self) = @_;
>+
>+    $self->_result->result_source->schema->txn_do(
>+        sub {
>+            $self->set( { branch_default => 0 } );
>+            $self->SUPER::store;
>+        }
>+    );
>+
>+    return $self;
> }
> 
> =head2 Internal methods
>diff --git a/Koha/Exceptions/Object.pm b/Koha/Exceptions/Object.pm
>index d36d9dc5fc..93081f09a1 100644
>--- a/Koha/Exceptions/Object.pm
>+++ b/Koha/Exceptions/Object.pm
>@@ -41,6 +41,11 @@ use Exception::Class (
>         isa => 'Koha::Exceptions::Object',
>         description => "Invalid property",
>     },
>+    'Koha::Exceptions::Object::ReadOnlyProperty' => {
>+        isa => 'Koha::Exceptions::Object',
>+        description => "Change of readonly property attempted",
>+        fields => [ 'property' ],
>+    },
>     'Koha::Exceptions::Object::MethodNotCoveredByTests' => {
>         isa => 'Koha::Exceptions::Object',
>         description => "Method not covered by tests",
>@@ -64,6 +69,9 @@ sub full_message {
>         elsif ( $self->isa('Koha::Exceptions::Object::BadValue') ) {
>             $msg = sprintf("Invalid value passed, %s=%s expected type is %s", $self->property, $self->value, $self->type );
>         }
>+        elsif ( $self->isa('Koha::Exceptions::Object::ReadOnlyProperty') ) {
>+            $msg = sprintf("Invalid attempt to change readonly property: %s", $self->property );
>+        }
>     }
> 
>     return $msg;
>diff --git a/admin/cash_registers.pl b/admin/cash_registers.pl
>index 4e76e72944..3f10ed97bb 100755
>--- a/admin/cash_registers.pl
>+++ b/admin/cash_registers.pl
>@@ -128,9 +128,38 @@ elsif ( $op eq 'unarchive' ) {
>     $op = 'list';
> }
> 
>+elsif ( $op eq 'make_default' ) {
>+    if ($registerid) {
>+        try {
>+            my $cash_register = Koha::Cash::Registers->find($registerid);
>+            $cash_register->make_default;
>+            push @messages, { code => 'success_on_default', type => 'message' };
>+        }
>+        catch {
>+            push @messages, { code => 'error_on_default', type => 'alert' };
>+        }
>+    }
>+    $op = 'list';
>+}
>+elsif ( $op eq 'drop_default' ) {
>+    if ($registerid) {
>+        try {
>+            my $cash_register = Koha::Cash::Registers->find($registerid);
>+            $cash_register->drop_default;
>+            push @messages, { code => 'success_on_default', type => 'message' };
>+        }
>+        catch {
>+            push @messages, { code => 'error_on_default', type => 'alert' };
>+        }
>+    }
>+    $op = 'list';
>+}
>+
>+
> if ( $op eq 'list' ) {
>     my $cash_registers =
>-      Koha::Cash::Registers->search( {}, { prefetch => 'branch' } );
>+      Koha::Cash::Registers->search( {},
>+        { prefetch => 'branch', order_by => { -asc => [qw/branch name/] } } );
>     $template->param( cash_registers => $cash_registers, );
> }
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/_tables.scss b/koha-tmpl/intranet-tmpl/prog/css/src/_tables.scss
>index 97da85a8bf..f30ed516b0 100644
>--- a/koha-tmpl/intranet-tmpl/prog/css/src/_tables.scss
>+++ b/koha-tmpl/intranet-tmpl/prog/css/src/_tables.scss
>@@ -267,6 +267,12 @@ tr {
>             }
>         }
>     }
>+
>+    &.default {
>+        td {
>+            font-weight: bold;
>+        }
>+    }
> }
> 
> .table_borrowers {
>@@ -752,4 +758,4 @@ div {
>         margin: 0;
>         padding: 0;
>     }
>-}
>\ No newline at end of file
>+}
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt
>index 4413583a7c..35e6d63d24 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/cash_registers.tt
>@@ -1,9 +1,9 @@
> [% USE raw %]
> [% USE Asset %]
>+[% USE Branches %]
> [% SET footerjs = 1 %]
>-[% USE ColumnsSettings %]
> [% INCLUDE 'doc-head-open.inc' %]
>-<title>Koha &rsaquo; Administration &rsaquo; Cash Registers
>+<title>Koha &rsaquo; Administration &rsaquo; Cash registers
> [% IF op == 'add_form' %]
>     &rsaquo;[% IF cash_register %]Modify cash register[% ELSE %]New cash register [% cash_register.id | html %][% END %]
> [% ELSIF op == 'delete_confirm' %]
>@@ -36,20 +36,22 @@
>                 [% FOREACH m IN messages %]
>                 <div class="dialog [% m.type | html %]">
>                     [% SWITCH m.code %]
>-                    [% CASE 'error_on_update' %]
>-                        An error occurred when updating this cash register.
>+                    [% CASE 'success_on_insert' %]
>+                        Cash register added successfully.
>                     [% CASE 'error_on_insert' %]
>                         An error occurred when adding this cash register.
>                     [% CASE 'success_on_update' %]
>                         Cash register updated successfully.
>-                    [% CASE 'success_on_insert' %]
>-                        Cash register added successfully.
>+                    [% CASE 'error_on_update' %]
>+                        An error occurred when updating this cash register.
>+                    [% CASE 'success_on_default' %]
>+                        Branch default updated successfully.
>+                    [% CASE 'error_on_update' %]
>+                        An error on setting branch default.
>                     [% CASE 'success_on_archive' %]
>                         Cash register archived successfully.
>                     [% CASE 'success_on_restore' %]
>                         Cash register restored successfully.
>-                    [% CASE 'success_on_delete' %]
>-                        Cash register deleted successfully.
>                     [% CASE %]
>                         [% m.code | html %]
>                     [% END %]
>@@ -81,7 +83,7 @@
>                                 <input type="text" name="description" id="description" value="[% cash_register.description %]"/>
>                             </li>
>                             <li>
>-                                <label for="branch">Branch: </label>
>+                                <label for="branch">Library: </label>
>                                 <select id="branch" name="branch">
>                                 [% FOREACH branch IN branch_list %]
>                                 [% IF cash_register.branch == branch.branchcode %]
>@@ -116,27 +118,44 @@
>                    <a class="btn btn-default" id="newcashregister" href="/cgi-bin/koha/admin/cash_registers.pl?op=add_form"><i class="fa fa-plus"></i> New cash register</a>
>                </div>
> 
>-               <h3>Cash Registers</h3>
>+               <h3>Cash registers for <select id="branch_filter" name="branch_filter">
>+                       <option value=""></option>
>+                       [% PROCESS options_for_libraries libraries => Branches.all( selected => branchcode, unfiltered => 1, ) %]
>+                   </select>
>+               </h3>
>+
>                [% IF cash_registers.count %]
>                <table id="table_cash_registers">
>                    <thead>
>-                       <th>Id</th>
>                        <th>Name</th>
>                        <th>Description</th>
>-                       <th>Branch</th>
>+                       <th>Library</th>
>+                       <th>Default</th>
>                        <th>Initial float</th>
>                        <th>Actions</th>
>                    </thead>
>                    <tbody>
>                    [% FOREACH cash_register IN cash_registers %]
>+                   [% IF cash_register.branch_default %]
>+                   <tr class="default">
>+                   [% ELSE %]
>                    <tr>
>-                       <td>[% cash_register.id %]</td>
>+                   [% END %]
>                        <td>[% cash_register.name %]</td>
>                        <td>[% cash_register.description %]</td>
>                        <td>[% cash_register.library.branchname %]</td>
>+                       <td>[% IF cash_register.branch_default %]Yes[% ELSE %]No[% END %]</td>
>                        <td>[% cash_register.starting_float %]</td>
>                        [% IF cash_register.archived == '0' %]
>-                       <td class="actions"><a class="btn btn-default btn-xs" href="cash_registers.pl?op=add_form&amp;id=[% cash_register.id | uri %]"><i class="fa fa-pencil"></i> Edit</a><a class="btn btn-default btn-xs" href="cash_registers.pl?op=archive&amp;id=[% cash_register.id %]"><i class="fa fa-archive"></i> Archive</a></td>
>+                       <td class="actions">
>+                           <a class="btn btn-default btn-xs" href="cash_registers.pl?op=add_form&amp;id=[% cash_register.id | uri %]"><i class="fa fa-pencil"></i> Edit</a>
>+                           [% IF cash_register.branch_default %]
>+                           <a class="btn btn-default btn-xs" href="cash_registers.pl?op=drop_default&amp;id=[% cash_register.id %]"><i class="fa fa-archive"></i> Drop default</a>
>+                           [% ELSE %]
>+                           <a class="btn btn-default btn-xs" href="cash_registers.pl?op=make_default&amp;id=[% cash_register.id %]"><i class="fa fa-archive"></i> Make default</a>
>+                           [% END %]
>+                           <a class="btn btn-default btn-xs" href="cash_registers.pl?op=archive&amp;id=[% cash_register.id %]"><i class="fa fa-archive"></i> Archive</a>
>+                       </td>
>                        [% ELSE %]
>                        <td class="actions"><a class="btn btn-default btn-xs" href="cash_registers.pl?op=unarchive&amp;id=[% cash_register.id %]"><i class="fa fa-trash-restore"></i> Restore</a></td>
>                        [% END %]
>@@ -161,16 +180,39 @@
> [% MACRO jsinclude BLOCK %]
>     [% Asset.js("js/admin-menu.js") | $raw %]
>     [% INCLUDE 'datatables.inc' %]
>-
>     <script>
>+    function filterDataTable( table, column, term ){
>+        if( column ){
>+            table.column( column ).search( term ).draw();
>+        } else {
>+            table.search( term ).draw();
>+        }
>+        clearFilter( term );
>+    }
>+
>+    function clearFilter( term ){
>+        if( term == "" ){
>+            $(".dt_button_clear_filter").addClass("disabled");
>+        } else {
>+            $(".dt_button_clear_filter").removeClass("disabled");
>+        }
>+    }
>+
>     $(document).ready(function() {
>-        $("#table_cash_registers").dataTable($.extend(true, {}, dataTablesDefaults, {
>+        var crtable = $("#table_cash_registers").DataTable($.extend(true, {}, dataTablesDefaults, {
>               "aoColumnDefs": [
>                   { "aTargets": [ -1, -2 ], "bSortable": false, "bSearchable":false },
>                ],
>                "aaSorting": [[ 1, "asc" ]],
>                "paginationType": "full",
>         }));
>+
>+        $("#branch_filter").on("change", function(){
>+            // Table must be filtered by the <option>'s text, not its value
>+            var opt = $(this).find("option:selected").text();
>+            filterDataTable( crtable, 2, opt );
>+        });
>+
>     });
>     </script>
> [% END %]
>diff --git a/t/db_dependent/Koha/Cash/Register.t b/t/db_dependent/Koha/Cash/Register.t
>index eb2fcb3b30..74bef789c8 100644
>--- a/t/db_dependent/Koha/Cash/Register.t
>+++ b/t/db_dependent/Koha/Cash/Register.t
>@@ -19,12 +19,10 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 1;
>+use Test::More tests => 2;
> 
>-use C4::Context;
>+use Test::Exception;
> 
>-use Koha::Library;
>-use Koha::Libraries;
> use Koha::Database;
> 
> use t::lib::TestBuilder;
>@@ -55,3 +53,58 @@ subtest 'library' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'branch_default' => sub {
>+    plan tests => 3;
>+
>+    $schema->storage->txn_begin;
>+    my $library = $builder->build_object( { class => 'Koha::Libraries' } );
>+    my $register1 = $builder->build_object(
>+        {
>+            class => 'Koha::Cash::Registers',
>+            value => { branch => $library->branchcode, branch_default => 1 },
>+        }
>+    );
>+    my $register2 = $builder->build_object(
>+        {
>+            class => 'Koha::Cash::Registers',
>+            value => { branch => $library->branchcode, branch_default => 0 },
>+        }
>+    );
>+
>+    subtest 'store' => sub {
>+        plan tests => 2;
>+
>+        $register1->name('Test till 1');
>+        ok( $register1->store(),
>+            "Store works as expected when branch_default is not changed" );
>+
>+        $register1->branch_default(0);
>+        throws_ok { $register1->store(); }
>+        'Koha::Exceptions::Object::ReadOnlyProperty',
>+          'Exception thrown if direct update to branch_default is attempted';
>+
>+    };
>+
>+    subtest 'make_default' => sub {
>+        plan tests => 3;
>+
>+        ok($register2->make_default,'Koha::Register->make_default ran');
>+
>+        $register1 = $register1->get_from_storage;
>+        $register2 = $register2->get_from_storage;
>+        is($register1->branch_default, 0, 'register1 was unset as expected');
>+        is($register2->branch_default, 1, 'register2 was set as expected');
>+    };
>+
>+    subtest 'drop_default' => sub {
>+        plan tests => 2;
>+
>+        ok($register2->drop_default,'Koha::Register->drop_default ran');
>+
>+        $register2 = $register2->get_from_storage;
>+        is($register2->branch_default, 0, 'register2 was unset as expected');
>+    };
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.23.0

Actions: View | Diff | Splinter Review

Attachments on bug 23321: 91596 | 91597 | 91598 | 91599 | 91600 | 91601 | 91602 | 91621 | 92057 | 92058 | 92059 | 92060 | 92061 | 92062 | 92063 | 92064 | 92065 | 92083 | 92084 | 92085 | 92086 | 92087 | 92088 | 92089 | 92090 | 92091 | 92394 | 92395 | 92396 | 92397 | 92398 | 92399 | 92400 | 92401 | 92402 | 92467 | 92468 | 92469 | 92470 | 92471 | 92472 | 92473 | 92474 | 92475 | 92803 | 92804 | 92805 | 92806 | 92807 | 92808 | 92809 | 92810 | 92811 | 92812 | 92813 | 92814 | 92829 | 92879 | 92880 | 92881 | 92882 | 92883 | 92884 | 92885 | 92886 | 92887 | 92888 | 92889 | 93019 | 93020 | 93021 | 93022 | 93023 | 93024 | 93025 | 93026 | 93027 | 93028 | 93029 | 93030 | 93031 | 93032 | 93033 | 93034 | 93035 | 93188