[Bug 12732] New: Sort late orders by basket creation or closing date
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Bug ID: 12732 Summary: Sort late orders by basket creation or closing date Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: simith.doliveira@inlibro.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |simith.doliveira@inlibro.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 --- Comment #1 from simith.doliveira@inlibro.com --- Created attachment 30592 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30592&action=edit Bug 12732 - Sort late orders by basket creation or closing date This fix permits to sort late orders by basket creation or closing date Modified: installer/data/mysql/sysprefs.sql -AcqLateOrderUseCreationDate preference added koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref -AcqLateOrderUseCreationDate preference added koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt -Change "Order date" column acqui/lateorders.pl - C4/Acquisition.pm -Change sql Testing: I Apply the patch II Run updatedatabase.pl 0) Create 2 orders each one with different baskets; 1) Change the first basket date (table aqbasket, column creationdate and closedate) to 2014-07-13; 2) Change the second basket creationdate to 2014-07-12 and closedate to 2014-07-14; 3) Set AcqLateOrderUseCreationDate to Don't use the creation date of the basket to sort late order; 4) Validate sort by "close date" (second basket came first) and "Order close date" column; 5) Set AcqLateOrderUseCreationDate to Do use the creation date of the basket to sort late order; 6) Validate sort by "creation date" (first basket came first) and "Order creation date" column; sponsored by the CCSR ( http://www.ccsr.qc.ca ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |eric.begin@inLibro.com, | |francois.charbonnier@inlibr | |o.com, | |philippe.blouin@inlibro.com Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |paola.rossi@cineca.it --- Comment #2 from Paola Rossi <paola.rossi@cineca.it> --- I've applied the patch against master 3.17.00.013 I pass the patch to "Signed Off" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30592|0 |1 is obsolete| | --- Comment #3 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 30794 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30794&action=edit Bug 12732 - Sort late orders by basket creation or closing date -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 30794 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30794 Bug 12732 - Sort late orders by basket creation or closing date Review of attachment 30794: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12732&attachment=30794) ----------------------------------------------------------------- Hi Simith, a couple of things: 0) The database update is missing from the patches. 1) Changes to core routines should be backed up by regression tests. ::: C4/Acquisition.pm @@ +2123,5 @@
my @query_params = (); my $select = " SELECT aqbasket.basketno, aqorders.ordernumber, + DATE(aqbasket.${aqbasketSortField}) AS orderdate,
2) I think the variable shouldn't be in the SQL, but be replaced by a placeholder. @@ +2162,4 @@
$select .= " aqorders.quantity - COALESCE(aqorders.quantityreceived,0) AS quantity, (aqorders.quantity - COALESCE(aqorders.quantityreceived,0)) * aqorders.rrp AS subtotal, + DATEDIFF(CAST(now() AS date),${aqbasketSortField}) AS latesince
Same here... @@ +2166,3 @@
"; if ( defined $delay ) { + $from .= " AND (${aqbasketSortField} <= DATE_SUB(CAST(now() AS date),INTERVAL ? DAY)) " ;
... and here... @@ +2182,3 @@
"; if ( defined $delay ) { + $from .= " AND (${aqbasketSortField} <= (CAST(now() AS date) -(INTERVAL ? DAY)) ";
... and here. ::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt @@ +111,4 @@
[% ELSE %] <th></th> [% END %] + <th class="title-string">Order [% lateOrderSort %] date</th>
3) This is a translation problem. Please avoid "Order [IFELSE construct] date" in favor of repeating the whole line for each possible setting - This will allow translators to create better translations. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref @@ +62,5 @@
+ choices: + creation: Do + closing: Don't + - Use the creation date of the basket to sort late order. +
4) Instead of a boolean, it would be nicer to rephrase a bit and use creation/closed date - Right now it is not clear at all what will be used instead of the creation date. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 --- Comment #5 from simith.doliveira@inlibro.com --- Hi Katrin, Concerning your comments: 0) Added in the new patch. 1) Ok for t/db_dependent/Acquisition.t and t/db_dependent/Acquisition. 2) I didn't know why, but using a placeholder in this code "DATEDIFF(CAST(now() AS date),${aqbasketSortField})" I got a different result, so I did not change it. 3) and 4) Changed in the new patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 --- Comment #6 from simith.doliveira@inlibro.com --- Created attachment 31064 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31064&action=edit Bug 12732 - Sort late orders by basket creation or closing date - This patch fixs the numbers 0, 3 and 4 in comment 4 Add: installer/data/mysql/updatedatabase.pl Modified: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref - AcqLateOrderUseCreationDate preference modified. koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt - translation problem fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to simith.doliveira from comment #5)
2) I didn't know why, but using a placeholder in this code "DATEDIFF(CAST(now() AS date),${aqbasketSortField})" I got a different result, so I did not change it.
I was not sure either so asked Jared to take a look - it won't work because it's a column name. So it's probably ok. I am switching back to 'signed off' treating the second patch as a QA follow-up. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Why don't you add the 2 infos in the table and let datatable does the sort job? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- The change in the subroutine needs unit tests. Switch to Failed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 --- Comment #10 from simith.doliveira@inlibro.com --- Created attachment 32298 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32298&action=edit updatedatabase.pl updated -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32298|1 |0 is patch| | Attachment #32298|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30794|0 |1 is obsolete| | --- Comment #11 from simith.doliveira@inlibro.com --- Created attachment 32301 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32301&action=edit Bug 12732 - Sort late orders by basket creation or closing date -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32301|1 |0 is patch| | Attachment #32301|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30592|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31064|0 |1 is obsolete| | --- Comment #12 from simith.doliveira@inlibro.com --- Created attachment 32303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32303&action=edit Bug 12732 - Sort late orders by basket creation or closing date - This patch fixs the numbers 0, 3 and 4 in comment 4 Add: installer/data/mysql/updatedatabase.pl Modified: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref - AcqLateOrderUseCreationDate preference modified. koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt - translation problem fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #13 from simith.doliveira@inlibro.com --- Sorry for the last 3 comments, I got some problems to fix merge conflict in installer/data/mysql/updatedatabase.pl - Regarding your question: Why don't you add the 2 infos in the table and let datatable does the sort job? The sponsor wanted just one column and the possibility to change the sort - The change in the subroutine needs unit tests. t/db_dependent/Acquisition.t alread has the tests for the subroutine that I changed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |kyle@bywatersolutions.com --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #8)
Why don't you add the 2 infos in the table and let datatable does the sort job?
I agree with this suggestion. Can you please address this? I feel like this is case of YASP when we could probably allow this default sorting to be changed via some javascript in intranetuserjs by splitting the data into separate table columns. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion --- Comment #15 from simith.doliveira@inlibro.com --- Kyle, This enhancement is sponsored by CCSR and that is the way they use. If the code is not suitable, so it must be followed up by someone else. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12732 Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org