datepicker's dateformat setting for reports is missing. Reports always run with ISO date format (yyyy-mm-dd) but this setting is missing in /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/LANGUAGE/modules/reports/guided_reports_start.tt. Because of this date format in datepicker plugin adopts the format defined in dateformat syspreferences variable and this is wrong. To solve this is needed to include this code below line 169: // redefine the datepicker's default because reports require ISO // http://jqueryui.com/demos/datepicker/#option-dateFormat $( ".datepicker" ).datepicker( "option", "dateFormat", "yy-mm-dd" ); Tested in 3.18.7 release. In 3.2 is not missing so it works fine.
Can you please include a step by step on how to make this bug visible in the GUI?
Created attachment 40127 [details] Step by step description I have attached a PDF file according your request.
Created attachment 40130 [details] guided_reports_start.tt Coded in 3.18.5.1
Created attachment 40157 [details] Step by step reloaded
I have checked feature is broken. Report are coming from SQL so need date in ISO format.
Created attachment 40357 [details] [review] Bug 14382 - datepicker's dateFormat setting for reports is missing Datepicker's dateFormat setting for reports is missing Report require date in ISO format(yyyy-mm-dd). Test Plan 1. Go to Report module 2. Create report from option (Create report from SQL). 3. Write the sql with date as input filter. 4. Choose the date (Now come in ISO format (yyyy-mm-dd). 5. Run the report.
Created attachment 40358 [details] [review] Bug 14382 - datepicker's dateFormat setting for reports is missing Datepicker's dateFormat setting for reports is missing Report require date in ISO format(yyyy-mm-dd). Test Plan 1. Go to Report module 2. Create report from option (Create report from SQL). 3. Write the sql with date as input filter. 4. Choose the date (Now date came in ISO format (yyyy-mm-dd). 5. Run the report.
When I select a value using the datepicker plugin, the input is filled with "20062006-05-31". Don't understand why the year is duplicated. Anyway, I think the date should be displayed according to the pref. If we need to do some manipulation with it, we can do it in the pl script.
Created attachment 40360 [details] [review] Bug 14382 - datepicker's dateFormat setting for reports is missing Datepicker's dateFormat setting for reports is missing Report require date in ISO format(yyyy-mm-dd). Test Plan 1. Go to Report module 2. Create report from option (Create report from SQL). 3. Write the sql with date as input filter. 4. Choose the date (Now date came in ISO format (yyyy-mm-dd). 5. Run the report.
Hi Jonathan Druart, I have fixed it. Please check again.
(In reply to Jonathan Druart from comment #8) > When I select a value using the datepicker plugin, the input is filled with > "20062006-05-31". Don't understand why the year is duplicated. Amit fixed this.... HOWEVER... > Anyway, I think the date should be displayed according to the pref. If we > need to do some manipulation with it, we can do it in the pl script. I agree with Jonathan on this. I will probably look at the technicalities of implementation tomorrow.
Created attachment 40593 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. The issue is that SQL expects ISO dates, but the user may wish to view dates according to the dateformat system preference. By detecting a date preference, the non-ISO dates can be converted to ISO dates before being stuffed back into the SQL query to be executed. TEST PLAN --------- 1) Add a report with date parameters. -- I used 'Holds placed in date range' from http://wiki.koha-community.org/wiki/SQL_Reports_Library 2) Set your dateformat to YYYY-MM-DD 3) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in YYYY-MM-DD}'..." -- If there is supposed to be data, there is some. 4) Set your dateformat to MM/DD/YYYY 5) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in MM/DD/YYYY}'..." -- If there is supposed to be data, there is none. 6) Apply patch 7) Repeat steps 2-5 -- The SQL will always read YYYY-MM-DD (ISO) format. -- The report will have data, if there is some. 8) koha qa test tools.
Created attachment 40594 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. The issue is that SQL expects ISO dates, but the user may wish to view dates according to the dateformat system preference. By detecting a date preference, the non-ISO dates can be converted to ISO dates before being stuffed back into the SQL query to be executed. TEST PLAN --------- 1) Add a report with date parameters. -- I used 'Holds placed in date range' from http://wiki.koha-community.org/wiki/SQL_Reports_Library 2) Set your dateformat to YYYY-MM-DD 3) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in YYYY-MM-DD}'..." -- If there is supposed to be data, there is some. 4) Set your dateformat to MM/DD/YYYY 5) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in MM/DD/YYYY}'..." -- If there is supposed to be data, there is none. 6) Apply patch 7) Repeat steps 2-5 -- The SQL will always read YYYY-MM-DD (ISO) format. -- The report will have data, if there is some. 8) koha qa test tools. Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Created attachment 40611 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. Works perfectly.. Bug 14382: Non-ISO Date parameters generate empty reports. The issue is that SQL expects ISO dates, but the user may wish to view dates according to the dateformat system preference. By detecting a date preference, the non-ISO dates can be converted to ISO dates before being stuffed back into the SQL query to be executed. TEST PLAN --------- 1) Add a report with date parameters. -- I used 'Holds placed in date range' from http://wiki.koha-community.org/wiki/SQL_Reports_Library 2) Set your dateformat to YYYY-MM-DD 3) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in YYYY-MM-DD}'..." -- If there is supposed to be data, there is some. 4) Set your dateformat to MM/DD/YYYY 5) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in MM/DD/YYYY}'..." -- If there is supposed to be data, there is none. 6) Apply patch 7) Repeat steps 2-5 -- The SQL will always read YYYY-MM-DD (ISO) format. -- The report will have data, if there is some. 8) koha qa test tools. Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com>
Comment on attachment 40611 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. Review of attachment 40611 [details] [review]: ----------------------------------------------------------------- ::: reports/guided_reports.pl @@ +755,4 @@ > # if there are special regexp chars, we must \ them > $split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; > + if ($split[$i*2+1] =~ /|date>>/) { > + $quoted = substr(dt_from_string($quoted),0,10); I blame Amit Gupta for showing me this mistake. output_pref({ dt => (dt_from_string() + $days), dateformat => 'iso', dateonly => 1 }); I should have used output_pref!
Created attachment 40684 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. The issue is that SQL expects ISO dates, but the user may wish to view dates according to the dateformat system preference. By detecting a date preference, the non-ISO dates can be converted to ISO dates before being stuffed back into the SQL query to be executed. TEST PLAN --------- 1) Add a report with date parameters. -- I used 'Holds placed in date range' from http://wiki.koha-community.org/wiki/SQL_Reports_Library 2) Set your dateformat to YYYY-MM-DD 3) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in YYYY-MM-DD}'..." -- If there is supposed to be data, there is some. 4) Set your dateformat to MM/DD/YYYY 5) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in MM/DD/YYYY}'..." -- If there is supposed to be data, there is none. 6) Apply patch 7) Repeat steps 2-5 -- The SQL will always read YYYY-MM-DD (ISO) format. -- The report will have data, if there is some. 8) koha qa test tools.
Created attachment 40691 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. Work perfectly!!
Created attachment 40776 [details] [review] Bug 14382: Non-ISO Date parameters generate empty reports. The issue is that SQL expects ISO dates, but the user may wish to view dates according to the dateformat system preference. By detecting a date preference, the non-ISO dates can be converted to ISO dates before being stuffed back into the SQL query to be executed. TEST PLAN --------- 1) Add a report with date parameters. -- I used 'Holds placed in date range' from http://wiki.koha-community.org/wiki/SQL_Reports_Library 2) Set your dateformat to YYYY-MM-DD 3) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in YYYY-MM-DD}'..." -- If there is supposed to be data, there is some. 4) Set your dateformat to MM/DD/YYYY 5) Run the report -- Note the SQL reads "... BETWEEN '{date formatted in MM/DD/YYYY}'..." -- If there is supposed to be data, there is none. 6) Apply patch 7) Repeat steps 2-5 -- The SQL will always read YYYY-MM-DD (ISO) format. -- The report will have data, if there is some. 8) koha qa test tools. Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Adding a QA follow-up.
Created attachment 40777 [details] [review] Bug 14382: [QA Follow-up] Bad regex alarm The regex /|date>>/ will match much more than you like :) The unescaped pipe is bad, but you also need to remove the >> because the split a few lines above it removes them already. This allows you to recover from an error like this one, running another report with a string parameter: The given date (india%) does not match the date format (us) at Koha/DateUtils.pm line 144. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
$a= "<<Between (yyyy-mm-dd)|date>>"; $b= "test"; if ($a =~ /|date>>/) { print "a true"; } if ($b =~ /|date>>/) { print "b true"; }
Comment on attachment 40777 [details] [review] Bug 14382: [QA Follow-up] Bad regex alarm Review of attachment 40777 [details] [review]: ----------------------------------------------------------------- ::: reports/guided_reports.pl @@ +754,4 @@ > my $quoted = $sql_params[$i]; > # if there are special regexp chars, we must \ them > $split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; > + if ($split[$i*2+1] =~ /\|date$/) { While I agree with the escaping (I really should have caught that, DOH!), the $ does not work. Why not leave >>? >'s are not special characters, are they? I was thinking it may be better to put: /\|\s*date\s*>>/
(In reply to M. Tompsett from comment #21) > Comment on attachment 40777 [details] [review] [review] > Bug 14382: [QA Follow-up] Bad regex alarm > > Review of attachment 40777 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: reports/guided_reports.pl > @@ +754,4 @@ > > my $quoted = $sql_params[$i]; > > # if there are special regexp chars, we must \ them > > $split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; > > + if ($split[$i*2+1] =~ /\|date$/) { > > While I agree with the escaping (I really should have caught that, DOH!), > the $ does not work. Why not leave >>? >'s are not special characters, are > they? > I was thinking it may be better to put: > /\|\s*date\s*>>/ Please explain why the $ should not work. I agree btw that \s* could be put in additionally. But the >> should be removed, because of the split /<<|>>/,$sql. When I tested this, it worked.
I wrote: >> I was thinking it may be better to put: >> /\|\s*date\s*>>/ Marcel wrote: > Please explain why the $ should not work. ... > But the >> should be removed, because of the split > /<<|>>/,$sql. DOH! Larger code context. You are right. > I agree btw that \s* could be put > in additionally. > When I tested this, it worked. So we agree that: /\|\s*date\s*$/ would be better. :)
Created attachment 40781 [details] [review] Bug 14382: [QA Follow-up] Bad regex alarm The regex /|date>>/ will match much more than you like :) The unescaped pipe is bad, but you also need to remove the >> because the split a few lines above it removes them already. This allows you to recover from an error like this one, running another report with a string parameter: The given date (india%) does not match the date format (us) at Koha/DateUtils.pm line 144. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended for possible spaces around the word date.
Patches pushed to master. Thanks Mark and Marcel!
Pushed to 3.20.x will be in 3.20.2
Pushed to 3.18.x will be in 3.18.09
*** Bug 14630 has been marked as a duplicate of this bug. ***
Pushed to 3.16.x, will be in 3.16.15