Bug 15481

Summary: Remove dead code in datatables.js
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, jonathan.druart, m.de.rooy, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15916
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 15481: Remove dead code in datatables.js
Bug 15481: Remove dead code in datatables.js
Bug 15481: Remove dead code in datatables.js

Description Julian Maurice 2016-01-06 14:35:50 UTC
Remove dead code in datatables.js
Comment 1 Julian Maurice 2016-01-06 14:36:28 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2016-01-06 14:45:31 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-01-07 12:17:04 UTC
Julian,
IIRC, these functions were originally used by an alternative patch of bug 5342, never pushed (see attachment 8910 [details] [review], Followup : DataTables server-side processing).
Are there no plan to rebase these changes?
Comment 4 Julian Maurice 2016-01-07 21:18:13 UTC
I'm not sure if there are plans to rebase this patch, but in this case I'm sure it could be rewritten differently (i.e. better) without these functions. And let's face it, replace_html is such a bad name for a JS function :)
Comment 5 Jonathan Druart 2016-01-08 10:24:43 UTC
Created attachment 46417 [details] [review]
Bug 15481: Remove dead code in datatables.js

This removes the following functions that are not used anywhere:
- dt_add_rangedate_filter
- dt_overwrite_string_sorting_localeCompare
- replace_html
- replace_html_date

Test plan:
1. Search for calls to the above functions in the code. There should be
   none.
2. Check that existing tables using DataTables are not broken.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Marcel de Rooy 2016-02-26 07:44:13 UTC
I am having the impression that some of this dead code was still used.
And this patch broke some datatables functionality.
For instance, look at some tables in Administration: Libraries, Item types.
Where is the search bar?
Comment 8 Marcel de Rooy 2016-02-26 07:46:17 UTC
Should we revert this one?
Comment 9 Julian Maurice 2016-02-26 08:34:22 UTC
(In reply to Marcel de Rooy from comment #7)
> I am having the impression that some of this dead code was still used.
> And this patch broke some datatables functionality.
> For instance, look at some tables in Administration: Libraries, Item types.
> Where is the search bar?

(In reply to Marcel de Rooy from comment #8)
> Should we revert this one?

If you revert it, does the search bar reappear ?
Comment 10 Owen Leonard 2016-02-26 14:01:36 UTC
> For instance, look at some tables in Administration: Libraries, Item types.
> Where is the search bar?

Running git bisect to diagnose a similar problem with the holds to pull report led to Bug 15916 - Regression: Many tables' sorting broken by JavaScript error. Perhaps the same issue?(In reply to Marcel de Rooy from comment #7)
Comment 11 Brendan Gallagher 2016-02-27 15:32:36 UTC
Will definitely look at this again - thanks.
Comment 12 Marcel de Rooy 2016-02-29 09:02:13 UTC
(In reply to Marcel de Rooy from comment #8)
> Should we revert this one?

No. Bug 15916 solved the issue. So this patch was fine after all :)
Comment 13 Brendan Gallagher 2016-02-29 15:18:15 UTC
(In reply to Marcel de Rooy from comment #12)
> (In reply to Marcel de Rooy from comment #8)
> > Should we revert this one?
> 
> No. Bug 15916 solved the issue. So this patch was fine after all :)

Awesome :)