From 288a35f06224363ee73a76af3906aeddaac6e225 Mon Sep 17 00:00:00 2001 From: Sebastian Castro Date: Wed, 6 Apr 2022 10:40:06 +0200 Subject: [PATCH] Reports Refactor 2: New templates abstract classes --- .rubocop_todo.yml | 10 --- app/controllers/admin/reports_controller.rb | 2 +- .../spree/admin/reports_controller.rb | 14 +--- .../spree/admin/reports/_table.html.haml | 8 +-- app/views/spree/admin/reports/show.html.haml | 5 +- lib/reporting/report_object_template.rb | 34 ++++++++++ lib/reporting/report_query_template.rb | 68 +++++++++++++++++++ lib/reporting/report_renderer.rb | 22 +++--- lib/reporting/report_template.rb | 52 ++------------ .../reports/bulk_coop/bulk_coop_report.rb | 11 +-- .../reports/customers/customers_report.rb | 12 +--- .../enterprise_fee_summary_report.rb | 18 ++--- lib/reporting/reports/list.rb | 11 +++ .../order_cycle_management_report.rb | 22 ++---- .../orders_and_distributors_report.rb | 11 +-- .../orders_and_fulfillment_report.rb | 23 ++++--- lib/reporting/reports/packing/base.rb | 2 +- .../reports/payments/payments_report.rb | 12 +--- .../lettuce_share_report.rb | 2 - .../products_and_inventory_default_report.rb | 2 - .../products_and_inventory_report.rb | 10 +-- .../reports/sales_tax/sales_tax_report.rb | 11 +-- .../users_and_enterprises_report.rb | 11 +-- .../xero_invoices/xero_invoices_report.rb | 23 +++---- spec/lib/reports/bulk_coop_report_spec.rb | 16 ++--- spec/lib/reports/customers_report_spec.rb | 4 +- spec/lib/reports/lettuce_share_report_spec.rb | 2 +- .../order_cycle_management_report_spec.rb | 6 +- .../orders_and_distributors_report_spec.rb | 4 +- .../customer_totals_report_spec.rb | 2 +- ...tributor_totals_by_supplier_report_spec.rb | 2 +- .../orders_and_fulfillment_report_spec.rb | 8 +-- ...plier_totals_by_distributor_report_spec.rb | 2 +- .../supplier_totals_report_spec.rb | 2 +- ...ducts_and_inventory_default_report_spec.rb | 4 +- spec/lib/reports/report_renderer_spec.rb | 4 +- spec/lib/reports/sales_tax_report_spec.rb | 2 +- .../users_and_enterprises_report_spec.rb | 12 ++-- spec/lib/reports/xero_invoices_report_spec.rb | 4 +- 39 files changed, 224 insertions(+), 246 deletions(-) create mode 100644 lib/reporting/report_object_template.rb create mode 100644 lib/reporting/report_query_template.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index ae5ba7ddd7..65b2fa5266 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1145,16 +1145,6 @@ Style/OptionalBooleanParameter: - 'app/models/spree/preferences/file_configuration.rb' - 'app/models/spree/shipment.rb' - 'engines/order_management/app/services/order_management/stock/estimator.rb' - - 'lib/reporting/reports/bulk_coop/bulk_coop_report.rb' - - 'lib/reporting/reports/customers/customers_report.rb' - - 'lib/reporting/reports/enterprise_fee_summary/enterprise_fee_summary_report.rb' - - 'lib/reporting/reports/order_cycle_management/order_cycle_management_report.rb' - - 'lib/reporting/reports/orders_and_distributors/orders_and_distributors_report.rb' - - 'lib/reporting/reports/orders_and_fulfillment/orders_and_fulfillment_report.rb' - - 'lib/reporting/reports/payments/payments_report.rb' - - 'lib/reporting/reports/products_and_inventory/products_and_inventory_report.rb' - - 'lib/reporting/reports/users_and_enterprises/users_and_enterprises_report.rb' - - 'lib/reporting/reports/xero_invoices/xero_invoices_report.rb' - 'lib/spree/core/controller_helpers/order.rb' - 'lib/spree/core/delegate_belongs_to.rb' - 'spec/support/request/web_helper.rb' diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 31393a79a6..d097f6527b 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -22,7 +22,7 @@ module Admin private def export_report - send_data @report.public_send("to_#{report_format}"), filename: report_filename + send_data @report.public_send("to_#{report_format}", self), filename: report_filename end def render_report diff --git a/app/controllers/spree/admin/reports_controller.rb b/app/controllers/spree/admin/reports_controller.rb index 253ebbf45a..3f1be8c33f 100644 --- a/app/controllers/spree/admin/reports_controller.rb +++ b/app/controllers/spree/admin/reports_controller.rb @@ -59,12 +59,6 @@ module Spree end def orders_and_fulfillment - now = Time.zone.now - raw_params[:q] ||= { - completed_at_gt: (now - 1.month).beginning_of_day, - completed_at_lt: (now + 1.day).beginning_of_day - } - form_options = Reporting::FrontendData.new(spree_current_user) @distributors = form_options.distributors @@ -118,14 +112,10 @@ module Spree @report_subtypes = report_types[action_name.to_sym] @report_subtype = params[:report_subtype] klass = "Reporting::Reports::#{action_name.camelize}::#{action_name.camelize}Report".constantize - @report = klass.new spree_current_user, raw_params, render_content? + @report = klass.new spree_current_user, raw_params if report_format.present? - data = Reporting::ReportRenderer.new(@report).public_send("to_#{report_format}") - send_data data, filename: report_filename + send_data @report.public_send("to_#{report_format}"), filename: report_filename else - @header = @report.table_headers - @table = @report.table_rows - render "show" end end diff --git a/app/views/spree/admin/reports/_table.html.haml b/app/views/spree/admin/reports/_table.html.haml index 81d7dd524f..b6b932b4e7 100644 --- a/app/views/spree/admin/reports/_table.html.haml +++ b/app/views/spree/admin/reports/_table.html.haml @@ -2,10 +2,10 @@ %table.report__table %thead %tr - - @header.each do |heading| + - @report.table_headers.each do |heading| %th= heading %tbody - - @table.each do |row| + - @report.table_rows.each do |row| %tr - row.each_with_index do |cell_value, column_index| %td @@ -14,6 +14,6 @@ = render partial, value: cell_value - else = cell_value - - if @table.empty? + - if @report.table_rows.empty? %tr - %td{colspan: @header.count}= t(:none) + %td{colspan: @report.table_headers.count}= t(:none) diff --git a/app/views/spree/admin/reports/show.html.haml b/app/views/spree/admin/reports/show.html.haml index 70e0424593..5ea2ed207e 100644 --- a/app/views/spree/admin/reports/show.html.haml +++ b/app/views/spree/admin/reports/show.html.haml @@ -1,6 +1,5 @@ -/ If the report object do not use ransack search, create a fake one just for the form_for -- ransack_search = @report.respond_to?(:search) ? @report.search : Ransack::Search.new(Spree::Order) -= form_for ransack_search, :url => url_for(only_path: false) do |f| + += form_for @report.search, :url => url_for(only_path: false) do |f| %fieldset.no-border-bottom.print-hidden %legend{ align: 'center'}= t(:report_filters) diff --git a/lib/reporting/report_object_template.rb b/lib/reporting/report_object_template.rb new file mode 100644 index 0000000000..1eba55a0f9 --- /dev/null +++ b/lib/reporting/report_object_template.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# This is the old way of managing report, by loading Models from the DB and building +# The result from those models +module Reporting + class ReportObjectTemplate < ReportTemplate + + attr_accessor :user, :params + + def initialize(user, params = {}) + @user = user + @params = params + end + + def table_headers + raise NotImplementedError + end + + def table_rows + raise NotImplementedError + end + + # If the report object do not use ransack search, create a fake one just for the form_for + # in reports/show.haml + def search + Ransack::Search.new(Spree::Order) + end + + # Rules for grouping, ordering, and summary rows + def rules + [] + end + end +end diff --git a/lib/reporting/report_query_template.rb b/lib/reporting/report_query_template.rb new file mode 100644 index 0000000000..f4b79d22aa --- /dev/null +++ b/lib/reporting/report_query_template.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +# This is the new report template that use QueryBuilder to directly get the data from the DB +module Reporting + class ReportQueryTemplate < ReportTemplate + attr_reader :options + + SUBTYPES = [].freeze + + def self.report_subtypes + self::SUBTYPES + end + + def initialize(current_user, ransack_params, options = {}) + @current_user = current_user + @ransack_params = ( ransack_params || {} ).with_indifferent_access + @options = ( options || {} ).with_indifferent_access + end + + def report_data + @report_data ||= report_query.raw_result + end + + def report_query + raise NotImplementedError + end + + def table_headers + report_data.columns + end + + def table_rows + report_data.rows + end + + private + + attr_reader :current_user, :ransack_params + + def ransacked_orders_relation + visible_orders_relation.ransack(ransack_params).result + end + + def ransacked_line_items_relation + visible_line_items_relation.ransack(ransack_params).result + end + + def visible_orders_relation + ::Permissions::Order.new(current_user). + visible_orders.complete.not_state(:canceled). + select(:id).distinct + end + + def visible_line_items_relation + ::Permissions::Order.new(current_user). + visible_line_items. + select(:id).distinct + end + + def managed_orders_relation + ::Enterprise.managed_by(current_user).select(:id).distinct + end + + def i18n_scope + "admin.reports" + end + end +end diff --git a/lib/reporting/report_renderer.rb b/lib/reporting/report_renderer.rb index a4cfcc1d6d..998b5529d1 100644 --- a/lib/reporting/report_renderer.rb +++ b/lib/reporting/report_renderer.rb @@ -9,39 +9,43 @@ module Reporting end def table_headers - @report.respond_to?(:report_data) ? @report.report_data.columns : @report.table_headers + @report.table_headers || [] end def table_rows - @report.respond_to?(:report_data) ? @report.report_data.rows : @report.table_rows + @report.table_rows || [] end def as_json - @report.report_data.as_json + table_rows.map do |row| + result = {} + table_headers.zip(row) { |a,b| result[a.to_sym] = b } + result + end.as_json end def as_arrays @as_arrays ||= [table_headers] + table_rows end - def to_csv + def to_csv(_context_controller = nil) SpreadsheetArchitect.to_csv(headers: table_headers, data: table_rows) end - def to_ods + def to_ods(_context_controller = nil) SpreadsheetArchitect.to_ods(headers: table_headers, data: table_rows) end - def to_xlsx + def to_xlsx(_context_controller = nil) SpreadsheetArchitect.to_xlsx(headers: table_headers, data: table_rows) end - def to_pdf + def to_pdf(context_controller) WickedPdf.new.pdf_from_string( - ActionController::Base.new.render_to_string( + context_controller.render_to_string( template: 'admin/reports/_table', layout: 'pdf', - locals: { report: self } + locals: { report: @report } ) ) end diff --git a/lib/reporting/report_template.rb b/lib/reporting/report_template.rb index 3192a56ae4..91c3fb73b2 100644 --- a/lib/reporting/report_template.rb +++ b/lib/reporting/report_template.rb @@ -2,61 +2,23 @@ module Reporting class ReportTemplate - delegate :as_json, :as_arrays, :table_headers, :table_rows, - :to_csv, :to_xlsx, :to_ods, :to_pdf, :to_json, to: :renderer + include ReportsHelper + attr_accessor :user, :params, :ransack_params - attr_reader :options + delegate :as_json, :as_arrays, :to_csv, :to_xlsx, :to_ods, :to_pdf, :to_json, to: :renderer - SUBTYPES = [] - - def self.report_subtypes - self::SUBTYPES + def table_headers + raise NotImplementedError end - def initialize(current_user, ransack_params, options = {}) - @current_user = current_user - @ransack_params = ransack_params.with_indifferent_access - @options = ( options || {} ).with_indifferent_access - end - - def report_data - @report_data ||= report_query.raw_result + def table_rows + raise NotImplementedError end private - attr_reader :current_user, :ransack_params - def renderer @renderer ||= ReportRenderer.new(self) end - - def ransacked_orders_relation - visible_orders_relation.ransack(ransack_params).result - end - - def ransacked_line_items_relation - visible_line_items_relation.ransack(ransack_params).result - end - - def visible_orders_relation - ::Permissions::Order.new(current_user). - visible_orders.complete.not_state(:canceled). - select(:id).distinct - end - - def visible_line_items_relation - ::Permissions::Order.new(current_user). - visible_line_items. - select(:id).distinct - end - - def managed_orders_relation - ::Enterprise.managed_by(current_user).select(:id).distinct - end - - def i18n_scope - "admin.reports" - end end end diff --git a/lib/reporting/reports/bulk_coop/bulk_coop_report.rb b/lib/reporting/reports/bulk_coop/bulk_coop_report.rb index d30641f5df..be15a9bdd4 100644 --- a/lib/reporting/reports/bulk_coop/bulk_coop_report.rb +++ b/lib/reporting/reports/bulk_coop/bulk_coop_report.rb @@ -3,13 +3,10 @@ module Reporting module Reports module BulkCoop - class BulkCoopReport - attr_reader :params + class BulkCoopReport < ReportObjectTemplate - def initialize(user, params = {}, render_table = false) - @params = params - @user = user - @render_table = render_table + def initialize(user, params = {}) + super(user, params) @supplier_report = BulkCoopSupplierReport.new @allocation_report = BulkCoopAllocationReport.new @@ -52,8 +49,6 @@ module Reporting end def table_items - return [] unless @render_table - report_line_items.list(line_item_includes) end diff --git a/lib/reporting/reports/customers/customers_report.rb b/lib/reporting/reports/customers/customers_report.rb index 0fa040646b..323bf97551 100644 --- a/lib/reporting/reports/customers/customers_report.rb +++ b/lib/reporting/reports/customers/customers_report.rb @@ -3,15 +3,7 @@ module Reporting module Reports module Customers - class CustomersReport - attr_reader :params - - def initialize(user, params = {}, compile_table = false) - @params = params - @user = user - @compile_table = compile_table - end - + class CustomersReport < ReportObjectTemplate def table_headers if is_mailing_list? [I18n.t(:report_header_email), @@ -31,8 +23,6 @@ module Reporting end def table_rows - return [] unless @compile_table - orders.map do |order| if is_mailing_list? [order.email, diff --git a/lib/reporting/reports/enterprise_fee_summary/enterprise_fee_summary_report.rb b/lib/reporting/reports/enterprise_fee_summary/enterprise_fee_summary_report.rb index c8bdd7895a..4298533d8e 100644 --- a/lib/reporting/reports/enterprise_fee_summary/enterprise_fee_summary_report.rb +++ b/lib/reporting/reports/enterprise_fee_summary/enterprise_fee_summary_report.rb @@ -3,10 +3,11 @@ module Reporting module Reports module EnterpriseFeeSummary - class EnterpriseFeeSummaryReport - attr_accessor :permissions, :parameters, :user + class EnterpriseFeeSummaryReport < ReportObjectTemplate + attr_accessor :permissions, :parameters - def initialize(user, params = {}, render_table = false) + def initialize(user, params = {}) + super(user, params) p = params[:q] if p.present? p['start_at'] = p.delete('completed_at_gt') @@ -14,8 +15,6 @@ module Reporting end @parameters = Reporting::Reports::EnterpriseFeeSummary::Parameters.new(p || {}) @parameters.validate! - @user = user - @render_table = render_table @permissions = Permissions.new(user) @parameters.authorize!(@permissions) end @@ -27,8 +26,6 @@ module Reporting end def table_rows - return [] unless @render_table - enterprise_fee_type_total_list.sort.map do |data| data_row_attributes.map do |attribute| data.public_send(attribute) @@ -36,13 +33,6 @@ module Reporting end end - # This report does not use ransack search, but all other are, so creating a fake - # Ransack search at least for the view to display correctly the selected - # ransack params like completed_at_gt and completed_at_lt - def search - Spree::Order.where('1=2').ransack(parameters) - end - private def data_row_attributes diff --git a/lib/reporting/reports/list.rb b/lib/reporting/reports/list.rb index d9ab57b945..917282b7db 100644 --- a/lib/reporting/reports/list.rb +++ b/lib/reporting/reports/list.rb @@ -9,6 +9,9 @@ module Reporting def all { + orders_and_distributors: [], + bulk_coop: bulk_coop_report_types, + payments: payments_report_types, orders_and_fulfillment: orders_and_fulfillment_report_types, products_and_inventory: products_and_inventory_report_types, customers: customers_report_types, @@ -41,6 +44,14 @@ module Reporting ] end + def payments_report_types + [ + [I18n.t(:report_payment_by), :payments_by_payment_type], + [I18n.t(:report_itemised_payment), :itemised_payment_totals], + [I18n.t(:report_payment_totals), :payment_totals] + ] + end + def customers_report_types [ [i18n_translate("mailing_list"), :mailing_list], diff --git a/lib/reporting/reports/order_cycle_management/order_cycle_management_report.rb b/lib/reporting/reports/order_cycle_management/order_cycle_management_report.rb index 1f095f4982..9236725549 100644 --- a/lib/reporting/reports/order_cycle_management/order_cycle_management_report.rb +++ b/lib/reporting/reports/order_cycle_management/order_cycle_management_report.rb @@ -3,15 +3,14 @@ module Reporting module Reports module OrderCycleManagement - class OrderCycleManagementReport + class OrderCycleManagementReport < ReportObjectTemplate DEFAULT_DATE_INTERVAL = { from: -1.month, to: 1.day }.freeze - attr_reader :params - - def initialize(user, params = {}, render_table = false) - @params = sanitize_params(params) - @user = user - @render_table = render_table + def initialize(user, params = {}) + super(user, params) + params[:q] ||= {} + params[:q][:completed_at_gt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:from] + params[:q][:completed_at_lt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:to] end def table_headers @@ -66,8 +65,6 @@ module Reporting end def table_rows - return [] unless @render_table - if is_payment_methods? orders.map { |o| payment_method_row o } else @@ -157,13 +154,6 @@ module Reporting customer = Customer.where(email: email).first customer.nil? ? "" : customer.code end - - def sanitize_params(params) - params[:q] ||= {} - params[:q][:completed_at_gt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:from] - params[:q][:completed_at_lt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:to] - params - end end end end diff --git a/lib/reporting/reports/orders_and_distributors/orders_and_distributors_report.rb b/lib/reporting/reports/orders_and_distributors/orders_and_distributors_report.rb index 5f5ef599c1..afd1120412 100644 --- a/lib/reporting/reports/orders_and_distributors/orders_and_distributors_report.rb +++ b/lib/reporting/reports/orders_and_distributors/orders_and_distributors_report.rb @@ -3,12 +3,9 @@ module Reporting module Reports module OrdersAndDistributors - class OrdersAndDistributorsReport - def initialize(user, params = {}, render_table = false) - @params = params - @user = user - @render_table = render_table - + class OrdersAndDistributorsReport < ReportObjectTemplate + def initialize(user, params = {}) + super(user, params) @permissions = ::Permissions::Order.new(user, @params[:q]) end @@ -44,8 +41,6 @@ module Reporting end def table_rows - return [] unless @render_table - orders = search.result orders.select{ |order| orders_with_hidden_details(orders).include? order }.each do |order| diff --git a/lib/reporting/reports/orders_and_fulfillment/orders_and_fulfillment_report.rb b/lib/reporting/reports/orders_and_fulfillment/orders_and_fulfillment_report.rb index 27a5bfa1e5..d99767f474 100644 --- a/lib/reporting/reports/orders_and_fulfillment/orders_and_fulfillment_report.rb +++ b/lib/reporting/reports/orders_and_fulfillment/orders_and_fulfillment_report.rb @@ -5,17 +5,20 @@ include Spree::ReportsHelper module Reporting module Reports module OrdersAndFulfillment - class OrdersAndFulfillmentReport - attr_reader :options, :report_type + class OrdersAndFulfillmentReport < ReportObjectTemplate + attr_reader :report_type delegate :table_headers, :rules, :columns, to: :report - def initialize(user, options = {}, render_table = false) - @user = user - @options = options - @report_type = options[:report_subtype] - @render_table = render_table + def initialize(user, params = {}) + super(user, params) + @report_type = params[:report_subtype] @variant_scopers_by_distributor_id = {} + now = Time.zone.now + params[:q] ||= { + completed_at_gt: (now - 1.month).beginning_of_day, + completed_at_lt: (now + 1.day).beginning_of_day + } end def search @@ -23,8 +26,6 @@ module Reporting end def table_items - return [] unless @render_table - report_line_items.list(report.line_item_includes) end @@ -96,11 +97,11 @@ module Reporting def order_permissions return @order_permissions unless @order_permissions.nil? - @order_permissions = ::Permissions::Order.new(@user, options[:q]) + @order_permissions = ::Permissions::Order.new(@user, params[:q]) end def report_line_items - @report_line_items ||= Reporting::LineItems.new(order_permissions, options) + @report_line_items ||= Reporting::LineItems.new(order_permissions, params) end def report_variant_overrides diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index 7102d9d553..5518906120 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -3,7 +3,7 @@ module Reporting module Reports module Packing - class Base < ReportTemplate + class Base < ReportQueryTemplate SUBTYPES = ["customer", "supplier"] def primary_model diff --git a/lib/reporting/reports/payments/payments_report.rb b/lib/reporting/reports/payments/payments_report.rb index 3d2a21a6b3..c7f02b6b12 100644 --- a/lib/reporting/reports/payments/payments_report.rb +++ b/lib/reporting/reports/payments/payments_report.rb @@ -3,15 +3,7 @@ module Reporting module Reports module Payments - class PaymentsReport - attr_reader :params - - def initialize(user, params = {}, render_table = false) - @params = params - @user = user - @render_table = render_table - end - + class PaymentsReport < ReportObjectTemplate def table_headers case params[:report_subtype] when "payments_by_payment_type" @@ -43,8 +35,6 @@ module Reporting end def table_items - return [] unless @render_table - orders = search.result payments = orders.includes(:payments).map do |order| order.payments.select(&:completed?) diff --git a/lib/reporting/reports/products_and_inventory/lettuce_share_report.rb b/lib/reporting/reports/products_and_inventory/lettuce_share_report.rb index 2c63e04a8c..8dd9ba1797 100644 --- a/lib/reporting/reports/products_and_inventory/lettuce_share_report.rb +++ b/lib/reporting/reports/products_and_inventory/lettuce_share_report.rb @@ -31,8 +31,6 @@ module Reporting end def table_rows - return [] unless render_table - variants.select(&:in_stock?) .map do |variant| [ diff --git a/lib/reporting/reports/products_and_inventory/products_and_inventory_default_report.rb b/lib/reporting/reports/products_and_inventory/products_and_inventory_default_report.rb index a733fe24ea..be54824b9c 100644 --- a/lib/reporting/reports/products_and_inventory/products_and_inventory_default_report.rb +++ b/lib/reporting/reports/products_and_inventory/products_and_inventory_default_report.rb @@ -28,8 +28,6 @@ module Reporting end def table_rows - return [] unless render_table - variants.map do |variant| [ variant.product.supplier.name, diff --git a/lib/reporting/reports/products_and_inventory/products_and_inventory_report.rb b/lib/reporting/reports/products_and_inventory/products_and_inventory_report.rb index b6f972f3b7..b8dc78977b 100644 --- a/lib/reporting/reports/products_and_inventory/products_and_inventory_report.rb +++ b/lib/reporting/reports/products_and_inventory/products_and_inventory_report.rb @@ -5,17 +5,9 @@ require 'open_food_network/scope_variant_to_hub' module Reporting module Reports module ProductsAndInventory - class ProductsAndInventoryReport - attr_reader :params, :render_table - + class ProductsAndInventoryReport < ReportObjectTemplate delegate :table_rows, :table_headers, :rules, :columns, :sku_for, to: :report - def initialize(user, params = {}, render_table = false) - @user = user - @params = params - @render_table = render_table - end - def report @report ||= report_klass.new(self) end diff --git a/lib/reporting/reports/sales_tax/sales_tax_report.rb b/lib/reporting/reports/sales_tax/sales_tax_report.rb index 9bf0248415..ad07846920 100644 --- a/lib/reporting/reports/sales_tax/sales_tax_report.rb +++ b/lib/reporting/reports/sales_tax/sales_tax_report.rb @@ -3,15 +3,8 @@ module Reporting module Reports module SalesTax - class SalesTaxReport + class SalesTaxReport < ReportObjectTemplate include Spree::ReportsHelper - attr_accessor :user, :params - - def initialize(user, params, render_table) - @user = user - @params = params - @render_table = render_table - end def table_headers case params[:report_subtype] @@ -49,8 +42,6 @@ module Reporting end def table_rows - return [] unless @render_table - case params[:report_subtype] when "tax_rates" orders.map do |order| diff --git a/lib/reporting/reports/users_and_enterprises/users_and_enterprises_report.rb b/lib/reporting/reports/users_and_enterprises/users_and_enterprises_report.rb index 34c86e97f2..dade08a64b 100644 --- a/lib/reporting/reports/users_and_enterprises/users_and_enterprises_report.rb +++ b/lib/reporting/reports/users_and_enterprises/users_and_enterprises_report.rb @@ -3,13 +3,10 @@ module Reporting module Reports module UsersAndEnterprises - class UsersAndEnterprisesReport - attr_reader :params + class UsersAndEnterprisesReport < ReportObjectTemplate - def initialize(user, params = {}, compile_table = false) - @user = user - @params = params - @compile_table = compile_table + def initialize(user, params = {}) + super(user, params) # Convert arrays of ids to comma delimited strings if @params[:enterprise_id_in].is_a? Array @@ -31,8 +28,6 @@ module Reporting end def table_rows - return [] unless @compile_table - users_and_enterprises.map do |uae| [ uae["user_email"], diff --git a/lib/reporting/reports/xero_invoices/xero_invoices_report.rb b/lib/reporting/reports/xero_invoices/xero_invoices_report.rb index 186dd2e830..2da61f0f78 100644 --- a/lib/reporting/reports/xero_invoices/xero_invoices_report.rb +++ b/lib/reporting/reports/xero_invoices/xero_invoices_report.rb @@ -3,18 +3,17 @@ module Reporting module Reports module XeroInvoices - class XeroInvoicesReport - def initialize(user, opts = {}, compile_table = false) - @user = user + class XeroInvoicesReport < ReportObjectTemplate + def initialize(user, params = {}) + super(user, params) - @opts = opts. + @params = @params. symbolize_keys. reject { |_k, v| v.blank? }. reverse_merge( report_subtype: 'summary', invoice_date: Time.zone.today, due_date: Time.zone.today + 1.month, account_code: 'food sales' ) - @compile_table = compile_table end def table_headers @@ -25,7 +24,7 @@ module Reporting def search permissions = ::Permissions::Order.new(@user) - permissions.editable_orders.complete.not_state(:canceled).ransack(@opts[:q]) + permissions.editable_orders.complete.not_state(:canceled).ransack(params[:q]) end def orders @@ -33,14 +32,12 @@ module Reporting end def table_rows - return [] unless @compile_table - rows = [] orders.each_with_index do |order, i| invoice_number = invoice_number_for(order, i) - rows += detail_rows_for_order(order, invoice_number, @opts) if detail? - rows += summary_rows_for_order(order, invoice_number, @opts) + rows += detail_rows_for_order(order, invoice_number, params) if detail? + rows += summary_rows_for_order(order, invoice_number, params) end rows.compact @@ -49,7 +46,7 @@ module Reporting private def report_options - @opts.merge(line_item_includes: line_item_includes) + params.merge(line_item_includes: line_item_includes) end def line_item_includes @@ -186,7 +183,7 @@ module Reporting end def invoice_number_for(order, idx) - @opts[:initial_invoice_number] ? @opts[:initial_invoice_number].to_i + idx : order.number + params[:initial_invoice_number] ? params[:initial_invoice_number].to_i + idx : order.number end def total_untaxable_products(order) @@ -227,7 +224,7 @@ module Reporting end def detail? - @opts[:report_subtype] == 'detailed' + params[:report_subtype] == 'detailed' end def tax_type(taxable) diff --git a/spec/lib/reports/bulk_coop_report_spec.rb b/spec/lib/reports/bulk_coop_report_spec.rb index 80f118a03a..49ed7d861d 100644 --- a/spec/lib/reports/bulk_coop_report_spec.rb +++ b/spec/lib/reports/bulk_coop_report_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Reporting::Reports::BulkCoop::BulkCoopReport do - subject { Reporting::Reports::BulkCoop::BulkCoopReport.new user, params, true } + subject { Reporting::Reports::BulkCoop::BulkCoopReport.new user, params } let(:user) { create(:admin_user) } describe '#table_items' do @@ -61,16 +61,16 @@ describe Reporting::Reports::BulkCoop::BulkCoopReport do li2 = build(:line_item_with_shipment) o2.line_items << li2 - report = Reporting::Reports::BulkCoop::BulkCoopReport.new user, {}, true + report = Reporting::Reports::BulkCoop::BulkCoopReport.new user, {} expect(report.table_items).to match_array [li1, li2] report = Reporting::Reports::BulkCoop::BulkCoopReport.new( - user, { q: { completed_at_gt: 2.days.ago } }, true + user, { q: { completed_at_gt: 2.days.ago } } ) expect(report.table_items).to eq([li1]) report = Reporting::Reports::BulkCoop::BulkCoopReport.new( - user, { q: { completed_at_lt: 2.days.ago } }, true + user, { q: { completed_at_lt: 2.days.ago } } ) expect(report.table_items).to eq([li2]) end @@ -85,16 +85,16 @@ describe Reporting::Reports::BulkCoop::BulkCoopReport do li2 = build(:line_item_with_shipment) o2.line_items << li2 - report = Reporting::Reports::BulkCoop::BulkCoopReport.new user, {}, true + report = Reporting::Reports::BulkCoop::BulkCoopReport.new user, {} expect(report.table_items).to match_array [li1, li2] report = Reporting::Reports::BulkCoop::BulkCoopReport.new( - user, { q: { distributor_id_in: [d1.id] } }, true + user, { q: { distributor_id_in: [d1.id] } } ) expect(report.table_items).to eq([li1]) report = Reporting::Reports::BulkCoop::BulkCoopReport.new( - user, { q: { distributor_id_in: [d2.id] } }, true + user, { q: { distributor_id_in: [d2.id] } } ) expect(report.table_items).to eq([li2]) end @@ -102,7 +102,7 @@ describe Reporting::Reports::BulkCoop::BulkCoopReport do context "as a manager of a supplier" do let!(:user) { create(:user) } - subject { Reporting::Reports::BulkCoop::BulkCoopReport.new user, {}, true } + subject { Reporting::Reports::BulkCoop::BulkCoopReport.new user, {} } let(:s1) { create(:supplier_enterprise) } diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 2ed1e950b5..dc73ac7617 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -12,7 +12,7 @@ module Reporting user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') user end - subject { CustomersReport.new user, {}, true } + subject { CustomersReport.new user, {} } describe "mailing list report" do before do @@ -86,7 +86,7 @@ module Reporting user end - subject { CustomersReport.new user, {}, true } + subject { CustomersReport.new user, {} } describe "fetching orders" do let(:supplier) { create(:supplier_enterprise) } diff --git a/spec/lib/reports/lettuce_share_report_spec.rb b/spec/lib/reports/lettuce_share_report_spec.rb index 6fd5b8fca5..f0c1e3d554 100644 --- a/spec/lib/reports/lettuce_share_report_spec.rb +++ b/spec/lib/reports/lettuce_share_report_spec.rb @@ -8,7 +8,7 @@ module Reporting describe LettuceShareReport do let(:user) { create(:user) } let(:base_report) { - ProductsAndInventoryReport.new(user, { report_subtype: 'lettuce_share' }, true) + ProductsAndInventoryReport.new(user, { report_subtype: 'lettuce_share' }) } let(:report) { base_report.report } let(:variant) { create(:variant) } diff --git a/spec/lib/reports/order_cycle_management_report_spec.rb b/spec/lib/reports/order_cycle_management_report_spec.rb index 7cd8160ab0..9cdb4846e8 100644 --- a/spec/lib/reports/order_cycle_management_report_spec.rb +++ b/spec/lib/reports/order_cycle_management_report_spec.rb @@ -7,7 +7,7 @@ module Reporting module OrderCycleManagement describe OrderCycleManagementReport do context "as a site admin" do - subject { OrderCycleManagementReport.new(user, params, true) } + subject { OrderCycleManagementReport.new(user, params) } let(:params) { {} } let(:user) do @@ -63,7 +63,7 @@ module Reporting context "as an enterprise user" do let!(:user) { create(:user) } - subject { OrderCycleManagementReport.new user, {}, true } + subject { OrderCycleManagementReport.new user, {} } describe "fetching orders" do let(:supplier) { create(:supplier_enterprise) } @@ -148,7 +148,7 @@ module Reporting end describe '#table_rows' do - subject { OrderCycleManagementReport.new(user, params, true) } + subject { OrderCycleManagementReport.new(user, params) } let(:distributor) { create(:distributor_enterprise) } before { distributor.enterprise_roles.create!(user: user) } diff --git a/spec/lib/reports/orders_and_distributors_report_spec.rb b/spec/lib/reports/orders_and_distributors_report_spec.rb index ca9a8eaaa6..a1a6eb196a 100644 --- a/spec/lib/reports/orders_and_distributors_report_spec.rb +++ b/spec/lib/reports/orders_and_distributors_report_spec.rb @@ -45,7 +45,7 @@ module Reporting end it 'should denormalise order and distributor details for display as csv' do - subject = OrdersAndDistributorsReport.new create(:admin_user), {}, true + subject = OrdersAndDistributorsReport.new create(:admin_user), {} table = subject.table_rows @@ -77,7 +77,7 @@ module Reporting it "prints one row per line item" do create(:line_item_with_shipment, order: order) - subject = OrdersAndDistributorsReport.new(create(:admin_user), {}, true) + subject = OrdersAndDistributorsReport.new(create(:admin_user)) table = subject.table_rows expect(table.size).to eq 2 diff --git a/spec/lib/reports/orders_and_fulfillment/customer_totals_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/customer_totals_report_spec.rb index 6c990d4bc0..a0a1ff5184 100644 --- a/spec/lib/reports/orders_and_fulfillment/customer_totals_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/customer_totals_report_spec.rb @@ -12,7 +12,7 @@ module Reporting let(:report) do report_options = { report_subtype: described_class::REPORT_TYPE } - OrdersAndFulfillmentReport.new(current_user, report_options, true) + OrdersAndFulfillmentReport.new(current_user, report_options) end let(:report_table) do diff --git a/spec/lib/reports/orders_and_fulfillment/distributor_totals_by_supplier_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/distributor_totals_by_supplier_report_spec.rb index 665cce0c67..e8a0fa3864 100644 --- a/spec/lib/reports/orders_and_fulfillment/distributor_totals_by_supplier_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/distributor_totals_by_supplier_report_spec.rb @@ -16,7 +16,7 @@ module Reporting let(:report) do report_options = { report_subtype: described_class::REPORT_TYPE } - OrdersAndFulfillmentReport.new(current_user, report_options, true) + OrdersAndFulfillmentReport.new(current_user, report_options) end let(:report_table) do diff --git a/spec/lib/reports/orders_and_fulfillment/orders_and_fulfillment_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/orders_and_fulfillment_report_spec.rb index 7d20f85638..5d6bc377ee 100644 --- a/spec/lib/reports/orders_and_fulfillment/orders_and_fulfillment_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/orders_and_fulfillment_report_spec.rb @@ -28,7 +28,7 @@ module Reporting before { order.line_items << line_item } context "as a site admin" do - subject { described_class.new(admin_user, {}, true) } + subject { described_class.new(admin_user, {}) } it "fetches completed orders" do o2 = create(:order) @@ -44,7 +44,7 @@ module Reporting end context "as a manager of a supplier" do - subject { described_class.new(user, {}, true) } + subject { described_class.new(user, {}) } let(:s1) { create(:supplier_enterprise) } @@ -129,7 +129,7 @@ module Reporting end context "as a manager of a distributor" do - subject { described_class.new(user, {}, true) } + subject { described_class.new(user, {}) } before do distributor.enterprise_roles.create!(user: user) @@ -180,7 +180,7 @@ module Reporting end let(:items) { - described_class.new(admin_user, { report_subtype: "order_cycle_customer_totals" }, true) + described_class.new(admin_user, { report_subtype: "order_cycle_customer_totals" }) .table_rows } diff --git a/spec/lib/reports/orders_and_fulfillment/supplier_totals_by_distributor_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/supplier_totals_by_distributor_report_spec.rb index 26fd0b7d31..b265e6c338 100644 --- a/spec/lib/reports/orders_and_fulfillment/supplier_totals_by_distributor_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/supplier_totals_by_distributor_report_spec.rb @@ -16,7 +16,7 @@ module Reporting let(:report) do report_options = { report_subtype: described_class::REPORT_TYPE } - OrdersAndFulfillmentReport.new(current_user, report_options, true) + OrdersAndFulfillmentReport.new(current_user, report_options) end let(:report_table) do diff --git a/spec/lib/reports/orders_and_fulfillment/supplier_totals_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/supplier_totals_report_spec.rb index d21839bed7..bbf2ad2ec4 100644 --- a/spec/lib/reports/orders_and_fulfillment/supplier_totals_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/supplier_totals_report_spec.rb @@ -16,7 +16,7 @@ module Reporting let(:report) do report_options = { report_subtype: described_class::REPORT_TYPE } - OrdersAndFulfillmentReport.new(current_user, report_options, true) + OrdersAndFulfillmentReport.new(current_user, report_options) end let(:report_table) do diff --git a/spec/lib/reports/products_and_inventory_default_report_spec.rb b/spec/lib/reports/products_and_inventory_default_report_spec.rb index 32f42f0c8d..f57a775844 100644 --- a/spec/lib/reports/products_and_inventory_default_report_spec.rb +++ b/spec/lib/reports/products_and_inventory_default_report_spec.rb @@ -13,7 +13,7 @@ module Reporting user end subject do - ProductsAndInventoryReport.new user, {}, true + ProductsAndInventoryReport.new user, {} end it "Should return headers" do @@ -82,7 +82,7 @@ module Reporting end subject do - ProductsAndInventoryReport.new enterprise_user, {}, true + ProductsAndInventoryReport.new enterprise_user, {} end describe "fetching child variants" do diff --git a/spec/lib/reports/report_renderer_spec.rb b/spec/lib/reports/report_renderer_spec.rb index 2377c96950..b7ad4c0565 100644 --- a/spec/lib/reports/report_renderer_spec.rb +++ b/spec/lib/reports/report_renderer_spec.rb @@ -9,9 +9,7 @@ describe Reporting::ReportRenderer do { "id" => 2, "name" => "onions", "quantity" => 6 } ] } - let(:report_data) { ActiveRecord::Result.new(data.first.keys, data.map(&:values)) } - let(:report) { OpenStruct.new(report_data: report_data) - } + let(:report) { OpenStruct.new(table_headers: data.first.keys, table_rows: data.map(&:values)) } let(:service) { described_class.new(report) } describe "#table_headers" do diff --git a/spec/lib/reports/sales_tax_report_spec.rb b/spec/lib/reports/sales_tax_report_spec.rb index f1695c2d4a..f316da1f48 100644 --- a/spec/lib/reports/sales_tax_report_spec.rb +++ b/spec/lib/reports/sales_tax_report_spec.rb @@ -7,7 +7,7 @@ module Reporting module SalesTax describe SalesTaxReport do let(:user) { create(:user) } - let(:report) { SalesTaxReport.new(user, {}, true) } + let(:report) { SalesTaxReport.new(user, {}) } describe "calculating totals for line items" do let(:li1) { double(:line_item, quantity: 1, amount: 12) } diff --git a/spec/lib/reports/users_and_enterprises_report_spec.rb b/spec/lib/reports/users_and_enterprises_report_spec.rb index b967ed3d5c..9929a4cb04 100644 --- a/spec/lib/reports/users_and_enterprises_report_spec.rb +++ b/spec/lib/reports/users_and_enterprises_report_spec.rb @@ -9,7 +9,7 @@ module Reporting describe "users_and_enterprises" do let!(:owners_and_enterprises) { double(:owners_and_enterprises) } let!(:managers_and_enterprises) { double(:managers_and_enterprises) } - let!(:subject) { UsersAndEnterprisesReport.new(nil, {}, true) } + let!(:subject) { UsersAndEnterprisesReport.new(nil, {}) } before do allow(subject).to receive(:owners_and_enterprises) { owners_and_enterprises } @@ -26,7 +26,7 @@ module Reporting end describe "sorting results" do - let!(:subject) { UsersAndEnterprisesReport.new(nil, {}, true) } + let!(:subject) { UsersAndEnterprisesReport.new(nil, {}) } it "sorts by creation date" do uae_mock = [ @@ -70,7 +70,7 @@ module Reporting describe "for owners and enterprises" do describe "by enterprise id" do let!(:params) { { enterprise_id_in: [enterprise1.id.to_s] } } - let!(:subject) { UsersAndEnterprisesReport.new nil, params, true } + let!(:subject) { UsersAndEnterprisesReport.new nil, params } it "excludes enterprises that are not explicitly requested" do results = subject.owners_and_enterprises.to_a.map{ |oae| oae["name"] } @@ -81,7 +81,7 @@ module Reporting describe "by user id" do let!(:params) { { user_id_in: [enterprise1.owner.id.to_s] } } - let!(:subject) { UsersAndEnterprisesReport.new nil, params, true } + let!(:subject) { UsersAndEnterprisesReport.new nil, params } it "excludes enterprises that are not explicitly requested" do results = subject.owners_and_enterprises.to_a.map{ |oae| oae["name"] } @@ -94,7 +94,7 @@ module Reporting describe "for managers and enterprises" do describe "by enterprise id" do let!(:params) { { enterprise_id_in: [enterprise1.id.to_s] } } - let!(:subject) { UsersAndEnterprisesReport.new nil, params, true } + let!(:subject) { UsersAndEnterprisesReport.new nil, params } it "excludes enterprises that are not explicitly requested" do results = subject.managers_and_enterprises.to_a.map{ |mae| mae["name"] } @@ -107,7 +107,7 @@ module Reporting let!(:manager1) { create(:user) } let!(:manager2) { create(:user) } let!(:params) { { user_id_in: [manager1.id.to_s] } } - let!(:subject) { UsersAndEnterprisesReport.new nil, params, true } + let!(:subject) { UsersAndEnterprisesReport.new nil, params } before do enterprise1.enterprise_roles.build(user: manager1).save diff --git a/spec/lib/reports/xero_invoices_report_spec.rb b/spec/lib/reports/xero_invoices_report_spec.rb index a5ad21e0cc..4d63acdad4 100644 --- a/spec/lib/reports/xero_invoices_report_spec.rb +++ b/spec/lib/reports/xero_invoices_report_spec.rb @@ -6,7 +6,7 @@ module Reporting module Reports module XeroInvoices describe XeroInvoicesReport do - subject { XeroInvoicesReport.new user, {}, true } + subject { XeroInvoicesReport.new user, {} } let(:user) { create(:user) } @@ -19,7 +19,7 @@ module Reporting around { |example| Timecop.travel(Time.zone.local(2015, 5, 5, 14, 0, 0)) { example.run } } it "uses defaults when blank params are passed" do - expect(report.instance_variable_get(:@opts)).to eq( invoice_date: Date.civil(2015, 5, 5), + expect(report.instance_variable_get(:@params)).to eq( invoice_date: Date.civil(2015, 5, 5), due_date: Date.civil(2015, 6, 5), account_code: 'food sales', report_subtype: 'summary' )