From 0d5d72e0e72de48b5157f326c7b9027fd1394882 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 17 May 2018 16:57:00 +0300 Subject: [PATCH] Bug 14806 - (follow-up) Action Log for Modifying message_transport_types Introducing improvements: - Centralizing code into new Koha::Patron::Message::Preference(s) objects - Addings tests --- C4/Form/MessagingPreferences.pm | 38 ++---------------- Koha/Patron/Message/Preference.pm | 35 ++++++++++++++++ Koha/Patron/Message/Preferences.pm | 24 +++++++++++ t/db_dependent/Koha/Patron/Message/Preferences.t | 51 +++++++++++++++++++++++- 4 files changed, 112 insertions(+), 36 deletions(-) diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index 7640fbf..9bf3916 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/C4/Form/MessagingPreferences.pm @@ -20,7 +20,6 @@ package C4::Form::MessagingPreferences; use strict; use warnings; -use Data::Dumper; use CGI qw ( -utf8 ); use C4::Context; use C4::Debug; @@ -145,12 +144,12 @@ sub handle_form_action { categorycode => $categorycode || $updater->{'categorycode'}, }); unless ($preference) { - Koha::Patron::Message::Preference->new($updater)->store; + $preference = Koha::Patron::Message::Preference->new($updater)->store; } else { $preference->set($updater)->store; } - _pushToActionLogBuffer($logEntries, $updater, $option); + $preference->_push_to_action_buffer($logEntries); if ($query->param( $option->{'message_attribute_id'})){ $prefs_set = 1; @@ -165,7 +164,7 @@ sub handle_form_action { # show the success message $template->param( settings_updated => 1 ); - _writeActionLogBuffer($logEntries, $borrowernumber); + Koha::Patron::Message::Preferences->_log_action_buffer($logEntries, $borrowernumber); } =head2 set_form_values @@ -227,37 +226,6 @@ sub _transport_set { return List::MoreUtils::firstidx { $_ eq $name } @transport_methods; } -sub _pushToActionLogBuffer { - return unless C4::Context->preference("BorrowersLog"); - my ($logEntries, $updater, $option) = @_; - - if ($updater->{message_transport_types} && scalar(@{$updater->{message_transport_types}})) { - my $entry = {}; - $entry->{cc} = $updater->{categorycode} if $updater->{categorycode}; - $entry->{dig} = $updater->{wants_digest} if $updater->{wants_digest}; - $entry->{da} = $updater->{days_in_advance} if $updater->{days_in_advance}; - $entry->{mtt} = $updater->{message_transport_types}; - $entry->{_name} = $option->{message_name}; - push(@$logEntries, $entry); - } -} - -sub _writeActionLogBuffer { - return unless C4::Context->preference("BorrowersLog"); - my ($logEntries, $borrowernumber) = @_; - - if (scalar(@$logEntries)) { - my $d = Data::Dumper->new([$logEntries]); - $d->Indent(0); - $d->Purity(0); - $d->Terse(1); - C4::Log::logaction('MEMBERS', 'MOD MTT', $borrowernumber, $d->Dump($logEntries)); - } - else { - C4::Log::logaction('MEMBERS', 'MOD MTT', $borrowernumber, 'All message_transports removed') - } -} - =head1 TODO =over 4 diff --git a/Koha/Patron/Message/Preference.pm b/Koha/Patron/Message/Preference.pm index 1e0c443..89f2dbe 100644 --- a/Koha/Patron/Message/Preference.pm +++ b/Koha/Patron/Message/Preference.pm @@ -583,6 +583,41 @@ sub _validate_message_transport_types { } } +sub _push_to_action_buffer { + return unless C4::Context->preference("BorrowersLog"); + my $self = shift; + my ($logEntries) = @_; + return unless $logEntries; + Koha::Exceptions::MissingParameter->throw( + error => 'You must pass an array reference to '. + 'Koha::Patron::Message::Preference::_push_to_action_buffer' + ) if ref($logEntries) ne 'ARRAY'; + + my @mtts = keys %{$self->message_transport_types}; + if (@mtts) { + my $entry = {}; + $entry->{cc} = $self->categorycode if $self->categorycode; + $entry->{dig} = $self->wants_digest if $self->wants_digest; + $entry->{da} = $self->days_in_advance if $self->days_in_advance; + $entry->{mtt} = \@mtts; + $entry->{_name} = $self->message_name; + push(@$logEntries, $entry); + } + + return $logEntries; +} + +sub _log_action_buffer { + my $self = shift; + my ($logEntries, $borrowernumber) = @_; + $borrowernumber //= defined $self->borrowernumber + ? $self->borrowernumber : undef; + + Koha::Patron::Message::Preferences->_log_action_buffer( + $logEntries, $borrowernumber + ); +} + =head3 type =cut diff --git a/Koha/Patron/Message/Preferences.pm b/Koha/Patron/Message/Preferences.pm index 093652c..bfc0b93 100644 --- a/Koha/Patron/Message/Preferences.pm +++ b/Koha/Patron/Message/Preferences.pm @@ -19,11 +19,15 @@ package Koha::Patron::Message::Preferences; use Modern::Perl; +use C4::Context; + use Koha::Database; use Koha::Patron::Message::Attributes; use Koha::Patron::Message::Preference; use Koha::Patron::Message::Transports; +use Data::Dumper; + use base qw(Koha::Objects); =head1 NAME @@ -154,6 +158,26 @@ sub TO_JSON { return $preferences; } +sub _log_action_buffer { + return 0 unless C4::Context->preference("BorrowersLog"); + my $self = shift; + my ($logEntries, $borrowernumber) = @_; + return 0 unless $logEntries; + + if (scalar(@$logEntries)) { + my $d = Data::Dumper->new([$logEntries]); + $d->Indent(0); + $d->Purity(0); + $d->Terse(1); + C4::Log::logaction('MEMBERS', 'MOD MTT', $borrowernumber, $d->Dump($logEntries)); + } + else { + C4::Log::logaction('MEMBERS', 'MOD MTT', $borrowernumber, 'All message_transport_types removed') + } + + return 1; +} + =head3 type =cut diff --git a/t/db_dependent/Koha/Patron/Message/Preferences.t b/t/db_dependent/Koha/Patron/Message/Preferences.t index 094c3a7..2c17112 100644 --- a/t/db_dependent/Koha/Patron/Message/Preferences.t +++ b/t/db_dependent/Koha/Patron/Message/Preferences.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 8; +use Test::More tests => 9; use t::lib::Mocks; use t::lib::TestBuilder; @@ -365,6 +365,55 @@ subtest 'Add preferences from defaults' => sub { $schema->storage->txn_rollback; }; +subtest 'Test action logging' => sub { + plan tests => 8; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference('BorrowersLog', 1); + + my $patron = build_a_test_patron(); + my ($default, $mtt1, $mtt2) = build_a_test_category_preference({ + patron => $patron, + }); + Koha::Patron::Message::Preference->new_from_default({ + borrowernumber => $patron->borrowernumber, + categorycode => $patron->categorycode, + message_attribute_id => $default->message_attribute_id, + })->store; + my $pref = Koha::Patron::Message::Preferences->find({ + borrowernumber => $patron->borrowernumber, + message_attribute_id => $default->message_attribute_id, + }); + + my $actionLog = []; + $pref->_push_to_action_buffer($actionLog); + is(ref($actionLog), 'ARRAY', 'Action log is an array'); + is(@{$actionLog}, 1, 'One element in action log'); + is($actionLog->[0]->{_name}, $pref->message_name, 'Correct message name'); + is(@{$actionLog->[0]->{mtt}}, keys %{$pref->message_transport_types}, + 'Correct mtts'); + + # Let's push it another time for the lulz + $pref->_push_to_action_buffer($actionLog); + is(@{$actionLog}, 2, 'Two elements in action log'); + + $pref->_log_action_buffer($actionLog, $patron->borrowernumber); + my $logs = C4::Log::GetLogs("","","",["MEMBERS"],["MOD MTT"], + $patron->borrowernumber,""); + is($logs->[0]->{action}, 'MOD MTT', 'Correct log action'); + is($logs->[0]->{object}, $patron->borrowernumber, 'Correct borrowernumber'); + + # Test empty action log buffer + my $patron2 = build_a_test_patron(); + $pref->_log_action_buffer([], $patron2->borrowernumber); + $logs = C4::Log::GetLogs("","","",["MEMBERS"],["MOD MTT"], + $patron2->borrowernumber,""); + is($logs->[0]->{info}, 'All message_transport_types removed', 'All message_transport_types removed'); + + $schema->storage->txn_rollback; +}; + subtest 'Test Koha::Patron::Message::Preference->message_transport_types' => sub { plan tests => 4; -- 2.7.4