From e81c71ef309773630de2461ba6e68766e7d8ed93 Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Mon, 28 Jan 2019 17:06:50 +0000 Subject: [PATCH] Bug 20750: (follow-up) Fix logging of status changes Since this bug now is dependent on Bug 20581, it should be aware of the custom statuses provided in 20581. This patch enables logging of request statuses being changed to custom ones. As such the test plan is modified: To test: - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the Authorised Values ILLSTATUS category - Create an ILL request and perform actions on it that change it's status. - Edit the request and change status to a custom one - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. --- Koha/Illrequest.pm | 43 +++++++++++++++++++++- Koha/Illrequest/Logger.pm | 8 ++++ .../prog/en/modules/ill/log/status_change.tt | 16 +++++++- t/db_dependent/Illrequest/Logger.t | 31 +--------------- t/db_dependent/Illrequests.t | 4 +- 5 files changed, 67 insertions(+), 35 deletions(-) diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 8a27bc3357..117e815ca6 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -171,6 +171,42 @@ sub patron { ); } +=head3 status_alias + + $Illrequest->status_alias(143); + +Overloaded getter/setter for request status_alias, +also records the fact that the status has changed + +=cut + +sub status_alias { + my ($self, $new_status_alias) = @_; + + my $current_status_alias = $self->SUPER::status_alias; + + if ($new_status_alias) { + # Keep a record of the previous status before we change it, + # we might need it + $self->{previous_status} = $current_status_alias ? + $current_status_alias : + scalar $self->status; + my $ret = $self->SUPER::status_alias($new_status_alias)->store; + if ($ret) { + my $logger = Koha::Illrequest::Logger->new; + $logger->log_status_change( + $self, + $new_status_alias + ); + } else { + delete $self->{previous_status}; + } + return $ret; + } else { + return $current_status_alias; + } +} + =head3 status $Illrequest->status('CANREQ'); @@ -184,14 +220,17 @@ sub status { my ( $self, $new_status) = @_; my $current_status = $self->SUPER::status; + my $current_status_alias = $self->SUPER::status_alias; if ($new_status) { # Keep a record of the previous status before we change it, # we might need it - $self->{previous_status} = $current_status; + $self->{previous_status} = $current_status_alias ? + $current_status_alias : + $current_status; my $ret = $self->SUPER::status($new_status)->store; if ($ret) { - $self->status_alias(undef); + $self->SUPER::status_alias(undef); my $logger = Koha::Illrequest::Logger->new; $logger->log_status_change( $self, diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm index c68d206bcc..1b016f84d8 100644 --- a/Koha/Illrequest/Logger.pm +++ b/Koha/Illrequest/Logger.pm @@ -21,6 +21,7 @@ use Modern::Perl; use JSON qw( to_json from_json ); use Time::Local; +use C4::Koha; use C4::Context; use C4::Templates; use C4::Log qw( logaction GetLogs ); @@ -238,7 +239,14 @@ sub get_request_logs { undef, undef ); + # Populate a lookup table for status aliases + my $aliases = GetAuthorisedValues('ILLSTATUS'); + my $alias_hash; + foreach my $alias(@{$aliases}) { + $alias_hash->{$alias->{id}} = $alias; + } foreach my $log(@{$logs}) { + $log->{aliases} = $alias_hash; $log->{info} = from_json($log->{info}); $log->{template} = $self->get_log_template( $request, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt index 78e994ee87..7fade01edb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt @@ -1,7 +1,19 @@

[% log.timestamp | $KohaDates with_hours => 1 %] : Status changed [% IF log.info.status_before %] -from "[% request.capabilities(log.info.status_before).name | html %]" +[% before = log.info.status_before %] +[% + display_before = log.info.status_before.match('^\d+$') ? + log.aliases.$before.lib : + request.capabilities.$before.name +%] +from "[% display_before %]" [% END %] -to "[% request.capabilities(log.info.status_after).name | html %]" +[% after = log.info.status_after %] +[% + display_after = log.info.status_after.match('^\d+$') ? + log.aliases.$after.lib : + request.capabilities.$after.name; +%] +to "[% display_after | html %]"

diff --git a/t/db_dependent/Illrequest/Logger.t b/t/db_dependent/Illrequest/Logger.t index 1e9ffcf5ad..b9c9ac8071 100644 --- a/t/db_dependent/Illrequest/Logger.t +++ b/t/db_dependent/Illrequest/Logger.t @@ -57,7 +57,7 @@ use_ok('Koha::Illrequest::Logger'); subtest 'Basics' => sub { - plan tests => 10; + plan tests => 9; $schema->storage->txn_begin; @@ -120,35 +120,6 @@ subtest 'Basics' => sub { 'get_log_template() fetches correct core template' ); - # get_request_logs - # - my $res_obj = [ - { - 'template' => 'mock', - 'info' => { 'log_origin' => 'core' }, - 'timestamp' => '2018-10-02 11:12:32', - 'action' => 'STATUS_CHANGE' - }, - { - 'template' => 'mock', - 'action' => 'STATUS_CHANGE', - 'timestamp' => '2018-10-02 11:12:22', - 'info' => { 'log_origin' => 'core' } - }, - { - 'template' => 'mock', - 'action' => 'STATUS_CHANGE', - 'info' => { 'log_origin' => 'core' }, - 'timestamp' => '2018-10-02 11:12:12' - } - ]; - my $me_mock = Test::MockModule->new('Koha::Illrequest::Logger'); - $me_mock->mock('get_log_template', sub { return 'mock'; }); - my $mock_req = Test::MockObject->new(); - $mock_req->mock('id', sub { 1 }); - is_deeply($logger->get_request_logs($mock_req), $res_obj, - 'get_request_logs() returns logs property structured and sorted'); - $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Illrequests.t b/t/db_dependent/Illrequests.t index fc20d2d0aa..620a8b84c7 100644 --- a/t/db_dependent/Illrequests.t +++ b/t/db_dependent/Illrequests.t @@ -39,7 +39,7 @@ use_ok('Koha::Illrequests'); subtest 'Basic object tests' => sub { - plan tests => 24; + plan tests => 25; $schema->storage->txn_begin; @@ -62,6 +62,8 @@ subtest 'Basic object tests' => sub { "Branchcode getter works."); is($illrq_obj->status, $illrq->{status}, "Status getter works."); + is($illrq_obj->status_alias, $illrq->{status_alias}, + "Status_alias getter works."); is($illrq_obj->placed, $illrq->{placed}, "Placed getter works."); is($illrq_obj->replied, $illrq->{replied}, -- 2.11.0