Bugzilla – Attachment 112511 Details for
Bug 24190
Add additional Acquisition logging
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24190: (follow-up) Respond to QA feedback
Bug-24190-follow-up-Respond-to-QA-feedback.patch (text/plain), 12.33 KB, created by
Andrew Isherwood
on 2020-10-26 11:46:49 UTC
(
hide
)
Description:
Bug 24190: (follow-up) Respond to QA feedback
Filename:
MIME Type:
Creator:
Andrew Isherwood
Created:
2020-10-26 11:46:49 UTC
Size:
12.33 KB
patch
obsolete
>From 252725c6ee0377fe5df094a5dfaf7fa1ed06cb4e Mon Sep 17 00:00:00 2001 >From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> >Date: Mon, 28 Sep 2020 10:41:16 +0100 >Subject: [PATCH] Bug 24190: (follow-up) Respond to QA feedback > >This commit makes changes in response to Jonathan's feedback in comment > >- Moved from using zero padded strings to store log data to a JSON >object >- Stopped storing formatted dates in logged data >--- > C4/Budgets.pm | 31 +++++++++++---------- > acqui/addorder.pl | 13 ++++----- > acqui/finishreceive.pl | 16 ++++++----- > acqui/invoice.pl | 58 +++++++++------------------------------- > admin/aqbudgetperiods.pl | 20 +++++++------- > 5 files changed, 57 insertions(+), 81 deletions(-) > >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 5086d71715..6e9442880b 100644 >--- a/C4/Budgets.pm >+++ b/C4/Budgets.pm >@@ -18,6 +18,7 @@ package C4::Budgets; > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >+use JSON; > use C4::Context; > use Koha::Database; > use Koha::Patrons; >@@ -649,15 +650,16 @@ sub AddBudget { > > # Log the addition > if (C4::Context->preference("AcqLog")) { >- my $infos = >- sprintf("%010d", $budget->{budget_amount}) . >- sprintf("%010d", $budget->{budget_encumb}) . >- sprintf("%010d", $budget->{budget_expend}); >+ my $infos = { >+ budget_amount => $budget->{budget_amount}, >+ budget_encumb => $budget->{budget_encumb}, >+ budget_expend => $budget->{budget_expend} >+ }; > logaction( > 'ACQUISITIONS', > 'CREATE_FUND', > $id, >- $infos >+ encode_json($infos) > ); > } > return $id; >@@ -672,19 +674,20 @@ sub ModBudget { > > # Log this modification > if (C4::Context->preference("AcqLog")) { >- my $infos = >- sprintf("%010d", $budget->{budget_amount}) . >- sprintf("%010d", $budget->{budget_encumb}) . >- sprintf("%010d", $budget->{budget_expend}) . >- sprintf("%010d", $result->budget_amount) . >- sprintf("%010d", $result->budget_encumb) . >- sprintf("%010d", $result->budget_expend) . >- sprintf("%010d", 0 - ($result->budget_amount - $budget->{budget_amount})); >+ my $infos = { >+ budget_amount_new => $budget->{budget_amount}, >+ budget_encumb_new => $budget->{budget_encumb}, >+ budget_expend_new => $budget->{budget_expend}, >+ budget_amount_old => $result->budget_amount, >+ budget_encumb_old => $result->budget_encumb, >+ budget_expend_old => $result->budget_expend, >+ budget_amount_change => 0 - ($result->budget_amount - $budget->{budget_amount}) >+ }; > logaction( > 'ACQUISITIONS', > 'MODIFY_FUND', > $budget->{budget_id}, >- $infos >+ encode_json($infos) > ); > } > >diff --git a/acqui/addorder.pl b/acqui/addorder.pl >index 96090e6c4d..2626c2a241 100755 >--- a/acqui/addorder.pl >+++ b/acqui/addorder.pl >@@ -119,6 +119,7 @@ if it is an order from an existing suggestion : the id of this suggestion. > > use Modern::Perl; > use CGI qw ( -utf8 ); >+use JSON; > use C4::Auth; # get_template_and_user > use C4::Acquisition; # ModOrder > use C4::Suggestions; # ModStatus >@@ -314,15 +315,15 @@ if ( $basket->{is_standing} || $orderinfo->{quantity} ne '0' ) { > ModOrder($orderinfo); > # Log the order modification > if (C4::Context->preference("AcqLog")) { >- my $infos = ''; >+ my $infos = {}; > foreach my $field(@log_order_fields) { >- $infos .= sprintf("%010d", $orderinfo->{$field}); >+ $infos->{$field} = $orderinfo->{$field}; > } > logaction( > 'ACQUISITIONS', > 'MODIFY_ORDER', > $orderinfo->{ordernumber}, >- $infos >+ encode_json($infos) > ); > } > } >@@ -330,15 +331,15 @@ if ( $basket->{is_standing} || $orderinfo->{quantity} ne '0' ) { > $order->store; > # Log the order creation > if (C4::Context->preference("AcqLog")) { >- my $infos = ''; >+ my $infos = {}; > foreach my $field(@log_order_fields) { >- $infos .= sprintf("%010d", $orderinfo->{$field}); >+ $infos->{$field} = $orderinfo->{$field}; > } > logaction( > 'ACQUISITIONS', > 'CREATE_ORDER', > $order->ordernumber, >- $infos >+ encode_json($infos) > ); > } > } >diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl >index d47a97b31b..a0f3aa1ffa 100755 >--- a/acqui/finishreceive.pl >+++ b/acqui/finishreceive.pl >@@ -22,6 +22,7 @@ > > use Modern::Perl; > use CGI qw ( -utf8 ); >+use JSON; > use C4::Auth; > use C4::Output; > use C4::Context; >@@ -193,18 +194,19 @@ if ($suggestion_id) { > > # Log the receipt > if (C4::Context->preference("AcqLog")) { >- my $infos = >- sprintf("%010d", $quantityrec) . >- sprintf("%010d", $bookfund) . >- sprintf("%010.2f", $input->param("tax_rate")) . >- sprintf("%010.2f", $replacementprice) . >- sprintf("%010.2f", $unitprice); >+ my $infos = { >+ quantityrec => $quantityrec, >+ bookfund => $bookfund, >+ tax_rate => $input->param("tax_rate"), >+ replacementprice => $replacementprice, >+ unitprice => $unitprice >+ }; > > logaction( > 'ACQUISITIONS', > 'RECEIVE_ORDER', > $ordernumber, >- $infos >+ encode_json($infos) > ); > } > >diff --git a/acqui/invoice.pl b/acqui/invoice.pl >index 5fbc3b9af7..32d9d607fa 100755 >--- a/acqui/invoice.pl >+++ b/acqui/invoice.pl >@@ -29,6 +29,7 @@ Invoice details > use Modern::Perl; > > use CGI qw ( -utf8 ); >+use JSON; > use C4::Auth; > use C4::Output; > use C4::Acquisition; >@@ -144,19 +145,18 @@ elsif ( $op && $op eq 'del_adj' ) { > my $del_adj = Koha::Acquisition::Invoice::Adjustments->find( $adjustment_id ); > if ($del_adj) { > if (C4::Context->preference("AcqLog")) { >- my $reason_length = length $del_adj->reason; >- my $reason_padded = ( ' ' x (80 - $reason_length) ) . $del_adj->reason; >- my $infos = >- sprintf("%010d", $del_adj->invoiceid) . >- sprintf("%010d", $del_adj->budget_id) . >- sprintf("%010d", $del_adj->encumber_open) . >- sprintf("%010.2f", $del_adj->adjustment) . >- $reason_padded; >+ my $infos = { >+ invoiceid => $del_adj->invoiceid, >+ budget_id => $del_adj->budget_id, >+ encumber_open => $del_adj->encumber_open, >+ adjustment => $del_adj->adjustment, >+ reason => $del_adj->reason >+ }; > logaction( > 'ACQUISITIONS', > 'DELETE_INVOICE_ADJUSTMENT', > $adjustment_id, >- $infos >+ encode_json($infos) > ); > } > $del_adj->delete(); >@@ -191,12 +191,11 @@ elsif ( $op && $op eq 'mod_adj' ) { > $new_adj->store(); > # Log this addition > if (C4::Context->preference("AcqLog")) { >- my $infos = get_log_string($adj); > logaction( > 'ACQUISITIONS', > 'CREATE_INVOICE_ADJUSTMENT', > $new_adj->adjustment_id, >- $infos >+ encode_json($adj) > ); > } > } >@@ -205,7 +204,7 @@ elsif ( $op && $op eq 'mod_adj' ) { > unless ( $old_adj->adjustment == $adjustment[$i] && $old_adj->reason eq $reason[$i] && $old_adj->budget_id == $budget_id[$i] && $old_adj->encumber_open == $e_open{$adjustment_id[$i]} && $old_adj->note eq $note[$i] ){ > # Log this modification > if (C4::Context->preference("AcqLog")) { >- my $infos = get_log_string({ >+ my $infos = { > adjustment => $adjustment[$i], > reason => $reason[$i], > budget_id => $budget_id[$i], >@@ -214,12 +213,12 @@ elsif ( $op && $op eq 'mod_adj' ) { > reason_old => $old_adj->reason, > budget_id_old => $old_adj->budget_id, > encumber_open_old => $old_adj->encumber_open >- }); >+ }; > logaction( > 'ACQUISITIONS', > 'UPDATE_INVOICE_ADJUSTMENT', > $adjustment_id[$i], >- $infos >+ encode_json($infos) > ); > } > $old_adj->timestamp(undef); >@@ -346,35 +345,4 @@ sub get_infos { > return \%line; > } > >-sub get_log_string { >- my ($data) = @_; >- >- # We specify the keys we're dealing for logging within an array in order >- # to preserve order, if we just iterate the hash keys, we could get >- # the values in any order >- my @keys = ( >- 'budget_id', >- 'encumber_open', >- 'budget_id_old', >- 'encumber_open_old' >- ); >- # Adjustment amount >- my $return = sprintf("%010.2f", $data->{adjustment}); >- # Left pad "reason" to the maximum length of aqinvoice_adjustments.reason >- # (80 characters) >- my $reason_len = length $data->{reason}; >- $return .= ( ' ' x (80 - $reason_len) ) . $data->{reason}; >- # Append the remaining fields >- foreach my $key(@keys) { >- $return .= sprintf("%010d", $data->{$key}); >- } >- # Old adjustment amount >- $return .= sprintf("%010.2f", $data->{adjustment_old}); >- # Left pad "reason_old" to the maximum length of aqinvoice_adjustments.reason >- # (80 characters) >- my $reason_old_len = length $data->{reason_old}; >- $return .= ( ' ' x (80 - $reason_old_len) ) . $data->{reason_old}; >- return $return; >-} >- > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/admin/aqbudgetperiods.pl b/admin/aqbudgetperiods.pl >index 570ed16b55..3a846fe655 100755 >--- a/admin/aqbudgetperiods.pl >+++ b/admin/aqbudgetperiods.pl >@@ -48,6 +48,7 @@ use Modern::Perl; > > use CGI qw ( -utf8 ); > use List::Util qw/min/; >+use JSON; > use Koha::DateUtils; > use Koha::Database; > use C4::Koha; >@@ -124,19 +125,20 @@ elsif ( $op eq 'add_validate' ) { > # Log the budget modification > if (C4::Context->preference("AcqLog")) { > my $diff = 0 - ($budgetperiod_old->{budget_period_total} - $budget_period_hashref->{budget_period_total}); >- my $infos = >- eval { output_pref({ dt => dt_from_string( $input->param('budget_period_startdate') ), dateformat => 'iso', dateonly => 1 } ); } . >- eval { output_pref({ dt => dt_from_string( $input->param('budget_period_enddate') ), dateformat => 'iso', dateonly => 1 } ); } . >- sprintf("%010d", $budget_period_hashref->{budget_period_total}) . >- eval { output_pref({ dt => dt_from_string( $budgetperiod_old->{budget_period_startdate} ), dateformat => 'iso', dateonly => 1 } ); } . >- eval { output_pref({ dt => dt_from_string( $budgetperiod_old->{budget_period_enddate} ), dateformat => 'iso', dateonly => 1 } ); } . >- sprintf("%010d", $budgetperiod_old->{budget_period_total}) . >- sprintf("%010d", $diff); >+ my $infos = { >+ budget_period_startdate => $input->param('budget_period_startdate'), >+ budget_period_enddate => $input->param('budget_period_enddate'), >+ budget_period_total => $budget_period_hashref->{budget_period_total}, >+ budget_period_startdate_old => $budgetperiod_old->{budget_period_startdate}, >+ budget_period_enddate_old => $budgetperiod_old->{budget_period_enddate}, >+ budget_period_total_old => $budgetperiod_old->{budget_period_total}, >+ budget_period_total_change => $diff >+ }; > logaction( > 'ACQUISITIONS', > 'MODIFY_BUDGET', > $budget_period_id, >- $infos >+ encode_json($infos) > ); > } > } >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=112511">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24190
:
96364
|
96369
|
100243
|
105082
|
105083
|
106096
|
106097
|
106098
|
106099
|
111080
|
111081
|
111082
|
111083
|
111084
|
112509
|
112510
|
112511
|
112512
|
112513
|
112514
|
116148
|
116149
|
116150
|
116151
|
116152
|
116153
|
122040
|
122041
|
122042
|
122043
|
122044
|
122045
|
123385
|
123386
|
123387
|
123388
|
123389
|
123390
|
123391
|
125119