Bug 6602 - Reports dictionary doesn't properly recognize text columns
Summary: Reports dictionary doesn't properly recognize text columns
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL: /cgi-bin/koha/reports/dictionary.pl
Keywords:
: 5148 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-07-18 19:36 UTC by Owen Leonard
Modified: 2013-05-23 06:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed fix (1.11 KB, patch)
2011-07-18 19:43 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6602 - Reports dictionary doesn't properly recognize text columns (1.23 KB, patch)
2011-08-14 14:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6602 - Reports dictionary doesn't properly recognize text columns (1.23 KB, patch)
2011-08-14 14:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6602 - Reports dictionary doesn't properly recognize text columns (1.15 KB, patch)
2012-11-20 18:55 UTC, Owen Leonard
Details | Diff | Splinter Review
[Signed off] Bug 6602 - Reports dictionary doesn't properly recognize text columns (1.19 KB, patch)
2012-11-27 01:15 UTC, Melia Meggs
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6602 - Reports dictionary doesn't properly recognize text columns (1.57 KB, patch)
2012-11-30 07:14 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2011-07-18 19:36:29 UTC
When defining a dictionary definition the script checks the type of the table column you have selected. If you've chosen a DATE column it displays date limiters, if you've chosen a VARCHAR column it shows a <select> of choices. If you choose a TEXT column you'll get a free text input to specify the comparison. However, the script doesn't check for MEDIUMTEXT. This means if you choose one of several MEDIUMTEXT columns you won't be able to input any string for comparison.
Comment 1 Owen Leonard 2011-07-18 19:43:25 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-07-18 19:59:06 UTC
The problem here is that this patch generates something like this:

AND borrowers.surname='en%'

If you enter a string with a wildcard - shouldn't the text strings be LIKE instead of equals?

This patch works in that it creates the rule, but I can't test the rule cause of the bug I reported that definitions aren't used in guided reports.
Comment 3 Owen Leonard 2011-07-18 20:01:50 UTC
This patch doesn't attempt to change anything about how SQL is generated. The patch only allows that text field to show up when it is required (in cases where the table column is TEXT or MEDIUMTEXT).
Comment 4 Nicole C. Engard 2011-07-18 20:03:29 UTC
Then it's only part of a fix ... or we need a note on the page that says not to enter wildcards and only enter exact matches.
Comment 5 Owen Leonard 2011-07-18 20:36:21 UTC
My patch is a whole fix for the problem I identified in this bug.
Comment 6 Katrin Fischer 2011-08-14 14:04:12 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2011-08-14 14:08:57 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2011-08-14 14:10:55 UTC
Signed-off, because the problem described in this bug is solved by Owen's patch. 

I filed bug 6719 for a problem I noticed with varchar(255).
I think the reports dictionary in general needs some work.
Comment 9 Owen Leonard 2011-08-23 19:51:32 UTC
*** Bug 5148 has been marked as a duplicate of this bug. ***
Comment 10 Chris Cormack 2011-08-24 23:51:00 UTC
Pushed, please test
Comment 11 Chris Nighswonger 2011-10-26 18:02:32 UTC
The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.
Comment 12 Owen Leonard 2012-11-20 18:51:35 UTC
This bug was reintroduced by the fix for Bug 7993 (I presume unintentionally).
Comment 13 Owen Leonard 2012-11-20 18:55:13 UTC Comment hidden (obsolete)
Comment 14 Melia Meggs 2012-11-27 01:15:56 UTC Comment hidden (obsolete)
Comment 15 Melia Meggs 2012-11-27 01:16:44 UTC
I had to add a patch complexity to sign this off.  Hopefully "small patch" is ok, but if not, someone please change it!
Comment 16 Katrin Fischer 2012-11-30 07:14:01 UTC
Created attachment 13798 [details] [review]
[SIGNED-OFF] Bug 6602 - Reports dictionary doesn't properly recognize text columns

Reports dictionary won't let you set a comparison for MEDIUMTEXT columns. To
test, choose a MEDIUMTEXT column like borrowers.surname in Step 3 of adding
a new dictionary definition. Before the patch, in Step 4 you would not see
any kind of form field for entering a comparison. After patching Step 4 will
offer you a field labeled "Search string matches."

Signed-off-by: Melia Meggs <melia@test.bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and perlcritic pass.

Test plan:
1) Reports > Reports dictionary > New definition
2) Step 1: Some name
3) Step 2: Patrons
4) Step 3: First name (mediumtext)
5) Step 4: Verify 'Search string matches' is offered.

For both, surname (mediumtext) and first name (text) the program should offer you
"Search string matches" on step 4.
Comment 17 Jared Camins-Esakov 2012-11-30 12:02:48 UTC
This patch has been pushed to master.
Comment 18 Chris Cormack 2012-12-02 06:20:12 UTC
Pushed to 3.10.x, will be in 3.10.1