Bug 27353 - Return the number of total records
Summary: Return the number of total records
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 27544
Blocks: 27467 28880 27402
  Show dependency treegraph
 
Reported: 2021-01-06 15:31 UTC by Jonathan Druart
Modified: 2022-06-06 20:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00


Attachments
Bug 27353: Set X-Base-Total-Count header for REST API (4.79 KB, patch)
2021-01-06 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (5.68 KB, patch)
2021-01-06 16:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Add tests (9.35 KB, patch)
2021-01-06 16:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (5.45 KB, patch)
2021-01-11 15:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (5.45 KB, patch)
2021-01-12 09:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (7.35 KB, patch)
2021-01-12 09:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (7.33 KB, patch)
2021-01-25 13:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (7.33 KB, patch)
2021-01-25 13:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27353: Add tests (9.25 KB, patch)
2021-01-25 17:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (6.48 KB, patch)
2021-01-25 17:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27353: (QA follow-up) Fix number of tests (775 bytes, patch)
2021-01-25 17:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27353: Add tests (9.31 KB, patch)
2021-01-26 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27353: Set X-Base-Total-Count header for REST API (6.53 KB, patch)
2021-01-26 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27353: (QA follow-up) Fix number of tests (838 bytes, patch)
2021-01-26 12:39 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 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?