refactored invoices controller

.rubocop.yml

@@ -16,3 +16,5 @@ Style/EmptyElse:
   EnforcedStyle: empty
 Style/ClassAndModuleChildren:
   EnforcedStyle: compact
+Style/AndOr:
+  EnforcedStyle: conditionals

app/controllers/api/invoices_controller.rb

@@ -1,15 +1,20 @@
+# frozen_string_literal: true
+
+# API Controller for resources of Invoice and Avoir
 class API::InvoicesController < API::ApiController
   before_action :authenticate_user!
-  before_action :set_invoice, only: [:show, :download]
+  before_action :set_invoice, only: %i[show download]
 
   def index
     authorize Invoice
-    @invoices = Invoice.includes(:avoir, :invoiced, invoice_items: [:subscription, :invoice_item], user: [:profile, :trainings]).all.order('reference DESC')
+    @invoices = Invoice.includes(
+      :avoir, :invoiced, invoice_items: %i[subscription invoice_item], user: %i[profile trainings]
+    ).all.order('reference DESC')
   end
 
   def download
     authorize @invoice
-    send_file File.join(Rails.root, @invoice.file), :type => 'application/pdf', :disposition => 'attachment'
+    send_file File.join(Rails.root, @invoice.file), type: 'application/pdf', disposition: 'attachment'
   end
 
   def list
@@ -17,44 +22,18 @@ class API::InvoicesController < API::ApiController
 
     p = params.require(:query).permit(:number, :customer, :date, :order_by, :page, :size)
 
-    unless p[:page].is_a? Integer
-      render json: {error: 'page must be an integer'}, status: :unprocessable_entity
-    end
+    render json: { error: 'page must be an integer' }, status: :unprocessable_entity and return unless p[:page].is_a? Integer
 
-    unless p[:size].is_a? Integer
-      render json: {error: 'size must be an integer'}, status: :unprocessable_entity
-    end
-
-
-    direction = (p[:order_by][0] == '-' ? 'DESC' : 'ASC')
-    order_key = (p[:order_by][0] == '-' ? p[:order_by][1, p[:order_by].size] : p[:order_by])
-
-    case order_key
-      when 'reference'
-        order_key = 'invoices.reference'
-      when 'date'
-        order_key = 'invoices.created_at'
-      when 'total'
-        order_key = 'invoices.total'
-      when 'name'
-        order_key = 'profiles.first_name'
-      else
-        order_key = 'invoices.id'
-    end
-
-    @invoices = Invoice.includes(:avoir, :invoiced, invoice_items: [:subscription, :invoice_item], user: [:profile, :trainings])
-                .joins(:user => :profile)
-                .order("#{order_key} #{direction}")
-                .page(p[:page])
-                .per(p[:size])
-
-    # ILIKE => PostgreSQL case-insensitive LIKE
-    @invoices = @invoices.where('invoices.reference LIKE :search', search: "#{p[:number].to_s}%") if p[:number].size > 0
-    @invoices = @invoices.where('profiles.first_name ILIKE :search OR profiles.last_name ILIKE :search', search: "%#{p[:customer]}%") if p[:customer].size > 0
-    @invoices = @invoices.where("date_trunc('day', invoices.created_at) = :search", search: "%#{DateTime.iso8601(p[:date]).to_time.to_date.to_s}%") unless p[:date].nil?
-
-    @invoices
+    render json: { error: 'size must be an integer' }, status: :unprocessable_entity and return unless p[:size].is_a? Integer
 
+    order = InvoicesService.parse_order(p[:order_by])
+    @invoices = InvoicesService.list(
+      order[:order_key],
+      order[:direction],
+      p[:page],
+      p[:size],
+      number: p[:number], customer: p[:customer], date: p[:date]
+    )
   end
 
   # only for create refund invoices (avoir)
@@ -64,9 +43,7 @@ class API::InvoicesController < API::ApiController
     @avoir = invoice.build_avoir(avoir_params)
     if @avoir.save
       # when saved, expire the subscription if needed
-      if @avoir.subscription_to_expire
-        @avoir.expire_subscription
-      end
+      @avoir.expire_subscription if @avoir.subscription_to_expire
       # then answer the API call
       render :avoir, status: :created
     else
@@ -75,11 +52,13 @@ class API::InvoicesController < API::ApiController
   end
 
   private
-    def avoir_params
-      params.require(:avoir).permit(:invoice_id, :avoir_date, :avoir_mode, :subscription_to_expire, :description, :invoice_items_ids => [])
-    end
 
-    def set_invoice
-      @invoice = Invoice.find(params[:id])
-    end
+  def avoir_params
+    params.require(:avoir).permit(:invoice_id, :avoir_date, :avoir_mode, :subscription_to_expire, :description,
+                                  invoice_items_ids: [])
+  end
+
+  def set_invoice
+    @invoice = Invoice.find(params[:id])
+  end
 end

app/services/invoices_service.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+# Provides methods for accessing Invoices resources and properties
+class InvoicesService
+  # return a paginated list of invoices, ordered by the given criterion and optionally filtered
+  # @param order_key {string} any column from invoices or joined a table
+  # @param direction {string} 'ASC' or 'DESC', linked to order_key
+  # @param page {number} page number, used to paginate results
+  # @param size {number} number of items per page
+  # @param filters {Hash} allowed filters: number, customer, date.
+  def self.list(order_key, direction, page, size, filters = {})
+    invoices = Invoice.includes(:avoir, :invoiced, invoice_items: %i[subscription invoice_item], user: %i[profile trainings])
+                      .joins(user: :profile)
+                      .order("#{order_key} #{direction}")
+                      .page(page)
+                      .per(size)
+
+
+    if filters[:number].size.positive?
+      invoices = invoices.where(
+        'invoices.reference LIKE :search',
+        search: "#{filters[:number]}%"
+      )
+    end
+    if filters[:customer].size.positive?
+      # ILIKE => PostgreSQL case-insensitive LIKE
+      invoices = invoices.where(
+        'profiles.first_name ILIKE :search OR profiles.last_name ILIKE :search',
+        search: "%#{filters[:customer]}%"
+      )
+    end
+    unless filters[:date].nil?
+      invoices = invoices.where(
+        "date_trunc('day', invoices.created_at) = :search",
+        search: "%#{DateTime.iso8601(filters[:date]).to_time.to_date}%"
+      )
+    end
+
+    invoices
+  end
+
+  # Parse the order_by clause provided by JS client from '-column' form to SQL compatible form
+  # @param order_by {string} expected form: 'column' or '-column'
+  def self.parse_order(order_by)
+    direction = (order_by[0] == '-' ? 'DESC' : 'ASC')
+    key = (order_by[0] == '-' ? order_by[1, order_by.size] : order_by)
+
+    order_key = case key
+                when 'reference'
+                  'invoices.reference'
+                when 'date'
+                  'invoices.created_at'
+                when 'total'
+                  'invoices.total'
+                when 'name'
+                  'profiles.first_name'
+                else
+                  'invoices.id'
+                end
+    { direction: direction, order_key: order_key }
+  end
+end

test/integration/invoices/as_admin_test.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class InvoicesTest < ActionDispatch::IntegrationTest
+
+  # Called before every test method runs. Can be used
+  # to set up fixture information.
+  def setup
+    @admin = User.find_by(username: 'admin')
+    login_as(@admin, scope: :user)
+  end
+
+  test 'admin list invoices' do
+
+    post '/api/invoices/list', { query: {
+      number: '',
+      customer: '',
+      date: nil,
+      order_by: '-reference',
+      page: 1,
+      size: 20 # test db may have < 20 invoices
+    } }.to_json, default_headers
+
+    # Check response format & status
+    assert_equal 200, response.status, response.body
+    assert_equal Mime::JSON, response.content_type
+
+    # Check that we have all invoices
+    invoices = json_response(response.body)
+    assert_equal Invoice.count, invoices.size, 'some invoices are missing'
+
+    # Check that invoices are ordered by reference
+    assert_equal '1604002', invoices.first[:reference]
+    assert_equal '1203001', invoices.last[:reference]
+  end
+
+end

test/integration/members/as_admin_test.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class MemebersTest < ActionDispatch::IntegrationTest
+class MembersTest < ActionDispatch::IntegrationTest
 
   # Called before every test method runs. Can be used
   # to set up fixture information.