Bug 18961 - Datatable column filters of style 'select' should do an exact match
Summary: Datatable column filters of style 'select' should do an exact match
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-20 07:55 UTC by Marc Véron
Modified: 2017-10-20 20:16 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18961: Use exact match for select filters on item search (1.55 KB, patch)
2017-10-10 15:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18961: Use exact match for select filters on item search (1.71 KB, patch)
2017-10-11 07:58 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18961: (alternate) Datatable column filters use exact match by default (3.40 KB, patch)
2017-10-13 13:08 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 18961: Use exact match for select filters on item search (1.80 KB, patch)
2017-10-18 14:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2017-07-20 07:55:49 UTC
Datatable column filters of type select do not do an exact match. E.g. for a select with values 'Fiction' and 'Non-fiction', filtering after 'Fiction' will match both.

To reproduce:
- Apply patch of Bug 16485 - Collection column in Item search is always empty 
- Make sure you have collection codes 'Fiction' and 'Non-fiction'
- Do an item search
- Filter column 'Collection', select 'Fiction'
- Result: Column contains items of both collections (Fiction and Non-fiction)

Similar problems can happen for other column filters of type select as well, see:
jQuery DataTable ColumnFilter plugin. Can the “select” filter style support exact match?
https://stackoverflow.com/questions/9520423/jquery-datatable-columnfilter-plugin-can-the-select-filter-style-support-exac
Comment 1 Jonathan Druart 2017-10-10 15:21:47 UTC
Created attachment 67883 [details] [review]
Bug 18961: Use exact match for select filters on item search

The SQL operator LIKE is always used for filters when searching for
items.

If the filter is a select, we should search for an exact match.
That way we avoid problematic search like "%NFIC%" and "%FIC%" (one
includes the other one).

Test plan:
- Make sure you have collection codes 'Fiction' and 'Non-fiction'
- Do an item search
- Filter column 'Collection', select 'Fiction'
- Result: Column contains items from Fiction only
Comment 2 Marc Véron 2017-10-11 07:58:45 UTC
Created attachment 67924 [details] [review]
Bug 18961: Use exact match for select filters on item search

The SQL operator LIKE is always used for filters when searching for
items.

If the filter is a select, we should search for an exact match.
That way we avoid problematic search like "%NFIC%" and "%FIC%" (one
includes the other one).

Test plan:
- Make sure you have collection codes 'Fiction' and 'Non-fiction'
- Do an item search
- Filter column 'Collection', select 'Fiction'
- Result: Column contains items from Fiction only

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 3 Marcel de Rooy 2017-10-13 10:19:48 UTC
if ( grep /^$columns[$i]$/, qw( ccode homebranch holdingbranch location notforloan ) ) {

How do you know for sure that this comes from a select ? And that another value is not ? Note that we can choose to limit values by authorized values etc.
Comment 4 Fridolin SOMERS 2017-10-13 12:38:53 UTC
(In reply to Marcel de Rooy from comment #3)
> if ( grep /^$columns[$i]$/, qw( ccode homebranch holdingbranch location
> notforloan ) ) {
> 
> How do you know for sure that this comes from a select ? And that another
> value is not ? Note that we can choose to limit values by authorized values
> etc.

Indeed.
We had this same problem.

That is why I propose that all filters would be exact match in items search, SQL wildcard characters can be used to search like before : %text%.

This is the way to search in items search form so I think it is logical.

I will propose an alternate patch.
Comment 5 Fridolin SOMERS 2017-10-13 13:08:09 UTC
Created attachment 68072 [details] [review]
Bug 18961: (alternate) Datatable column filters use exact match by default

Datatable column filters of type select do not do an exact match. E.g. for a select with values 'Fiction' and 'Non-fiction', filtering after 'Fiction' will match both.

Similar problems can happen for other column filters of type select as well, see:
jQuery DataTable ColumnFilter plugin. Can the 'select' filter style support exact match?
https://stackoverflow.com/questions/9520423/jquery-datatable-columnfilter-plugin-can-the-select-filter-style-support-exac

This patch removes the fact that search text in colum filter behaves as 'contains', match is exact by default so that filter with select are good.
For text filters, one can always use whilcards characters like in items search form.
Patch adds an hint text after search results.

Test plan :
- Apply patch of Bug 16485 - Collection column in Item search is always empty
- Make sure you have collection codes 'Fiction' and 'Non-fiction'
- Do an item search
- Filter column 'Collection', select 'Fiction'
- Result: Column contains items of both collections (Fiction and Non-fiction)
- Choose a barcode from results, for example 123456
- Search on barcode with '3' of this barcode, you get no result
- Search on barcode with '%3%', you get the barcode in results
Comment 6 Jonathan Druart 2017-10-13 15:44:49 UTC
(In reply to Marcel de Rooy from comment #3)
> if ( grep /^$columns[$i]$/, qw( ccode homebranch holdingbranch location
> notforloan ) ) {
> 
> How do you know for sure that this comes from a select ? And that another
> value is not ? Note that we can choose to limit values by authorized values
> etc.

It's the column headers: https://screenshots.firefox.com/NypKJESICLDt95Vi/pro.kohadev.org
I am not sure I understand this concern.
Comment 7 Katrin Fischer 2017-10-13 22:25:38 UTC
I am not sure about making this change for the text input fields as well. The truncated search behaviour is often helpful there.
Comment 8 Marcel de Rooy 2017-10-16 06:46:58 UTC
(In reply to Jonathan Druart from comment #6)
> (In reply to Marcel de Rooy from comment #3)
> > if ( grep /^$columns[$i]$/, qw( ccode homebranch holdingbranch location
> > notforloan ) ) {
> > 
> > How do you know for sure that this comes from a select ? And that another
> > value is not ? Note that we can choose to limit values by authorized values
> > etc.
> 
> It's the column headers:
> https://screenshots.firefox.com/NypKJESICLDt95Vi/pro.kohadev.org
> I am not sure I understand this concern.

No, this is about filters passed to SearchItems.
Comment 9 Marcel de Rooy 2017-10-16 06:47:18 UTC
(In reply to Katrin Fischer from comment #7)
> I am not sure about making this change for the text input fields as well.
> The truncated search behaviour is often helpful there.

I would agree.
Comment 10 Jonathan Druart 2017-10-16 13:31:55 UTC
I suggest to push my patch and assume that only these 5 columns are linked to a AV. It's not perfect but I do not think it worth the effort.
Comment 11 Marc Véron 2017-10-16 16:50:11 UTC
(In reply to Jonathan Druart from comment #10)
> I suggest to push my patch and assume that only these 5 columns are linked
> to a AV. It's not perfect but I do not think it worth the effort.

+1
Marc
Comment 12 Jonathan Druart 2017-10-18 13:18:21 UTC
Back to QA
Comment 13 Fridolin SOMERS 2017-10-18 13:46:10 UTC
(In reply to Jonathan Druart from comment #10)
> I suggest to push my patch and assume that only these 5 columns are linked
> to a AV. It's not perfect but I do not think it worth the effort.

Ok, its better to have a fast solution than a long one.

I'm keeping the idea in mind.
Comment 14 Marcel de Rooy 2017-10-18 14:52:12 UTC
Created attachment 68258 [details] [review]
Bug 18961: Use exact match for select filters on item search

The SQL operator LIKE is always used for filters when searching for
items.

If the filter is a select, we should search for an exact match.
That way we avoid problematic search like "%NFIC%" and "%FIC%" (one
includes the other one).

Test plan:
- Make sure you have collection codes 'Fiction' and 'Non-fiction'
- Do an item search
- Filter column 'Collection', select 'Fiction'
- Result: Column contains items from Fiction only

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2017-10-18 14:52:31 UTC
Passing QA with reference to the discussion above.
Comment 16 Jonathan Druart 2017-10-20 20:16:21 UTC
Pushed to master for 17.11, thanks to everybody involved!