From 908aa7a7cfd7da1ef7a1c22ae3cb23f04375f673 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 Signed-off-by: Tomas Cohen Arazi --- 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' %] -Koha › Administration › Cash Registers +<title>Koha › Administration › Cash registers [% IF op == 'add_form' %] ›[% 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&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&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&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&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&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&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&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