[Bug 11253] New: tools/inventory.pl output_pref ignores 'iso' parameter passed
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Bug ID: 11253 Summary: tools/inventory.pl output_pref ignores 'iso' parameter passed Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org I found this as a result of looking briefly at bug 11240. When the parameter was changed to a hash reference in bug 10380, I believe tools/inventory.pl was missed. It currently allowed for a non-hash to be passed, but assumes that is just the date. Any other parameters would be ignored. I am unsure of the correct course of action, whether the parameter needs to be passed, or whether the system preference is the preferred result. I'm looking further into this. I figure this is minor, since most people probably don't change the dateformat system preference to non-iso, and don't run tools/inventory.pl frequently. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Mark, I am not sure I understand from your description how this bug is visible in tools, could you add a test plan? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- tools/inventory.pl passes two parameters a DateTime and 'iso'. But output_pref in Koha/DateUtils.pm only pays attention to the first parameter for non-hash parameters. This means the results would be affected by the I18N/L10N preferences dateformat system preference. 1) Should it be affected by the system preference like that? OR 2) Should it always be iso regardless of the system preference? NOTE: 'it' being the output of the clicking submit on the tools inventory page, as far as I can tell. If (1), then 'iso' should be chopped from tools/inventory.pl. If (2), then the second parameter should be checked in Koha::DateUtils. As an aside, I think I found another bug while trying to create a sample test plan. Could someone else confirm that going to the inventory tool page and just clicking submit without actually uploading any barcodes generates a 500 error? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Create a small barcode file from books in a given branch. Set system preference of dateformat to mm/dd/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set system preference of dateformat to yyyy-mm-dd. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays nothing in "Unseen since" column. Set system preference of dateformat to dd/mm/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit KABOOM! I would expect to see dates all the time. I would also expect that it is formatted either (a) according to the system preference, or (b) YYYY-MM-DD. But generally, the second parameter which is supposed to be setting 'iso' is ignored completely, and this weirdness ensues. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23179 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23179&action=edit Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Took all the output_pref function calls and converted them to the newer hash format parameter. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23179|0 |1 is obsolete| | --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23180 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23180&action=edit Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Took all the output_pref function calls and converted them to the newer hash format parameter. Added special case for dd-mm-yyyy calls, so dt_from_string works correctly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23180|0 |1 is obsolete| | --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 23876 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23876&action=edit Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Took all the output_pref function calls and converted them to the newer hash format parameter. Added special case for dd-mm-yyyy calls, so dt_from_string works correctly. TEST PLAN --------- Create a small barcode file from books in a given branch. Set I18N/L10N system preference of dateformat to mm/dd/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to yyyy-mm-dd. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to dd/mm/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit KABOOM! Apply patch Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 wajasu <matted-34813@mypacks.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23876|0 |1 is obsolete| | CC| |matted-34813@mypacks.net --- Comment #7 from wajasu <matted-34813@mypacks.net> --- Created attachment 25679 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25679&action=edit [SIGNED-OFF] Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Created a SQL report select barcode from items; my set was small. Ran it and downloaded Then ran test plan before and after patch, seeing desired results. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 wajasu <matted-34813@mypacks.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 25679 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25679 [SIGNED-OFF] Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Review of attachment 25679: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11253&attachment=25679) ----------------------------------------------------------------- ::: tools/inventory.pl @@ +150,5 @@
my @errorloop; if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { my $dbh = C4::Context->dbh; + my $date; + if (C4::Context->preference('dateformat') eq 'dd/mm/yyyy') {
This test is done in dt_from_string. You should not check the dateformat pref here. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff 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=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25679|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 26437 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26437&action=edit Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Took all the output_pref function calls and converted them to the newer hash format parameter. This then gave blank output. Looking at the template file, I decided that the html filter should be after the KohaDates filter. This corrected the display of the date for the dd/mm/yyyy case. TEST PLAN --------- Create a small barcode file from books in a given branch. Set I18N/L10N system preference of dateformat to mm/dd/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to yyyy-mm-dd. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to dd/mm/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit KABOOM! Apply patch Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26437|0 |1 is obsolete| | --- Comment #10 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 27081 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27081&action=edit Bug 11253 - tools/inventory.pl output_pref ignores 'iso' parameter passed Took all the output_pref function calls and converted them to the newer hash format parameter. This then gave blank output. Looking at the template file, I decided that the html filter should be after the KohaDates filter. This corrected the display of the date for the dd/mm/yyyy case. TEST PLAN --------- Create a small barcode file from books in a given branch. Set I18N/L10N system preference of dateformat to mm/dd/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to yyyy-mm-dd. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Set I18N/L10N system preference of dateformat to dd/mm/yyyy. Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit KABOOM! Apply patch Go to inventory/stocktaking tool. Browse for the barcode file. Select the branch (i.e. not All libraries) to match. Click submit Table displays dates in "Unseen since" column. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Before the patch, I had this error when using dateformat=ddmmyy: Template process failed: undef error - The 'day' parameter ("2014") to DateTime::new did not pass the 'an integer which is a possible valid day of month' callback at /usr/local/share/perl5/Params/Validate/PP.pm line 633. This patch corrects the problem. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Why the last past does not change the call to output_pref for the $cdls variable? It made sense! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Mark, please take a look at Jonathan's question in comment 11. As Marcel signed off on the existing patch and I can find no wrong with it, I am going to pass. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27081|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 27317 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27317&action=edit 0001-PASSED-QA-Bug-11253-tools-inventory.pl-output_pref-i.patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #14 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Mark! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #11)
Why the last past does not change the call to output_pref for the $cdls variable? It made sense!
Ah, I was looking at the line that caused the explosion. :) There are two lines, much later in the code, which could be tweaked. However, their disrespect for the iso parameter seems irrelevant, as dd/mm/yyyy didn't blow up (clicking submit button in the barcode section after choosing a file). I suspect that $_->{datelastseen} is in SQL format (YYYY-MM-DD) for the $cdls, and the first one's disrespect still generates a valid date for $dls. I think there is merit for a follow up if one wants truly nice code (i.e. not working by accident). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 --- Comment #16 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to M. Tompsett from comment #15)
I think there is merit for a follow up if one wants truly nice code (i.e. not working by accident).
To answer the question you posed in IRC, please open a new bug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12115 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11253 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #17 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Pushed to 3.14.x, will be in 3.14.07 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org