From aa8778c4e6b4a6fb88607e105465cdb499d8d76b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20=C5=9Awi=C4=85tkowski?= <katafrakt@vivaldi.net>
Date: Wed, 13 Oct 2021 23:41:48 +0200
Subject: [PATCH 1/2] Update the pager object with fresh dataset before return

Pager object was keeping reference to old dataset in situations when
some additionap predicated were called after using #page or #per_page
methods, e.g.:

```
users.page(2).where(active: true)
```

In the example above, pager was still holding reference to a dataset not
having active=true condition set. This resulted in a correct dataset
being returned, but the pager methods like #total_pages returning wrong
results.

In this commit, a new public method #pager is created which shadows
regular usage from dry-initialize by firstly update the pager with fresh
dataset, so the calculations are correct.
---
 lib/rom/sql/plugin/pagination.rb    | 14 ++++++++++++++
 spec/unit/plugin/pagination_spec.rb | 18 ++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/lib/rom/sql/plugin/pagination.rb b/lib/rom/sql/plugin/pagination.rb
index 948c436ba..955ff889b 100644
--- a/lib/rom/sql/plugin/pagination.rb
+++ b/lib/rom/sql/plugin/pagination.rb
@@ -105,6 +105,11 @@ def at(dataset, current_page, per_page = self.per_page)
             )
           end
 
+          # @api private
+          def with_dataset(dataset)
+            self.class.new(dataset, current_page: self.current_page, per_page: self.per_page)
+          end
+
           alias_method :limit_value, :per_page
         end
 
@@ -147,6 +152,15 @@ def per_page(num)
           next_pager = pager.at(dataset, pager.current_page, num)
           new(next_pager.dataset, pager: next_pager)
         end
+
+        # Return a pager object updated with most up-to-date dataset
+        #
+        # @return [Pager]
+        #
+        # @api public
+        def pager
+          @pager_with_fresh_dataset ||= @pager.with_dataset(dataset)
+        end
       end
     end
   end
diff --git a/spec/unit/plugin/pagination_spec.rb b/spec/unit/plugin/pagination_spec.rb
index 04c734458..e2012361c 100644
--- a/spec/unit/plugin/pagination_spec.rb
+++ b/spec/unit/plugin/pagination_spec.rb
@@ -62,6 +62,24 @@
         users = container.relations[:users].per_page(9)
         expect(users.pager.total_pages).to eql(1)
       end
+
+      it 'returns one page when relation is filtered' do
+        users = container.relations[:users].per_page(1).where(name: "User 1")
+        expect(users.pager.total_pages).to eql(1)
+
+        users = container.relations[:users].where(name: "User 1").per_page(1)
+        expect(users.pager.total_pages).to eql(1)
+      end
+    end
+
+    describe "#next_page" do
+      it "returns nil when there is no next page in filtered relation" do
+        users = container.relations[:users].per_page(1).where(name: "User 1")
+        expect(users.pager.next_page).to be(nil)
+
+        users = container.relations[:users].where(name: "User 1").per_page(1)
+        expect(users.pager.next_page).to be(nil)
+      end
     end
 
     describe '#pager' do

From bad3047d94bfcec0043bdfe401e919d3d812a8f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20=C5=9Awi=C4=85tkowski?= <katafrakt@vivaldi.net>
Date: Thu, 14 Oct 2021 20:38:06 +0200
Subject: [PATCH 2/2] Alternative approach without #with_dataset pager method

---
 lib/rom/sql/plugin/pagination.rb | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/lib/rom/sql/plugin/pagination.rb b/lib/rom/sql/plugin/pagination.rb
index 955ff889b..56a4c2d7b 100644
--- a/lib/rom/sql/plugin/pagination.rb
+++ b/lib/rom/sql/plugin/pagination.rb
@@ -105,11 +105,6 @@ def at(dataset, current_page, per_page = self.per_page)
             )
           end
 
-          # @api private
-          def with_dataset(dataset)
-            self.class.new(dataset, current_page: self.current_page, per_page: self.per_page)
-          end
-
           alias_method :limit_value, :per_page
         end
 
@@ -120,7 +115,7 @@ def self.included(klass)
           klass.class_eval do
             defines :per_page
 
-            option :pager, default: -> {
+            option :_pager, default: -> {
               Pager.new(dataset, per_page: self.class.per_page)
             }
           end
@@ -136,8 +131,8 @@ def self.included(klass)
         #
         # @api public
         def page(num)
-          next_pager = pager.at(dataset, num)
-          new(next_pager.dataset, pager: next_pager)
+          next_pager = _pager.at(dataset, num)
+          new(next_pager.dataset, _pager: next_pager)
         end
 
         # Set limit for pagination
@@ -149,8 +144,8 @@ def page(num)
         #
         # @api public
         def per_page(num)
-          next_pager = pager.at(dataset, pager.current_page, num)
-          new(next_pager.dataset, pager: next_pager)
+          next_pager = _pager.at(dataset, _pager.current_page, num)
+          new(next_pager.dataset, _pager: next_pager)
         end
 
         # Return a pager object updated with most up-to-date dataset
@@ -159,7 +154,7 @@ def per_page(num)
         #
         # @api public
         def pager
-          @pager_with_fresh_dataset ||= @pager.with_dataset(dataset)
+          Pager.new(dataset, current_page: _pager.current_page, per_page: _pager.per_page)
         end
       end
     end