From 13c8557e4bc515a6f54e387506f0943eb8434f1d Mon Sep 17 00:00:00 2001 From: Charles Farmer Date: Fri, 23 Feb 2018 17:33:20 -0500 Subject: [PATCH] Bug 12502: QA Follow-up 1) The method under C4 was deleted. There wasn't any reason for a method under Koha, in the end, because it's a simple update that requires no additional processing. If I had to do any legwork with the data before updating the Order fields, I would've written a wrapper. As it stands right now, the work can be done directly in the controller without polluting the Koha/Acquisition/Orders.pm class. 2) Added the ISBN field to csv_header, and added that value to lateorders-export I've also changed the string internaldate to internalnote, small typo fix. 3) Made adjustements client-side and server-side. In the browser, you can now see a small loading icon to inform the user that the 'Save note' button actually triggered an action. A new alert message also informs the user if something awry happened while updating the orders' notes. I took a look at the user page, to see how notes were saved, but I was not satisfied with the idea of mixing an Ajax call with a window.location.reload; doesn't make sense in my mind to uselessly reload the page if the information had already reached the server, and since it was already coded that way, I felt like this change was the best course of action. Server-side, it is now possible to save an empty field. Before this patch, 'if($note)' would actually fail on an empty string, which meant you could never erase a note, for any reason whatsoever. After testing this patch, if the width of the input is still something that you'd like to see changed, I'll have no problem reducing it a little, but you'll have to tell me how large is _too_ large :) 4) Order no. -> Order line 5) Added the new columns to the admin/visibility yaml 6) It would be possible to also display an ISSN, but I won't make this patch any large than it is for now. --- C4/Acquisition.pm | 21 --------------------- acqui/lateorders-export.pl | 3 ++- acqui/lateorders.pl | 12 +++++++++++- admin/columns_settings.yml | 8 ++++++++ .../en/includes/csv_headers/acqui/lateorders.tt | 2 +- .../prog/en/modules/acqui/lateorders.tt | 21 +++++++++++++++------ 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 58c12ef..587dd65 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -93,8 +93,6 @@ BEGIN { &NotifyOrderUsers &FillWithDefaultValues - - &ModNotesOrder ); } @@ -1373,25 +1371,6 @@ sub ModOrder { #------------------------------------------------------------# -=head3 ModNotesOrder - - ModNotesOrder($orders); - -Modifies internal and vendor note off all items in orders - -=cut - -sub ModNotesOrder{ - my $orders = shift; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("UPDATE aqorders SET order_internalnote = ?, order_vendornote = ? WHERE ordernumber = ?"); - foreach my $ordernumber (keys %$orders) { - $sth->execute($orders->{$ordernumber}->{'i'}, $orders->{$ordernumber}->{'v'}, $ordernumber) if ($orders->{$ordernumber}->{'i'} || $orders->{$ordernumber}->{'v'}); - } -} - - #------------------------------------------------------------# - =head3 ModItemOrder ModItemOrder($itemnumber, $ordernumber); diff --git a/acqui/lateorders-export.pl b/acqui/lateorders-export.pl index 70f73e2..332f652 100755 --- a/acqui/lateorders-export.pl +++ b/acqui/lateorders-export.pl @@ -53,8 +53,9 @@ for my $ordernumber ( @ordernumbers ) { basketno => $order->{basketno}, claims_count => $order->{claims_count}, claimed_date => $order->{claimed_date}, - internalnote => $order->{internaldate}, + internalnote => $order->{internalnote}, vendornote => $order->{vendornote}, + isbn => $order->{isbn}, } ; } diff --git a/acqui/lateorders.pl b/acqui/lateorders.pl index 1f440ca..77f5f10 100755 --- a/acqui/lateorders.pl +++ b/acqui/lateorders.pl @@ -53,6 +53,7 @@ use C4::Context; use C4::Acquisition; use C4::Letters; use Koha::DateUtils; +use Koha::Acquisition:Orders; my $input = new CGI; my ($template, $loggedinuser, $cookie) = get_template_and_user( @@ -133,7 +134,16 @@ if ($op && $op eq "save"){ $listorders->{$split[0]}->{$split[1]} = $input->param($order); } } - ModNotesOrder($listorders); + + my $orders_rs = Koha::Acquisition::Orders->search({ ordernumber => [ keys %$listorders ] }); + while ( my $order = $orders_rs->next ) { + my $internalnote = $listorders->{$order->ordernumber}->{i}; + my $vendornote = $listorders->{$order->ordernumber}->{v}; + + $order->order_internalnote($internalnote) if defined $internalnote; + $order->order_vendornote($vendornote) if defined $vendornote; + $order->store; + } } my @parameters = ( $delay ); diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml index f56a745..134a6a2 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -38,6 +38,8 @@ modules: cannot_be_toggled: 1 cannot_be_modified: 1 - + columnname: order_line + - columnname: order_date - columnname: estimated_delivery_date @@ -59,6 +61,12 @@ modules: columnname: claims_count - columnname: claimed_date + - + columnname: internal_note + - + columnname: vendor_note + - + columnname: isbn admin: currency: diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt index f2c55d6..d27d2b0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csv_headers/acqui/lateorders.tt @@ -1,4 +1,4 @@ [%- USE Koha -%] [%- SET delimiter = Koha.Preference( 'delimiter' ) || ',' -%] -[%- BLOCK -%]ORDER DATE[%- delimiter -%]ESTIMATED DELIVERY DATE[%- delimiter -%]VENDOR[%- delimiter -%]INFORMATION[%- delimiter -%]TOTAL COST[%- delimiter -%]BASKET[%- delimiter -%]CLAIMS COUNT[%- delimiter -%]CLAIMED DATE[%- delimiter -%]INTERNAL NOTE[%- delimiter -%]VENDOR NOTE[%- END -%] +[%- BLOCK -%]ORDER DATE[%- delimiter -%]ESTIMATED DELIVERY DATE[%- delimiter -%]VENDOR[%- delimiter -%]INFORMATION[%- delimiter -%]TOTAL COST[%- delimiter -%]BASKET[%- delimiter -%]CLAIMS COUNT[%- delimiter -%]CLAIMED DATE[%- delimiter -%]INTERNAL NOTE[%- delimiter -%]VENDOR NOTE[%- delimiter -%]ISBN[%- END -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt index e30e137..2514adb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt @@ -66,6 +66,7 @@ $(document).ready(function() { }); // Update all notes $("#savenotes").click(function(){ + $("#savenotes").after("\"\" "+_("Loading")+""); var all_nodes = $(late_orderst.fnGetNodes()); var req = new XMLHttpRequest(); var url = '/cgi-bin/koha/acqui/lateorders.pl'; @@ -73,17 +74,25 @@ $(document).ready(function() { data.append("op","save"); for (var i = 0; i < all_nodes.length; i++) { var order = $(all_nodes[i]).find("input[name='ordernumber']"); - var internalnote = $(all_nodes[i]).find("input[name='internalnote']"); - var vendornote = $(all_nodes[i]).find("input[name='vendornote']"); - data.append(order[0].value + "_i",internalnote[0].value); - data.append(order[0].value + "_v",vendornote[0].value); + var internalnote = $(all_nodes[i]).find("input[name='internalnote']")[0].value; + var vendornote = $(all_nodes[i]).find("input[name='vendornote']")[0].value; + var internalnote_key = order[0].value + "_i"; + var vendornote_key = order[0].value + "_v"; + data.append(internalnote_key, internalnote); + data.append(vendornote_key, vendornote); } req.open("POST",url); req.send(data); req.onreadystatechange = function () { + $("#loading").remove(); if (req.readyState == '4' ){ - alert(_("all notes is modified correctly")); + if ( req.status == 200 ){ + alert(_("Notes successfully saved")); + } + else { + alert(_("Error while saving the notes. Please try again")); + } } } }); @@ -139,7 +148,7 @@ $(document).ready(function() { [% ELSE %] [% END %] - Order No. + Order line Order date Estimated delivery date Vendor -- 2.7.4