Bug 9463 - Use DataTables on patron sent notices page
Summary: Use DataTables on patron sent notices page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Jonathan Druart
URL: /cgi-bin/koha/members/notices.pl?borr...
Keywords:
Depends on: 9887
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-23 17:25 UTC by Owen Leonard
Modified: 2014-05-26 21:04 UTC (History)
3 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
Bug 9463 - Use DataTables on patron sent notices page (2.12 KB, patch)
2013-01-23 17:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9463 - Use DataTables on patron sent notices page (2.14 KB, patch)
2013-01-24 10:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9463 - Use DataTables on patron sent notices page (2.14 KB, patch)
2013-01-28 08:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9463 - Use DataTables on patron sent notices page (2.11 KB, patch)
2013-02-26 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
screenshot (45.87 KB, image/png)
2013-03-31 08:49 UTC, Katrin Fischer
Details
Bug 9463 [Revised] Use DataTables on patron sent notices page (2.77 KB, patch)
2013-04-01 14:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9463: FIX DT sort by date for the last column (1.69 KB, patch)
2013-04-02 07:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9463 [Revised] Use DataTables on patron sent notices page (2.79 KB, patch)
2013-04-07 03:10 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9463: FIX DT sort by date for the last column (1.74 KB, patch)
2013-04-07 03:10 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9463 [Revised] Use DataTables on patron sent notices page (4.71 KB, patch)
2013-04-08 12:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9463 [Revised] Use DataTables on patron sent notices page (4.72 KB, patch)
2013-04-12 15:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9463 [Revised] Use DataTables on patron sent notices page (4.79 KB, patch)
2013-04-15 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2013-01-23 17:25:22 UTC
The patron sent notices page uses the old tablesorter plugin. It should use the DataTables plugin instead.
Comment 1 Owen Leonard 2013-01-23 17:28:21 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-01-24 10:48:17 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-01-24 10:49:30 UTC
I found an issue (not introduced with your patch) : the time column is not displayed in the syspref format. I opened a new bug 9479
Comment 4 Jonathan Druart 2013-01-28 08:10:03 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-02-17 15:28:24 UTC
Patch no longer applies - please rebase and switch back to signed off if the conflict is easy to resolve.
Comment 6 Owen Leonard 2013-02-26 14:36:20 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2013-03-31 08:49:07 UTC
Created attachment 17081 [details]
screenshot

Hi Owen,
a recent patch fixed date sorting on this page and I think this patch breaks it again (see screenshot). Sorting on master is correct, sorting with patch is not. Can you take a look please?
Comment 8 Owen Leonard 2013-04-01 14:45:22 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2013-04-02 07:39:56 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2013-04-02 07:40:35 UTC
Owen, I got a js error with your patch:
  TypeError: y[(("string" + "-") + m[Q][1])] is not a function

I fix it with 3 simple changes, I use uk_date for the Time column and it seems to be well sorted.
Comment 11 Owen Leonard 2013-04-02 09:54:27 UTC
Jonathan did you apply the patch for Bug 9887 first? I don't mind solving this sort with the uk_date routine, but I've found the title attribute sort to be really easy to implement, especially on pages where a lot of logic is going into displaying columns or not.
Comment 12 Jonathan Druart 2013-04-02 11:55:36 UTC
(In reply to comment #11)
> Jonathan did you apply the patch for Bug 9887 first? I don't mind solving
> this sort with the uk_date routine, but I've found the title attribute sort
> to be really easy to implement, especially on pages where a lot of logic is
> going into displaying columns or not.

No, I forgot to apply the patch for bug 9887! And now it works.

This means all dates in DT tables should be replaced using this method?
We should standardized the way to sort dates and your patch add a new one. The goal is to delete the uk_date js functions and add a span around td values?
Comment 13 Chris Cormack 2013-04-03 09:22:24 UTC
Would like to sign off, but its currently blocked by bug 9887 which is in failed qa
Comment 14 Chris Cormack 2013-04-07 03:10:28 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2013-04-07 03:10:45 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2013-04-07 21:06:05 UTC
On a patron with an empty notices table an alert pops up:
DataTables warning (table id = 'noticestable'): Requested unknown parameter '1' from the data source for row 0
Comment 17 Owen Leonard 2013-04-08 12:48:09 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2013-04-12 15:33:34 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2013-04-15 09:10:30 UTC
QA Comment:
This patch replaces tablesorter with DataTables on the patron sent notices page.
All works great.
Marked as Passed QA.
Comment 20 Jonathan Druart 2013-04-15 09:10:49 UTC
Created attachment 17440 [details] [review]
Bug 9463 [Revised] Use DataTables on patron sent notices page

Replace the tablesorter plugin with the DataTables plugin on the
patron sent notices page.

This patch adds default sorting by time (descending).

This patch depends on the one for Bug 9887, which introduces a new
DataTables plugin to allow sorting on data embedded in a <span>'s
"title" attribute. This allows sorting to be done on an unformatted
datetime.

To test, open the sent notices page for a patron who has multiple sent
notices. Confirm that table sorting works correctly with various date
format settings.

Revision corrects an error which occurs when there are no notices:
Instead of displaying an empty table with a message the table is not
displayed and a standard dialog message is displayed. In order to
prevent page-specific CSS and JS from conflicting with the standard
"message" class the page-specific classes have been changed from
"message*" to "notice*" Test this change by viewing a page with existing
notices and clicking the titles to show/hide the notice contents.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 21 Jared Camins-Esakov 2013-04-15 13:16:25 UTC
This patch has been pushed to master and 3.12.x.