Bug 13887

Summary: Add datatables to item specific holds table
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, katrin.fischer, nengard, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 13887 - Add datatables to item specific holds table
datatables
Bug 13887 - Add datatables to item specific holds table
Bug 13887 - Add datatables to item specific holds table
Bug 13887 - Add datatables to item specific holds table
Bug 13887 - Add datatables to item specific holds table
[QA Follow up] Tab fix and "intranet-tmpl" to "[% interface %]"
Bug 13887 - Add datatables to item specific holds table
Bug 13887: Tab fix and "intranet-tmpl" to "[% interface %]"
Bug 13887: Fix sDom to display what is needed in the table
Bug 13887: Move the caption into a h2
Bug 13887: Display the due date as a due date + sort on info
Bug 13887: Fix sDom to display what is needed in the table
Bug 13887: Move the caption into a h2
Bug 13887: Display the due date as a due date + sort on info
[PASSED QA] Bug 13887 - Add datatables to item specific holds table
[PASSED QA] Bug 13887: Tab fix and "intranet-tmpl" to "[% interface %]"
[PASSED QA] Bug 13887: Fix sDom to display what is needed in the table
[PASSED QA] Bug 13887: Move the caption into a h2
[PASSED QA] [PASSED QA] Bug 13887: Display the due date as a due date + sort on info
[PASSED QA] Bug 13887: Display the due date as a due date + sort on info

Description Nick Clemens (kidclamp) 2015-03-21 17:28:46 UTC
Currently when placing an item specific hold there is no, discernable, ordering to the table or a way to search. On titles with many items it can be hard to locate a copy for a specific library
Comment 1 Nick Clemens (kidclamp) 2015-03-21 17:48:46 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2015-03-29 20:06:52 UTC
Created attachment 37345 [details]
datatables

Don't look quite right. See attached.
Comment 3 Nick Clemens (kidclamp) 2015-03-30 20:13:19 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens (kidclamp) 2015-03-30 20:15:47 UTC
That should look better.

The only way I could figure out to fix the styling was using js to alter the css but am definitely willing to update if there is a better/preferred method
Comment 5 Nicole C. Engard 2015-03-30 21:26:13 UTC
Sorry, this one doesn't apply.

16:25 ~/kohaclone (master % u=)$ git qa 13887
From git://git.koha-community.org/koha
 * branch            master     -> FETCH_HEAD
Branch bug13887-qa set up to track remote branch master from origin.
Switched to a new branch 'bug13887-qa'
Bug 13887 - Add datatables to item specific holds table

37385 - Bug 13887 - Add datatables to item specific holds table

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 13887 - Add datatables to item specific holds table
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 13887 - Add datatables to item specific holds table
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-13887---Add-datatables-to-item-specific-holds--D8nqyD.patch
16:25 ~/kohaclone (bug13887-qa *+%|AM u=)$
Comment 6 Nick Clemens (kidclamp) 2015-03-30 22:02:12 UTC Comment hidden (obsolete)
Comment 7 Nicole C. Engard 2015-03-31 18:01:26 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-04-20 15:09:53 UTC
Hi Nick,

1/ The table does not have the tbody element
2/ Why don't you add the search box? It could be useful
3/ The "information" column should sort on due date value, don't you think?

Marked as Failed QA for 1 and Discussion for 2-3.
Comment 9 Nick Clemens (kidclamp) 2015-04-22 01:19:34 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens (kidclamp) 2015-04-22 01:19:40 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens (kidclamp) 2015-04-22 01:27:40 UTC
(In reply to Jonathan Druart from comment #8)
> Hi Nick,
> 
> 1/ The table does not have the tbody element

Added - did the whitespace fixes in patch as it was adding a level

> 2/ Why don't you add the search box? It could be useful

I thought the search box is added by default on koha datatables, appears for me when applied, does it need to be added explicitly in this template?

> 3/ The "information" column should sort on due date value, don't you think?

Do you mean that should be the default sort?  I liked it sorting on the hold column/ability to place a hold and that seemed the most useful.  It is common for us to have only 2 or 3 items out of 50 available for hold, but we do use holds in an uncommon manner.

> Marked as Failed QA for 1 and Discussion for 2-3.

Setting back to signed-off but willing to change #2 or #3 if necessary

Thanks Jonathan
Comment 12 Jonathan Druart 2015-04-22 09:10:26 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-04-22 09:10:30 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-04-22 09:10:34 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-04-22 09:10:38 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-04-22 09:10:43 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-04-22 09:14:10 UTC
Nick,
Could you please test the 3 followups I have added?
They improves the ergonomic of the page, 
The first one modify the default sDom for a better render, the second one move the caption above the filter and the last one displays the due dates correctly (and fixes the column sorting).

Feel free to obsolete them if you disagree.
Comment 18 Nick Clemens (kidclamp) 2015-04-22 11:17:53 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens (kidclamp) 2015-04-22 11:18:00 UTC Comment hidden (obsolete)
Comment 20 Nick Clemens (kidclamp) 2015-04-22 11:18:06 UTC Comment hidden (obsolete)
Comment 21 Nick Clemens (kidclamp) 2015-04-22 11:19:47 UTC
(In reply to Jonathan Druart from comment #17)
> Could you please test the 3 followups I have added?

Tested, they definitely improve the page, thanks for fixing it up!
Comment 22 Katrin Fischer 2015-04-22 20:32:36 UTC
Created attachment 38369 [details] [review]
[PASSED QA] Bug 13887 - Add datatables to item specific holds table

This patch adds datatables functionality to the item specific holds table

To test:
1 - Apply patch
2 - Attempt to place an item specific hold for a patron
3 - Note that columns are now sortable and searchable
4 - Ensure that you can place both specific and next available holds as before

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2015-04-22 20:32:39 UTC
Created attachment 38370 [details] [review]
[PASSED QA] Bug 13887: Tab fix and "intranet-tmpl" to "[% interface %]"

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2015-04-22 20:32:43 UTC
Created attachment 38371 [details] [review]
[PASSED QA] Bug 13887: Fix sDom to display what is needed in the table

1/ Add the information 'showing x to y of z'
2/ Remove some lost pixels

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2015-04-22 20:32:46 UTC
Created attachment 38372 [details] [review]
[PASSED QA] Bug 13887: Move the caption into a h2

With the DT filter, the caption is between the table and the filter.
It looks better with the caption as a h2 on top of the table.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2015-04-22 20:32:50 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2015-04-22 20:36:42 UTC
Created attachment 38374 [details] [review]
[PASSED QA] Bug 13887: Display the due date as a due date + sort on info

The due dates should be displayed as due dates :)
i.e not displayed with 23:59

On the way, this patch fixes the sort on the info column.
The column is now sorted using the due dates

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Tomás Cohen Arazi 2015-05-04 14:47:33 UTC
Patches pushed to master.

Thanks Nick!