Bug 12127 - Incorrect DataTable types in Holds to Pull list
Summary: Incorrect DataTable types in Holds to Pull list
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 12088
  Show dependency treegraph
 
Reported: 2014-04-23 05:48 UTC by David Cook
Modified: 2014-12-07 20:07 UTC (History)
2 users (show)

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


Attachments
Bug 12127 - Incorrect DataTable types in Holds to Pull list (2.55 KB, patch)
2014-04-23 06:46 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12127 - Incorrect DataTable types in Holds to Pull list (2.63 KB, patch)
2014-04-23 13:53 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 12127 - Incorrect DataTable types in Holds to Pull list (2.66 KB, patch)
2014-04-25 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2014-04-23 05:48:05 UTC
At the moment, it appears that the branch filtering isn't working correctly in the Holds to Pull list. That is, it is being too restrictive rather than too fuzzy (bug 11502). It will only show rows that have a single branch when filtering. It won't show rows that have multiple available branches.

I would swear that this wasn't happening earlier in the year. I repeatedly tested the patch on bug 11502 and it went through the QA process without a problem.

I would also swear that the DataTables JS hasn't changed in the past few months. I would swear this because I'm using the exact same code on my machine but the filtering isn't working correctly. The only thing that seems to remotely make sense to me is perhaps a change in the browsers' internal JS processing, but I have no idea really...

In any case, the immediate cause appears to be that DataTables is typing the column data as "html" rather than "string", so it is stripping out the <br> tags and creating one long string of branchcodes on which to search.  

This is a problem after the implementation of bug 11502 which uses the angle brackets from the <br> tags as a separator in regex to make the filtering more precise.
Comment 1 David Cook 2014-04-23 06:46:02 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2014-04-23 13:53:20 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2014-04-25 11:37:27 UTC
Created attachment 27609 [details] [review]
[PASSED QA] Bug 12127 - Incorrect DataTable types in Holds to Pull list

This patch explicitly sets the sType in the Holds to Pull list table
for the libraries, itypes, and locations columns. It appears that the
type is automatically being set to "html" when it should be set to
"string".

It needs to have a sType of "string", since DataTables will strip out
the <br> tags when using "html", and we need them for the regexp
we use to create a more precise filter search.

_TEST PLAN_

Before applying:

1) Place a "next available" hold on a record with an item available
from one library/branch (e.g. "Branch A")
2) Place a "next available" hold on a record with an item available
from two different libraries/branches
3) Place a "next available" hold on a record with an item available
from three different libraries/branches - one of which is the same
as the library/branch from Step 1.
4) Go to the Holds to Pull List
5) Filter the branches by "Branch A".
6) You should only see the hold from Step 1 - not Step 3.
7) Try filtering on a branch for a hold that has multiple
libraries/branches. (They shouldn't appear when filtering, except
when using the "None" filter).

Apply the patch.

After applying:
1) Repeat Steps 4-7 from above. When filtering on "Branch A", you
should be able to see all holds that are available at "Branch A",
even if they are available at other branches (i.e. the holds has
multiple libraries listed).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2014-04-25 11:38:29 UTC
It would be nice if this patch used classes right from the start, but it's still better than having a bug!
Comment 5 Owen Leonard 2014-04-25 12:43:21 UTC
(In reply to Kyle M Hall from comment #4)
> It would be nice if this patch used classes right from the start

My patch for Bug 12088 corrects this.
Comment 6 Galen Charlton 2014-04-28 17:37:27 UTC
Pushed to master.  Thanks, David!