Bug 27353

Summary: Return the number of total records
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: REST APIAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: caroline.cyr-la-rose, fridolin.somers, nugged, 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:
21.05.00
Bug Depends on: 27544    
Bug Blocks: 27467, 28880, 27402    
Attachments: Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Add tests
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: Add tests
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: (QA follow-up) Fix number of tests
Bug 27353: Add tests
Bug 27353: Set X-Base-Total-Count header for REST API
Bug 27353: (QA follow-up) Fix number of tests

Description Jonathan Druart 2021-01-06 15:31:59 UTC
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.
Comment 1 Jonathan Druart 2021-01-06 15:37:38 UTC
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)
Comment 2 Jonathan Druart 2021-01-06 16:12:58 UTC
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)
Comment 3 Jonathan Druart 2021-01-06 16:13:01 UTC
Created attachment 114897 [details] [review]
Bug 27353: Add tests
Comment 4 Jonathan Druart 2021-01-11 15:50:01 UTC
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)
Comment 5 Jonathan Druart 2021-01-12 09:44:35 UTC
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)
Comment 6 Jonathan Druart 2021-01-12 09:45:33 UTC
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 7 Tomás Cohen Arazi 2021-01-25 13:22:05 UTC
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.
Comment 8 Jonathan Druart 2021-01-25 13:34:28 UTC
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)
Comment 9 Jonathan Druart 2021-01-25 13:35:47 UTC
I fixed the 2 copy/paste errors.

> This looks like an actual bug in the original code!

What exactly?
Comment 10 Jonathan Druart 2021-01-25 13:52:12 UTC
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)
Comment 11 Jonathan Druart 2021-01-25 13:54:56 UTC
(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.
Comment 12 Tomás Cohen Arazi 2021-01-25 17:52:44 UTC
Created attachment 115784 [details] [review]
Bug 27353: Add tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2021-01-25 17:52:49 UTC
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>
Comment 14 Tomás Cohen Arazi 2021-01-25 17:52:56 UTC
Created attachment 115786 [details] [review]
Bug 27353: (QA follow-up) Fix number of tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Martin Renvoize 2021-01-26 12:39:00 UTC
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>
Comment 16 Martin Renvoize 2021-01-26 12:39:04 UTC
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>
Comment 17 Martin Renvoize 2021-01-26 12:39:07 UTC
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>
Comment 18 Martin Renvoize 2021-01-26 12:39:36 UTC
Nice little enhancement, works as expected, no regressions found and passes QA scripts.

Passing QA
Comment 19 Jonathan Druart 2021-01-28 16:34:45 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 20 Fridolin Somers 2021-02-01 13:44:09 UTC
What do you say about backporting to 20.11.x ?
Comment 21 Andrii Nugged 2021-08-18 18:26:40 UTC
Bug 28800: X-Base-Total-Count undefined in "old-style-usage", should we add backward compatibility?