Bugzilla – Attachment 72170 Details for
Bug 12502
Add columns for note, order number and ISBN to late orders page
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12502: QA Follow-up
Bug-12502-QA-Follow-up.patch (text/plain), 9.22 KB, created by
Charles Farmer
on 2018-02-23 22:53:34 UTC
(
hide
)
Description:
Bug 12502: QA Follow-up
Filename:
MIME Type:
Creator:
Charles Farmer
Created:
2018-02-23 22:53:34 UTC
Size:
9.22 KB
patch
obsolete
>From 13c8557e4bc515a6f54e387506f0943eb8434f1d Mon Sep 17 00:00:00 2001 >From: Charles Farmer <charles.farmer@inLibro.com> >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("<span id=\"loading\" style=\"margin:4px;\"><img src=\"[% interface %]/[% theme %]/img/loading-small.gif\" alt=\"\" /> "+_("Loading")+"</span>"); > 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 %] > <th></th> > [% END %] >- <th>Order No.</th> >+ <th>Order line</th> > <th class="title-string">Order date</th> > <th class="title-string">Estimated delivery date</th> > <th>Vendor</th> >-- >2.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=72170">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 12502
:
67349
|
67354
|
67586
|
67587
|
72168
|
72169
|
72170
|
73499
|
73500
|
73501
|
73502
|
73503
|
79231
|
80160
|
80602
|
87236
|
91564
|
95418
|
95419
|
95438
|
95439
|
95440
|
95441
|
97427
|
97428
|
97429
|
97430