Merge pull request #6605 from PauloVilarinho/refactor-pagination-data

Refactor pagination data

app/assets/javascripts/admin/index_utils/services/paged_fetcher.js.coffee

@@ -16,7 +16,7 @@ angular.module("admin.indexUtils").factory "PagedFetcher", (dataFetcher) ->
     fetchPages: (url, page, pageCallback) ->
       dataFetcher(@urlForPage(url, page)).then (data) =>
         @page++
-        @last_page = data.pages
+        @last_page = data.pagination.pages
 
         pageCallback(data) if pageCallback
 

app/controllers/admin/bulk_line_items_controller.rb

@@ -1,5 +1,6 @@
 module Admin
   class BulkLineItemsController < Spree::Admin::BaseController
+    include PaginationData
     # GET /admin/bulk_line_items.json
     #
     def index
@@ -12,9 +13,12 @@ module Admin
         ransack(params[:q]).result.
         reorder('spree_line_items.order_id ASC, spree_line_items.id ASC')
 
-      @line_items = @line_items.page(page).per(params[:per_page]) if using_pagination?
+      @line_items = @line_items.page(page).per(params[:per_page]) if pagination_required?
 
-      render json: { line_items: serialized_line_items, pagination: pagination_data }
+      render json: {
+        line_items: serialized_line_items,
+        pagination: pagination_data(@line_items)
+      }
     end
 
     # PUT /admin/bulk_line_items/:id.json
@@ -87,21 +91,6 @@ module Admin
       ::Permissions::Order.new(spree_current_user)
     end
 
-    def using_pagination?
-      params[:per_page]
-    end
-
-    def pagination_data
-      return unless using_pagination?
-
-      {
-        results: @line_items.total_count,
-        pages: @line_items.num_pages,
-        page: page.to_i,
-        per_page: params[:per_page].to_i
-      }
-    end
-
     def page
       params[:page] || 1
     end

app/controllers/api/exchange_products_controller.rb

@@ -5,6 +5,7 @@
 # Pagination is optional and can be required by using param[:page]
 module Api
   class ExchangeProductsController < Api::BaseController
+    include PaginationData
     DEFAULT_PER_PAGE = 100
 
     skip_authorization_check only: [:index]
@@ -77,29 +78,16 @@ module Api
     end
 
     def render_paginated_products(paginated_products)
-      serializer = ActiveModel::ArraySerializer.new(
+      serialized_products = ActiveModel::ArraySerializer.new(
         paginated_products,
         each_serializer: Api::Admin::ForOrderCycle::SuppliedProductSerializer,
         order_cycle: @order_cycle
       )
 
-      result = { products: serializer }
-      result = result.merge(pagination: pagination_data(paginated_products)) if pagination_required?
-
-      render text: result.to_json
-    end
-
-    def pagination_data(paginated_products)
-      {
-        results: paginated_products.total_count,
-        pages: paginated_products.num_pages,
-        page: params[:page].to_i,
-        per_page: (params[:per_page] || DEFAULT_PER_PAGE).to_i
+      render json: {
+        products: serialized_products,
+        pagination: pagination_data(paginated_products)
       }
     end
-
-    def pagination_required?
-      params[:page].present?
-    end
   end
 end

app/controllers/api/orders_controller.rb

@@ -1,5 +1,7 @@
 module Api
   class OrdersController < Api::BaseController
+    include PaginationData
+
     def show
       authorize! :read, order
       render json: order, serializer: Api::OrderDetailedSerializer, current_order: order
@@ -8,11 +10,11 @@ module Api
     def index
       authorize! :admin, Spree::Order
 
-      search_results = SearchOrders.new(params, current_api_user)
+      orders = SearchOrders.new(params, current_api_user).orders
 
       render json: {
-        orders: serialized_orders(search_results.orders),
-        pagination: search_results.pagination_data
+        orders: serialized_orders(orders),
+        pagination: pagination_data(orders)
       }
     end
 

app/controllers/api/products_controller.rb

@@ -3,8 +3,8 @@ require 'spree/core/product_duplicator'
 
 module Api
   class ProductsController < Api::BaseController
+    include PaginationData
     respond_to :json
-    DEFAULT_PAGE = 1
     DEFAULT_PER_PAGE = 15
 
     skip_authorization_check only: [:show, :bulk_products, :overridable]
@@ -63,7 +63,7 @@ module Api
       @products = product_query.
         ransack(query_params_with_defaults).
         result.
-        page(params[:page] || DEFAULT_PAGE).
+        page(params[:page] || 1).
         per(params[:per_page] || DEFAULT_PER_PAGE)
 
       render_paged_products @products
@@ -130,33 +130,21 @@ module Api
     end
 
     def render_paged_products(products, product_serializer = ::Api::Admin::ProductSerializer)
-      serializer = ActiveModel::ArraySerializer.new(
+      serialized_products = ActiveModel::ArraySerializer.new(
         products,
         each_serializer: product_serializer
       )
 
-      render text: {
-        products: serializer,
-        # This line is used by the PagedFetcher JS service (inventory).
-        pages: products.num_pages,
-        # This hash is used by the BulkProducts JS service.
+      render json: {
+        products: serialized_products,
         pagination: pagination_data(products)
-      }.to_json
+      }
     end
 
     def query_params_with_defaults
       (params[:q] || {}).reverse_merge(s: 'created_at desc')
     end
 
-    def pagination_data(results)
-      {
-        results: results.total_count,
-        pages: results.num_pages,
-        page: (params[:page] || DEFAULT_PAGE).to_i,
-        per_page: (params[:per_page] || DEFAULT_PER_PAGE).to_i
-      }
-    end
-
     def product_params
       params.require(:product).permit PermittedAttributes::Product.attributes
     end

app/controllers/concerns/pagination_data.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module PaginationData
+  extend ActiveSupport::Concern
+
+  def pagination_data(objects)
+    return unless objects.respond_to? :total_count
+
+    {
+      results: objects.total_count,
+      pages: objects.total_pages,
+      page: (params[:page] || 1).to_i,
+      per_page: (params[:per_page] || default_per_page).to_i
+    }
+  end
+
+  def pagination_required?
+    params[:page].present? || params[:per_page].present?
+  end
+
+  def default_per_page
+    return unless defined? self.class::DEFAULT_PER_PAGE
+
+    self.class::DEFAULT_PER_PAGE
+  end
+end

app/services/products_renderer.rb

@@ -2,7 +2,6 @@ require 'open_food_network/scope_product_to_hub'
 
 class ProductsRenderer
   class NoProducts < RuntimeError; end
-  DEFAULT_PAGE = 1
   DEFAULT_PER_PAGE = 10
 
   def initialize(distributor, order_cycle, customer, args = {})
@@ -51,7 +50,7 @@ class ProductsRenderer
     query.
       ransack(args[:q]).
       result.
-      page(args[:page] || DEFAULT_PAGE).
+      page(args[:page] || 1).
       per(args[:per_page] || DEFAULT_PER_PAGE)
   end
 

app/services/search_orders.rb

@@ -8,17 +8,6 @@ class SearchOrders
     @orders = fetch_orders
   end
 
-  def pagination_data
-    return unless using_pagination?
-
-    {
-      results: @orders.total_count,
-      pages: @orders.num_pages,
-      page: params[:page].to_i,
-      per_page: params[:per_page].to_i
-    }
-  end
-
   private
 
   attr_reader :params, :current_user
@@ -50,6 +39,6 @@ class SearchOrders
   end
 
   def using_pagination?
-    params[:per_page]
+    params[:page]
   end
 end

spec/services/search_orders_spec.rb

@@ -18,20 +18,4 @@ describe SearchOrders do
       expect(service.orders.count).to eq 3
     end
   end
-
-  describe '#pagination_data' do
-    let(:params) { { per_page: 15, page: 1 } }
-    let(:service) { SearchOrders.new(params, enterprise_user) }
-
-    it 'returns pagination data' do
-      pagination_data = {
-        results: 3,
-        pages: 1,
-        page: 1,
-        per_page: 15
-      }
-
-      expect(service.pagination_data).to eq pagination_data
-    end
-  end
 end