Improved offset range pagination labels (#76)

* better pagination labels for offset ranges

* remarks

lib/karafka/web/ui/controllers/base.rb

@@ -42,7 +42,7 @@ def paginate(*args)
             engine = case args.count
                      when 2
                        Ui::Lib::Paginations::PageBased
-                     when 3
+                     when 4
                        Ui::Lib::Paginations::OffsetBased
                      else
                        raise ::Karafka::Errors::UnsupportedCaseError, args.count

lib/karafka/web/ui/controllers/errors.rb

@@ -13,7 +13,12 @@ def index
             @watermark_offsets = Ui::Models::WatermarkOffsets.find(errors_topic, 0)
             previous_offset, @error_messages, next_offset, = current_page_data
 
-            paginate(previous_offset, @params.current_offset, next_offset)
+            paginate(
+              previous_offset,
+              @params.current_offset,
+              next_offset,
+              @error_messages.map(&:offset)
+            )
 
             respond
           end

lib/karafka/web/ui/lib/paginations/offset_based.rb

@@ -14,14 +14,18 @@ class OffsetBased < Base
             # @param current_offset [Integer] current offset
             # @param next_offset [Integer, Boolean] should we show next offset page button. If
             #   false it will not be presented.
+            # @param visible_offsets [Array<Integer>] offsets that are visible in the paginated
+            #   view. It is needed for the current page label
             def initialize(
               previous_offset,
               current_offset,
-              next_offset
+              next_offset,
+              visible_offsets
             )
               @previous_offset = previous_offset
               @current_offset = current_offset
               @next_offset = next_offset
+              @visible_offsets = visible_offsets
               super()
             end
 
@@ -52,11 +56,10 @@ def previous_offset?
               !!@previous_offset
             end
 
-            # @return [Boolean] Since this is offset based pagination, there is no notion of
-            #   the current page and we operate on offsets. Because of that there is no continuous
-            #   pagination, thus we hide the current page.
+            # @return [Boolean] We show current label with offsets that are present on the given
+            #   page
             def current_offset?
-              false
+              true
             end
 
             # @return [Boolean] move to the next page if not false. False indicates, that there is
@@ -72,6 +75,15 @@ def next_offset
               next_offset? ? @next_offset : 0
             end
 
+            # @return [String] label of the current page. It is combined out of the first and
+            #   the last offsets to show the range where we are. It will be empty if no offsets
+            #   but this is not a problem as then we should not display pagination at all
+            def current_label
+              first = @visible_offsets.first
+              last = @visible_offsets.last
+              [first, last].compact.uniq.join(' - ').to_s
+            end
+
             # @return [String] for offset based pagination we use the offset param name
             def offset_key
               'offset'

lib/karafka/web/ui/lib/paginations/page_based.rb

@@ -47,6 +47,11 @@ def current_offset?
               true
             end
 
+            # @return [String] label of the current page
+            def current_label
+              @current_offset.to_s
+            end
+
             # @return [Boolean] move to the next page if not false. False indicates, that there is
             #   no next page to move to
             def next_offset?

lib/karafka/web/ui/pro/controllers/errors.rb

@@ -23,9 +23,14 @@ def index(partition_id)
               @partition_id = partition_id
               @watermark_offsets = Ui::Models::WatermarkOffsets.find(errors_topic, @partition_id)
 
-              previous_page, @error_messages, next_page, @partitions_count = current_page_data
+              previous_offset, @error_messages, next_offset, @partitions_count = current_page_data
 
-              paginate(previous_page, @params.current_offset, next_page)
+              paginate(
+                previous_offset,
+                @params.current_offset,
+                next_offset,
+                @error_messages.map(&:offset)
+              )
 
               respond
             end

lib/karafka/web/ui/pro/controllers/explorer.rb

@@ -40,7 +40,12 @@ def partition(topic_id, partition_id)
 
               previous_offset, @messages, next_offset, @partitions_count = current_page_data
 
-              paginate(previous_offset, @params.current_offset, next_offset)
+              paginate(
+                previous_offset,
+                @params.current_offset,
+                next_offset,
+                @messages.map(&:offset)
+              )
 
               respond
             end

lib/karafka/web/ui/views/shared/_pagination.erb

@@ -18,7 +18,9 @@
           <% if @pagination.current_offset? %>
             <li class="page-item active disabled">
               <a class="page-link" href="<%= current_path(@pagination.offset_key => @pagination.current_offset) %>">
-                <span><%= @pagination.current_offset %></span>
+                <span>
+                  <%= @pagination.current_label %>
+                </span>
               </a>
             </li>
           <% end %>

spec/lib/karafka/web/ui/lib/paginations/offset_based_spec.rb

@@ -4,8 +4,16 @@
   let(:previous_offset) { 10 }
   let(:current_offset) { 20 }
   let(:next_offset) { 30 }
-
-  subject(:pagination) { described_class.new(previous_offset, current_offset, next_offset) }
+  let(:visible_offsets) { [10, 20, 30] }
+
+  subject(:pagination) do
+    described_class.new(
+      previous_offset,
+      current_offset,
+      next_offset,
+      visible_offsets
+    )
+  end
 
   describe '#paginate?' do
     context 'when there is more than one page' do
@@ -64,7 +72,7 @@
 
   describe '#current_offset?' do
     it 'returns false' do
-      expect(pagination.current_offset?).to be false
+      expect(pagination.current_offset?).to be true
     end
   end
 
@@ -105,4 +113,36 @@
       expect(pagination.offset_key).to eq('offset')
     end
   end
+
+  describe '#current_label' do
+    context 'when visible_offsets are empty' do
+      let(:visible_offsets) { [] }
+
+      it 'returns an empty string' do
+        expect(pagination.current_label).to eq('')
+      end
+    end
+
+    context 'when visible_offsets contain only one offset' do
+      let(:visible_offsets) { [10] }
+
+      it 'returns the single offset as a string' do
+        expect(pagination.current_label).to eq('10')
+      end
+    end
+
+    context 'when visible_offsets contain multiple offsets' do
+      it 'returns the range of the first and last offsets joined with a hyphen' do
+        expect(pagination.current_label).to eq('10 - 30')
+      end
+    end
+
+    context 'when visible_offsets contain duplicate offsets' do
+      let(:visible_offsets) { [10, 20, 10, 30, 30] }
+
+      it 'returns the unique offsets in the range' do
+        expect(pagination.current_label).to eq('10 - 30')
+      end
+    end
+  end
 end

spec/lib/karafka/web/ui/lib/paginations/page_based_spec.rb

@@ -25,6 +25,7 @@
     it { expect(pagination.current_offset).to eq(1) }
     it { expect(pagination.next_offset?).to eq(2) }
     it { expect(pagination.offset_key).to eq('page') }
+    it { expect(pagination.current_label).to eq('1') }
   end
 
   context 'when last page' do
@@ -40,5 +41,6 @@
     it { expect(pagination.current_offset).to eq(10) }
     it { expect(pagination.next_offset?).to eq(false) }
     it { expect(pagination.offset_key).to eq('page') }
+    it { expect(pagination.current_label).to eq('10') }
   end
 end