Bug 25777 - Datatables on z3950_search.pl show incorrect number of entries
Summary: Datatables on z3950_search.pl show incorrect number of entries
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact: Martin Renvoize
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2020-06-17 04:27 UTC by David Cook
Modified: 2021-12-13 21:09 UTC (History)
7 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:
21.05.00,20.11.04,20.05.10


Attachments
Bug 25777: Disable misleading datatables table information display z3950 (1.45 KB, patch)
2020-06-17 04:37 UTC, David Cook
Details | Diff | Splinter Review
Bug 25777: Disable misleading datatables table information display z3950 (1.53 KB, patch)
2021-01-21 22:18 UTC, Ava
Details | Diff | Splinter Review
Bug 25777: Disable misleading datatables table information display z3950 (1.58 KB, patch)
2021-01-25 17:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25777: (follow-up) Make the same configuration change to Z39.50 search in Acquisitions (1.34 KB, patch)
2021-01-25 17:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25777: Disable misleading datatables table information display z3950 (1.60 KB, patch)
2021-02-03 15:12 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25777: (follow-up) Make the same configuration change to Z39.50 search in Acquisitions (1.35 KB, patch)
2021-02-03 15:12 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25777: Disable misleading datatables table information display z3950 (1.66 KB, patch)
2021-02-04 04:15 UTC, David Cook
Details | Diff | Splinter Review
Bug 25777: (follow-up) Make the same configuration change to Z39.50 search in Acquisitions (1.41 KB, patch)
2021-02-04 04:15 UTC, David Cook
Details | Diff | Splinter Review
Bug 25777: (QA follow-up) Correct option: paging (1.08 KB, patch)
2021-02-05 11:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25777: Disable misleading datatables table information display z3950 (2.98 KB, patch)
2021-02-26 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-06-17 04:27:44 UTC
Datatables on z3950_search.pl will say "Showing 1 to 20 of 20 entries", which makes you think that there are only 20 search results.

However, if you scroll to the bottom of the page, you'll often see it saying something like "Showing page 1 of 501".

We should hide the "Showing 1 to 20 of 20 entries" datatables information if we're not going to implement a JSON sourced table.
Comment 1 David Cook 2020-06-17 04:37:13 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2020-06-17 12:12:44 UTC
Should be fixed on other Z3950 search pages as well.

I would also remove the search box as it only filter on the first page.
Comment 3 David Cook 2020-06-18 02:31:14 UTC
(In reply to Jonathan Druart from comment #2)
> Should be fixed on other Z3950 search pages as well.
> 
> I would also remove the search box as it only filter on the first page.

Mmm I was wondering about that. Fixed this one a bit too hastily. I'll re-visit it.
Comment 4 David Cook 2021-01-21 04:48:17 UTC
Ava, did you mean to take over as Assignee? 

(I hope you will as I haven't had time for this one.)
Comment 5 Ava 2021-01-21 20:22:07 UTC
(In reply to David Cook from comment #4)
> Ava, did you mean to take over as Assignee? 
> 
> (I hope you will as I haven't had time for this one.)

Yes I did! I was planning on working on it yesterday but I think the Library of Congress was down. If it's up today I will work on it
Comment 6 Ava 2021-01-21 22:18:25 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2021-01-25 17:53:47 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2021-01-25 17:53:50 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2021-02-03 15:12:35 UTC
Created attachment 116269 [details] [review]
Bug 25777: Disable misleading datatables table information display z3950

This patch disables the table information display on z3950_search.pl,
since it only includes search result counts for that page, which is
misleading to end users.

To test:
1) Do not apply the patch
2) Do a Z39.50 search for "test" against Library of Congress
3) Note the bottom of the page says "Show 1 to 20 of 20" and
"Showing page 1 of 501"
4) Apply the patch and restart Plack
5) Repeat Step #2
6) Note the bottom of the page now only says "Showing page 1 of 501"
7) Note the search bar is gone

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Owen Leonard 2021-02-03 15:12:38 UTC
Created attachment 116270 [details] [review]
Bug 25777: (follow-up) Make the same configuration change to Z39.50 search in Acquisitions

To test, apply the patch and go to Acquisitions -> Vendor -> Basket ->
Add to basket -> From external source. Perform a Z39.50 search and check
that the DataTable of results doesn't have a search filter or number of
results information at the top.
Comment 11 David Cook 2021-02-04 04:15:27 UTC
Created attachment 116306 [details] [review]
Bug 25777: Disable misleading datatables table information display z3950

This patch disables the table information display on z3950_search.pl,
since it only includes search result counts for that page, which is
misleading to end users.

To test:
1) Do not apply the patch
2) Do a Z39.50 search for "test" against Library of Congress
3) Note the bottom of the page says "Show 1 to 20 of 20" and
"Showing page 1 of 501"
4) Apply the patch and restart Plack
5) Repeat Step #2
6) Note the bottom of the page now only says "Showing page 1 of 501"
7) Note the search bar is gone

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

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 12 David Cook 2021-02-04 04:15:30 UTC
Created attachment 116307 [details] [review]
Bug 25777: (follow-up) Make the same configuration change to Z39.50 search in Acquisitions

To test, apply the patch and go to Acquisitions -> Vendor -> Basket ->
Add to basket -> From external source. Perform a Z39.50 search and check
that the DataTable of results doesn't have a search filter or number of
results information at the top.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 13 David Cook 2021-02-04 04:16:13 UTC
2020 lasted so long that I forgot that I wrote that first patch...
Comment 14 Marcel de Rooy 2021-02-05 07:17:36 UTC
git grep -E "bPaginate.*true"
koha-tmpl/intranet-tmpl/lib/datatables/datatables.js:           "bPaginate": true,
koha-tmpl/intranet-tmpl/prog/en/modules/tools/quotes-upload.tt:                "bPaginate"         : true,
Comment 15 Marcel de Rooy 2021-02-05 07:19:55 UTC
-                "bPaginate": false
+                "bPaginate": false,

-                "bPaginate": false
+                "paginate": false,

Two very small patches. But not the same ;)
Comment 16 Marcel de Rooy 2021-02-05 07:20:18 UTC
Not sure if this is a bug btw or just reading the documentation or so ?
Comment 17 Owen Leonard 2021-02-05 11:20:57 UTC
Created attachment 116391 [details] [review]
Bug 25777: (QA follow-up) Correct option: paging

The correct upgrade from the old "bPaginate" option is "paging"
Comment 18 Katrin Fischer 2021-02-07 13:02:58 UTC
Sorry for asking again, but should not both spots be changed to use "paging" now?

+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/z3950_search.tt
@@ -246,7 +246,9 @@
                     { "aTargets": [ 1 ], "sType": "nsb-nse" },
                 ],
                 "aaSorting": [[ 1, "asc" ]],
-                "bPaginate": false
+                "bPaginate": false,
+                "searching": false,
+                "bInfo":false
Comment 19 Owen Leonard 2021-02-08 11:41:39 UTC
(In reply to Katrin Fischer from comment #18)
> Sorry for asking again, but should not both spots be changed to use "paging"
> now?

Both are valid at this time. This may change later in a future DataTables upgrade. I chose to change it in my patch because I was touching that part of the template anyway.

Without an established rule that these options should be updated I don't think it should be a QA failure.
Comment 20 Marcel de Rooy 2021-02-12 07:46:30 UTC
(In reply to Owen Leonard from comment #19)
> Without an established rule that these options should be updated I don't
> think it should be a QA failure.

When patches of even two lines are inconsistent..
Comment 21 Marcel de Rooy 2021-02-12 07:52:10 UTC
-                "bPaginate": false
+                "paging": false,
+                "searching": false,
+                "info":false

-                "bPaginate": false
+                "bPaginate": false,
+                "searching": false,
+                "bInfo":false

Come on, what are we doing here?
Please squash and be consistent :)
Comment 22 Martin Renvoize 2021-02-26 08:58:45 UTC
Created attachment 117367 [details] [review]
Bug 25777: Disable misleading datatables table information display z3950

This patch disables the table information display on z3950_search.pl,
since it only includes search result counts for that page, which is
misleading to end users.

To test:
1)  Do not apply the patch
2)  Do a Z39.50 search from cataloguing for "test" against Library of
    Congress.
3)  Note the bottom of the page says "Show 1 to 20 of 20" and
    "Showing page 1 of 501"
4)  Do a Z39.50 search from Acquisitions for "test" against Library of
    Congress (Acquisitions -> Vendor -> Basket -> Add to basket -> From
    external source).
5)  Apply the patch and restart Plack
6)  Repeat Step #2
7)  Note the bottom of the page now only says "Showing page 1 of 501"
8)  Note the search bar is gone
9)  Repeat step #4
10) Note the bottom of the page now only says "Showing page 1 of 501"
11) Note the search bar is gone

Squashed and mixed authorship added below
Co-authored-by: Owen Leonard <oleonard@myacpl.org>
Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2021-02-26 09:00:02 UTC
Patches squashed, syntax updated to be consistent between the two files, QA scripts passing, tests passing, manual testing proves it works as expected.


Passing QA
Comment 24 Jonathan Druart 2021-03-01 10:28:47 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 25 Fridolin Somers 2021-03-04 09:34:28 UTC
Pushed to 20.11.x for 20.11.04
Comment 26 Andrew Fuerste-Henry 2021-03-05 21:12:09 UTC
Pushed to 20.05.x for 20.05.10
Comment 27 Victor Grousset/tuxayo 2021-03-22 16:05:10 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.