| Summary: | Remove dead code in datatables.js | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
| Component: | Architecture, internals, and plumbing | Assignee: | 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: | ||
| Circulation function: | |||
| 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
Created attachment 46330 [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. Created attachment 46331 [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> 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? 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 :) 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> 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? Should we revert this one? (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 ? > 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) Will definitely look at this again - thanks. (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 :) (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 :) |