We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows.
Created attachment 114893 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
Created attachment 114896 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
Created attachment 114897 [details] [review] Bug 27353: Add tests
Created attachment 115039 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
Created attachment 115058 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
Created attachment 115059 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
Comment on attachment 115059 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API >diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm >index 93feeaeb24..1dd68d264d 100644 >--- a/Koha/REST/V1/Checkouts.pm >+++ b/Koha/REST/V1/Checkouts.pm >@@ -90,13 +90,13 @@ sub list { > > # Perform search > my $checkouts = $checkouts_set->search( $filtered_params, $attributes ); >+ my $total = $checkouts_rs->search->count; $checkouts_set vs. $checkouts_rs. >diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm >index 5eba4bdee3..ad67243cf1 100644 >--- a/Koha/REST/V1/Patrons.pm >+++ b/Koha/REST/V1/Patrons.pm >@@ -82,14 +82,15 @@ sub list { > if $restricted; > > my $patrons = $patrons_rs->search( $filtered_params, $attributes ); >- if ( $patrons_rs->is_paged ) { >- $c->add_pagination_headers( >- { >- total => $patrons->pager->total_entries, >- params => $args, >- } >- ); >- } This looks like an actual bug in the original code! >+ my $total = $orders_rs->search->count; >+ >+ $c->add_pagination_headers( >+ { >+ total => ($patrons->is_paged ? $patrons->pager->total_entries : $patrons->count), >+ base_total => $total, >+ params => $args, >+ } >+ ); $orders_rs looks like a copy and paste error. Overall: I like the implementation. Besides this minor glitches, I think we could have unit tests for the changes to Patrons.pm and Checkouts.pm.
Created attachment 115769 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
I fixed the 2 copy/paste errors. > This looks like an actual bug in the original code! What exactly?
Created attachment 115771 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries)
(In reply to Jonathan Druart from comment #9) > I fixed the 2 copy/paste errors. > > > This looks like an actual bug in the original code! > > What exactly? See bug 27544.
Created attachment 115784 [details] [review] Bug 27353: Add tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 115785 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 115786 [details] [review] Bug 27353: (QA follow-up) Fix number of tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 115807 [details] [review] Bug 27353: Add tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 115808 [details] [review] Bug 27353: Set X-Base-Total-Count header for REST API We already set X-Total-Count to the total number of filtered rows, but we don't have the total number of non-filtered rows. Test plan: This is easy to test on top of bug 27352 or bug 27251, apply them if not pushed yet. 1. Create 40 items with public notes = "xxx" for biblionumber=4 then, using Postman (or whatever you prefer): http://kohadev-intra.mydnsname.org:8081/api/v1/biblios/4/items?_page=1&_per_page=20&q=[{"me.public_notes"%3A{"like"%3A"%25x%25"}}]&_match=contains Check the headers and confirm you see X-Total-Count=40 and X-Base-Total-Count=44 2. go to /cgi-bin/koha/tools/quotes.pl You see "Showing 1 to 20 of 28 entries" Search "he" Showing 1 to 20 of 22 entries (filtered from 28 total entries) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 115809 [details] [review] Bug 27353: (QA follow-up) Fix number of tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice little enhancement, works as expected, no regressions found and passes QA scripts. Passing QA
Pushed to master for 21.05, thanks to everybody involved!
What do you say about backporting to 20.11.x ?
Bug 28800: X-Base-Total-Count undefined in "old-style-usage", should we add backward compatibility?