Bugzilla – Attachment 123387 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.93 KB, created by
Nick Clemens (kidclamp)
on 2021-08-02 13:47:44 UTC
(
hide
)
Description:
Bug 24190: (follow-up) Respond to QA feedback
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2021-08-02 13:47:44 UTC
Size:
12.93 KB
patch
obsolete
>From 654131664a8fbb5d35b15978b59e8969a9ae3562 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 > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Budgets.pm | 31 +++++++++++---------- > acqui/addorder.pl | 14 +++++----- > acqui/finishreceive.pl | 16 ++++++----- > acqui/invoice.pl | 58 +++++++++------------------------------- > admin/aqbudgetperiods.pl | 20 +++++++------- > 5 files changed, 57 insertions(+), 82 deletions(-) > >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 439fcaf4b7..8b6d63c7e5 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; >@@ -645,15 +646,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; >@@ -668,19 +670,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 c7aa868968..dbfad999a1 100755 >--- a/acqui/addorder.pl >+++ b/acqui/addorder.pl >@@ -119,7 +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 qw( to_json ); >+use JSON qw ( to_json encode_json ); > use C4::Auth qw( get_template_and_user ); > use C4::Acquisition qw( FillWithDefaultValues populate_order_with_prices ModOrder ModOrderUsers ); > use C4::Suggestions qw( ModSuggestion ); >@@ -320,15 +320,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) > ); > } > } >@@ -336,15 +336,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 e3305be479..9934af6aa0 100755 >--- a/acqui/finishreceive.pl >+++ b/acqui/finishreceive.pl >@@ -23,6 +23,7 @@ > use Modern::Perl; > use CGI qw ( -utf8 ); > use C4::Auth qw( checkauth ); >+use JSON qw( encode_json ); > use C4::Output; > use C4::Context; > use C4::Acquisition qw( GetInvoice GetOrder populate_order_with_prices ModReceiveOrder ); >@@ -192,18 +193,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 2e05c0dbe0..8e73e6dd63 100755 >--- a/acqui/invoice.pl >+++ b/acqui/invoice.pl >@@ -33,6 +33,7 @@ use C4::Auth qw( get_template_and_user ); > use C4::Output qw( output_and_exit output_html_with_http_headers ); > use C4::Acquisition qw( CloseInvoice ReopenInvoice ModInvoice MergeInvoices DelInvoice GetInvoice GetInvoiceDetails get_rounded_price ); > use C4::Budgets qw( GetBudgetHierarchy GetBudget CanUserUseBudget ); >+use JSON qw( encode_json ); > use C4::Log qw(logaction); > > use Koha::Acquisition::Booksellers; >@@ -148,19 +149,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(); >@@ -195,12 +195,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) > ); > } > } >@@ -209,7 +208,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], >@@ -218,12 +217,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); >@@ -350,35 +349,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 d6192c2d7c..091943acb3 100755 >--- a/admin/aqbudgetperiods.pl >+++ b/admin/aqbudgetperiods.pl >@@ -48,6 +48,7 @@ use Modern::Perl; > > use CGI qw ( -utf8 ); > use Koha::DateUtils qw( dt_from_string ); >+use JSON qw( encode_json ); > use Koha::Database; > use C4::Koha; > use C4::Context; >@@ -121,19 +122,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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
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