From 452a3fa9332a6732d0084d4a0db7f023c9b79f0e Mon Sep 17 00:00:00 2001 From: Sebastian Castro Date: Thu, 31 Mar 2022 09:49:08 +0200 Subject: [PATCH] Reports Refactor 2: Merge Spree::reports into Admin::Reports --- app/controllers/admin/reports_controller.rb | 37 ++-- app/controllers/api/v0/reports_controller.rb | 2 +- app/controllers/concerns/reports_actions.rb | 25 ++- .../spree/admin/reports_controller.rb | 198 ------------------ app/helpers/reports_helper.rb | 21 +- app/helpers/spree/admin/navigation_helper.rb | 2 +- app/helpers/spree/reports_helper.rb | 28 --- app/models/spree/ability.rb | 31 +-- .../admin/reports/_date_range_form.html.haml | 11 +- app/views/admin/reports/_table.html.haml | 1 + .../reports/filters/_bulk_coop.html.haml | 6 + .../reports/filters/_customers.html.haml | 6 +- .../filters/_enterprise_fee_summary.html.haml | 2 +- .../filters/_order_cycle_management.html.haml | 6 +- .../_orders_and_distributors.html.haml | 1 + .../filters/_orders_and_fulfillment.html.haml | 14 ++ .../admin/reports/filters/_packing.html.haml | 8 +- .../admin/reports/filters/_payments.html.haml | 6 + .../filters/_products_and_inventory.html.haml | 6 +- .../reports/filters/_sales_tax.html.haml | 6 + .../filters/_users_and_enterprises.html.haml | 0 .../reports/filters/_xero_invoices.html.haml | 6 +- app/views/admin/reports/index.html.haml | 25 +++ app/views/admin/reports/show.html.haml | 12 +- .../reports/_customer_names_message.html.haml | 2 - .../reports/_customers_description.html.haml | 4 - .../admin/reports/_date_range_form.html.haml | 7 - .../spree/admin/reports/_link_order.html.haml | 2 - ...der_cycle_management_description.html.haml | 4 - ...ders_and_fulfillment_description.html.haml | 4 - .../reports/_packing_description.html.haml | 5 - ...oducts_and_inventory_description.html.haml | 4 - .../reports/_sales_tax_description.html.haml | 4 - .../spree/admin/reports/_table.html.haml | 19 -- .../reports/filters/_bulk_coop.html.haml | 6 - .../_orders_and_distributors.html.haml | 1 - .../filters/_orders_and_fulfillment.html.haml | 14 -- .../admin/reports/filters/_payments.html.haml | 6 - .../reports/filters/_sales_tax.html.haml | 6 - app/views/spree/admin/reports/index.html.haml | 14 -- app/views/spree/admin/reports/show.html.haml | 19 -- app/views/spree/admin/shared/_tabs.html.haml | 2 +- config/locales/en.yml | 3 - config/routes/admin.rb | 3 +- config/routes/spree.rb | 15 -- lib/reporting/frontend_data.rb | 31 ++- lib/reporting/report_loader.rb | 29 +-- lib/reporting/report_object_template.rb | 13 -- lib/reporting/report_query_template.rb | 32 +-- lib/reporting/report_template.rb | 21 ++ .../reports/bulk_coop/bulk_coop_report.rb | 5 +- .../enterprise_fee_summary_report.rb | 4 + lib/reporting/reports/list.rb | 24 +-- .../customer_totals_report.rb | 2 + .../orders_and_fulfillment_report.rb | 8 +- lib/reporting/reports/packing/base.rb | 4 +- .../reports/sales_tax/sales_tax_report.rb | 2 +- .../xero_invoices/xero_invoices_report.rb | 4 - .../admin/reports_controller_spec.rb | 107 +++++----- .../api/v0/reports_controller_spec.rb | 1 - .../{spree => }/admin/reports_helper_spec.rb | 2 +- spec/helpers/navigation_helper_spec.rb | 4 +- .../reports/packing/packing_report_spec.rb | 2 +- spec/lib/reports/report_loader_spec.rb | 69 +----- spec/models/spree/ability_spec.rb | 21 +- spec/support/ability_helper.rb | 18 -- spec/support/ability_helpers.rb | 15 ++ spec/system/admin/reports_spec.rb | 22 +- 68 files changed, 352 insertions(+), 692 deletions(-) delete mode 100644 app/controllers/spree/admin/reports_controller.rb delete mode 100644 app/helpers/spree/reports_helper.rb create mode 100644 app/views/admin/reports/filters/_bulk_coop.html.haml rename app/views/{spree => }/admin/reports/filters/_customers.html.haml (66%) rename app/views/{spree => }/admin/reports/filters/_enterprise_fee_summary.html.haml (97%) rename app/views/{spree => }/admin/reports/filters/_order_cycle_management.html.haml (70%) create mode 100644 app/views/admin/reports/filters/_orders_and_distributors.html.haml create mode 100755 app/views/admin/reports/filters/_orders_and_fulfillment.html.haml create mode 100644 app/views/admin/reports/filters/_payments.html.haml rename app/views/{spree => }/admin/reports/filters/_products_and_inventory.html.haml (66%) create mode 100644 app/views/admin/reports/filters/_sales_tax.html.haml rename app/views/{spree => }/admin/reports/filters/_users_and_enterprises.html.haml (100%) rename app/views/{spree => }/admin/reports/filters/_xero_invoices.html.haml (79%) create mode 100644 app/views/admin/reports/index.html.haml delete mode 100644 app/views/spree/admin/reports/_customer_names_message.html.haml delete mode 100644 app/views/spree/admin/reports/_customers_description.html.haml delete mode 100644 app/views/spree/admin/reports/_date_range_form.html.haml delete mode 100644 app/views/spree/admin/reports/_link_order.html.haml delete mode 100644 app/views/spree/admin/reports/_order_cycle_management_description.html.haml delete mode 100644 app/views/spree/admin/reports/_orders_and_fulfillment_description.html.haml delete mode 100644 app/views/spree/admin/reports/_packing_description.html.haml delete mode 100644 app/views/spree/admin/reports/_products_and_inventory_description.html.haml delete mode 100644 app/views/spree/admin/reports/_sales_tax_description.html.haml delete mode 100644 app/views/spree/admin/reports/_table.html.haml delete mode 100644 app/views/spree/admin/reports/filters/_bulk_coop.html.haml delete mode 100644 app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml delete mode 100755 app/views/spree/admin/reports/filters/_orders_and_fulfillment.html.haml delete mode 100644 app/views/spree/admin/reports/filters/_payments.html.haml delete mode 100644 app/views/spree/admin/reports/filters/_sales_tax.html.haml delete mode 100644 app/views/spree/admin/reports/index.html.haml delete mode 100644 app/views/spree/admin/reports/show.html.haml rename spec/controllers/{spree => }/admin/reports_controller_spec.rb (75%) rename spec/helpers/{spree => }/admin/reports_helper_spec.rb (94%) delete mode 100644 spec/support/ability_helper.rb diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index d097f6527b..b5c400a33e 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -5,12 +5,21 @@ module Admin include ReportsActions helper ReportsHelper - before_action :authorize_report + before_action :authorize_report, only: [:show] + + # Define custom model class for Cancan permissions + def model_class + Admin::ReportsController + end + + def index + @reports = reports.select do |report_type, _description| + can? report_type, :report + end + end def show - render_report && return if ransack_params.blank? - - @report = report_class.new(spree_current_user, ransack_params, report_options) + @report = report_class.new(spree_current_user, params) if report_format.present? export_report @@ -27,29 +36,15 @@ module Admin def render_report assign_view_data - load_form_options render "show" end def assign_view_data @report_type = report_type - @report_subtype = report_subtype || report_loader.default_report_subtype - @report_subtypes = report_class.report_subtypes.map do |subtype| - [t("packing.#{subtype}_report", scope: i18n_scope), subtype] - end - if @report_type == "packing" - @report_message = I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") - end - end + @report_subtypes = report_subtypes + @report_subtype = report_subtype - def load_form_options - return unless form_options_required? - - form_options = Reporting::FrontendData.new(spree_current_user) - - @distributors = form_options.distributors.to_a - @suppliers = form_options.suppliers.to_a - @order_cycles = form_options.order_cycles.to_a + @data = Reporting::FrontendData.new(spree_current_user) end end end diff --git a/app/controllers/api/v0/reports_controller.rb b/app/controllers/api/v0/reports_controller.rb index b6cc0facc9..1e4462df85 100644 --- a/app/controllers/api/v0/reports_controller.rb +++ b/app/controllers/api/v0/reports_controller.rb @@ -10,7 +10,7 @@ module Api before_action :validate_report, :authorize_report, :validate_query def show - @report = report_class.new(current_api_user, ransack_params, report_options) + @report = report_class.new(current_api_user, params) render_report end diff --git a/app/controllers/concerns/reports_actions.rb b/app/controllers/concerns/reports_actions.rb index 8a4cd10afa..35a508b154 100644 --- a/app/controllers/concerns/reports_actions.rb +++ b/app/controllers/concerns/reports_actions.rb @@ -3,10 +3,14 @@ module ReportsActions extend ActiveSupport::Concern + def reports + Reporting::Reports::List.all + end + private def authorize_report - authorize! report_type&.to_sym, :report + authorize! report_type.to_sym, :report end def report_class @@ -23,27 +27,26 @@ module ReportsActions params[:report_type] end + def report_subtypes + reports[report_type.to_sym] || [] + end + + def report_subtypes_codes + report_subtypes.map(&:second).map(&:to_s) + end + def report_subtype - params[:report_subtype] + params[:report_subtype] || report_subtypes_codes.first end def ransack_params raw_params[:q] end - def report_options - raw_params[:options] - end - def report_format params[:report_format] end - def form_options_required? - [:packing, :customers, :products_and_inventory, :order_cycle_management]. - include? report_type.to_sym - end - def report_filename "#{report_type || action_name}_#{file_timestamp}.#{report_format}" end diff --git a/app/controllers/spree/admin/reports_controller.rb b/app/controllers/spree/admin/reports_controller.rb deleted file mode 100644 index 3f1be8c33f..0000000000 --- a/app/controllers/spree/admin/reports_controller.rb +++ /dev/null @@ -1,198 +0,0 @@ -# frozen_string_literal: true - -# require 'open_food_network/orders_and_distributors_report' -# require 'open_food_network/products_and_inventory_report' -# require 'open_food_network/lettuce_share_report' -# require 'open_food_network/order_grouper' -# require 'open_food_network/customers_report' -# require 'open_food_network/users_and_enterprises_report' -# require 'open_food_network/order_cycle_management_report' -# require 'open_food_network/sales_tax_report' -# require 'open_food_network/xero_invoices_report' -# require 'open_food_network/payments_report' -# require 'open_food_network/orders_and_fulfillment_report' -# require 'open_food_network/bulk_coop_report' - -module Spree - module Admin - class ReportsController < Spree::Admin::BaseController - include Spree::ReportsHelper - include ReportsActions - helper ::ReportsHelper - - helper_method :render_content? - - # Fetches user's distributors, suppliers and order_cycles - before_action :load_basic_data, only: [:customers, :products_and_inventory, :order_cycle_management] - - respond_to :html - - def report_types - Reporting::Reports::List.all - end - - def index - @reports = authorized_reports - respond_with(@reports) - end - - def customers - render_report - end - - def order_cycle_management - render_report - end - - def orders_and_distributors - render_report - end - - def sales_tax - @distributors = my_distributors - render_report - end - - def payments - @distributors = my_distributors - render_report - end - - def orders_and_fulfillment - form_options = Reporting::FrontendData.new(spree_current_user) - - @distributors = form_options.distributors - @suppliers = form_options.suppliers - @order_cycles = form_options.order_cycles - - @report_message = I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") - - render_report - end - - def products_and_inventory - render_report - end - - def users_and_enterprises - render_report - end - - def xero_invoices - @distributors = my_distributors - @order_cycles = my_order_cycles - - render_report - end - - def bulk_coop - @distributors = my_distributors - @report_message = I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") - render_report - end - - def enterprise_fee_summary - @report_message = I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") - render_report - end - - private - - def model_class - Spree::Admin::ReportsController - end - - # We don't want to render data unless search params are supplied. - # Compiling data can take a long time. - def render_content? - request.post? - end - - def render_report - @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 - if report_format.present? - send_data @report.public_send("to_#{report_format}"), filename: report_filename - else - render "show" - end - end - - def load_basic_data - @distributors = my_distributors - @suppliers = my_suppliers | suppliers_of_products_distributed_by(@distributors) - @order_cycles = my_order_cycles - end - - # Load managed distributor enterprises of current user - def my_distributors - Enterprise.is_distributor.managed_by(spree_current_user) - end - - # Load managed producer enterprises of current user - def my_suppliers - Enterprise.is_primary_producer.managed_by(spree_current_user) - end - - def suppliers_of_products_distributed_by(distributors) - supplier_ids = Spree::Product.in_distributors(distributors.select('enterprises.id')). - select('spree_products.supplier_id') - - Enterprise.where(id: supplier_ids) - end - - # Load order cycles the current user has access to - def my_order_cycles - OrderCycle. - active_or_complete. - visible_by(spree_current_user). - order('orders_close_at DESC') - end - - def authorized_reports - all_reports = [ - :orders_and_distributors, - :bulk_coop, - :payments, - :orders_and_fulfillment, - :customers, - :products_and_inventory, - :users_and_enterprises, - :enterprise_fee_summary, - :order_cycle_management, - :sales_tax, - :xero_invoices, - :packing - ] - reports = all_reports.select { |action| can? action, Spree::Admin::ReportsController } - reports.map { |report| [report, describe_report(report)] }.to_h - end - - def describe_report(report) - name = I18n.t(:name, scope: [:admin, :reports, report]) - description = begin - I18n.t!(:description, scope: [:admin, :reports, report]) - rescue I18n::MissingTranslationData - render_to_string( - partial: "#{report}_description", - layout: false, - locals: { report_types: report_types[report] } - ).html_safe - end - { name: name, url: url_for_report(report), description: description } - end - - def url_for_report(report) - spree.public_send("#{report}_admin_reports_url".to_sym) - rescue NoMethodError - main_app.admin_reports_url(report_type: report) - end - - def timestamp - Time.zone.now.strftime("%Y%m%d") - end - end - end -end diff --git a/app/helpers/reports_helper.rb b/app/helpers/reports_helper.rb index bfcc4232db..953a4bbffa 100644 --- a/app/helpers/reports_helper.rb +++ b/app/helpers/reports_helper.rb @@ -9,7 +9,24 @@ module ReportsHelper end end - def report_subtypes(report) - Reporting::ReportLoader.new(report).report_subtypes + def report_payment_method_options(orders) + orders.map do |order| + payment_method = order.payments.last&.payment_method + + next unless payment_method + + [payment_method.name, payment_method.id] + end.compact.uniq + end + + def report_shipping_method_options(orders) + orders.map do |o| + sm = o.shipping_method + [sm&.name, sm&.id] + end.uniq + end + + def currency_symbol + Spree::Money.currency_symbol end end diff --git a/app/helpers/spree/admin/navigation_helper.rb b/app/helpers/spree/admin/navigation_helper.rb index 324e056330..56fc759660 100644 --- a/app/helpers/spree/admin/navigation_helper.rb +++ b/app/helpers/spree/admin/navigation_helper.rb @@ -77,7 +77,7 @@ module Spree klass = EnterpriseGroup if klass == :group klass = VariantOverride if klass == :Inventory klass = ProductImport::ProductImporter if klass == :import - klass = Spree::Admin::ReportsController if klass == :report + klass = ::Admin::ReportsController if klass == :report klass end diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb deleted file mode 100644 index 05b3fa7af6..0000000000 --- a/app/helpers/spree/reports_helper.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require 'spree/money' - -module Spree - module ReportsHelper - def report_payment_method_options(orders) - orders.map do |order| - payment_method = order.payments.last&.payment_method - - next unless payment_method - - [payment_method.name, payment_method.id] - end.compact.uniq - end - - def report_shipping_method_options(orders) - orders.map do |o| - sm = o.shipping_method - [sm&.name, sm&.id] - end.uniq - end - - def currency_symbol - Spree::Money.currency_symbol - end - end -end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index fcb1ffc37a..2e9aec849f 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -236,12 +236,10 @@ module Spree :validate_data, :reset_absent_products], ProductImport::ProductImporter # Reports page - can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :payments, - :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :packing], - Spree::Admin::ReportsController - can [:admin, :show, :packing], :report - add_bulk_coop_abilities - add_enterprise_fee_summary_abilities + can [:admin, :index, :show], ::Admin::ReportsController + can [:admin, :show, :customers, :orders_and_distributors, :group_buys, :payments, + :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, + :packing, :enterprise_fee_summary, :bulk_coop], :report end def add_order_cycle_management_abilities(user) @@ -317,11 +315,10 @@ module Spree end # Reports page - can [:admin, :index, :customers, :group_buys, :sales_tax, :payments, + can [:admin, :index, :show], ::Admin::ReportsController + can [:admin, :customers, :group_buys, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, - :order_cycle_management, :xero_invoices], Spree::Admin::ReportsController - add_bulk_coop_abilities - add_enterprise_fee_summary_abilities + :order_cycle_management, :xero_invoices, :enterprise_fee_summary, :bulk_coop], :report can [:create], Customer can [:admin, :index, :update, @@ -346,19 +343,5 @@ module Spree user.enterprises.include?(enterprise_relationship.child) end end - - def add_bulk_coop_abilities - # Reveal the report link in spree/admin/reports#index - can [:bulk_coop], Spree::Admin::ReportsController - # Allow direct access to the report resource - can [:admin, :new, :create], :bulk_coop - end - - def add_enterprise_fee_summary_abilities - # Reveal the report link in spree/admin/reports#index - can [:enterprise_fee_summary], Spree::Admin::ReportsController - # Allow direct access to the report resource - can [:admin, :new, :create], :enterprise_fee_summary - end end end diff --git a/app/views/admin/reports/_date_range_form.html.haml b/app/views/admin/reports/_date_range_form.html.haml index 134e7d967f..34bd30ec84 100644 --- a/app/views/admin/reports/_date_range_form.html.haml +++ b/app/views/admin/reports/_date_range_form.html.haml @@ -1,9 +1,10 @@ +-# Field used for ransack search. This date range is mostly used for Spree::Order +-# so default field is 'completed_at' +- field ||= 'completed_at' .row.date-range-filter - .alpha.two.columns - = label_tag nil, t(:date_range) + .alpha.two.columns= label_tag nil, t(:date_range) .omega.fourteen.columns - = text_field_tag "q[order_completed_at_gt]", params.dig(:q, :order_completed_at_gt), :class => 'datetimepicker datepicker-from', :placeholder => t(:start) + = f.text_field "#{field}_gt", :class => 'datetimepicker datepicker-from', :placeholder => t(:start) %span.range-divider %i.icon-arrow-right - = text_field_tag "q[order_completed_at_lt]", params.dig(:q, :order_completed_at_lt), :class => 'datetimepicker datepicker-to', :placeholder => t(:stop) - + = f.text_field "#{field}_lt", :class => 'datetimepicker datepicker-to', :placeholder => t(:stop) diff --git a/app/views/admin/reports/_table.html.haml b/app/views/admin/reports/_table.html.haml index b5dd0586a5..1d83358fd1 100644 --- a/app/views/admin/reports/_table.html.haml +++ b/app/views/admin/reports/_table.html.haml @@ -6,6 +6,7 @@ - report.table_headers.each do |heading| %th = t("admin.reports.table.headings.#{heading}") + -# = heading %tbody - report.table_rows.each do |row| - if row diff --git a/app/views/admin/reports/filters/_bulk_coop.html.haml b/app/views/admin/reports/filters/_bulk_coop.html.haml new file mode 100644 index 0000000000..264facbf9f --- /dev/null +++ b/app/views/admin/reports/filters/_bulk_coop.html.haml @@ -0,0 +1,6 @@ += render 'admin/reports/date_range_form', f: f + +.row + .alpha.two.columns= label_tag nil, t(:report_hubs) + .omega.fourteen.columns + = f.collection_select(:distributor_id_in, @data.distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) diff --git a/app/views/spree/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml similarity index 66% rename from app/views/spree/admin/reports/filters/_customers.html.haml rename to app/views/admin/reports/filters/_customers.html.haml index 83fb9f87b5..caa88f4e5d 100644 --- a/app/views/spree/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -2,17 +2,17 @@ .alpha.two.columns= label_tag nil, t(:report_customers_distributor) .omega.fourteen.columns = select_tag(:distributor_id, - options_from_collection_for_select(@distributors, :id, :name, params[:distributor_id]), + options_from_collection_for_select(@data.distributors, :id, :name, params[:distributor_id]), {:include_blank => true, :class => "select2 fullwidth light"}) .row .alpha.two.columns= label_tag nil, t(:report_customers_supplier) .omega.fourteen.columns = select_tag(:supplier_id, - options_from_collection_for_select(@suppliers, :id, :name, params[:supplier_id]), + options_from_collection_for_select(@data.suppliers, :id, :name, params[:supplier_id]), {:include_blank => true, :class => "select2 fullwidth light"}) .row .alpha.two.columns= label_tag nil, t(:report_customers_cycle) .omega.fourteen.columns = select_tag(:order_cycle_id, - options_for_select(report_order_cycle_options(@order_cycles), params[:order_cycle_id]), + options_for_select(report_order_cycle_options(@data.order_cycles), params[:order_cycle_id]), {:include_blank => true, :class => "select2 fullwidth light"}) \ No newline at end of file diff --git a/app/views/spree/admin/reports/filters/_enterprise_fee_summary.html.haml b/app/views/admin/reports/filters/_enterprise_fee_summary.html.haml similarity index 97% rename from app/views/spree/admin/reports/filters/_enterprise_fee_summary.html.haml rename to app/views/admin/reports/filters/_enterprise_fee_summary.html.haml index c64a55823c..79998980cd 100644 --- a/app/views/spree/admin/reports/filters/_enterprise_fee_summary.html.haml +++ b/app/views/admin/reports/filters/_enterprise_fee_summary.html.haml @@ -1,4 +1,4 @@ -= render 'spree/admin/reports/date_range_form', f: f += render 'admin/reports/date_range_form', f: f .row .alpha.two.columns= label_tag nil, t(:report_hubs) diff --git a/app/views/spree/admin/reports/filters/_order_cycle_management.html.haml b/app/views/admin/reports/filters/_order_cycle_management.html.haml similarity index 70% rename from app/views/spree/admin/reports/filters/_order_cycle_management.html.haml rename to app/views/admin/reports/filters/_order_cycle_management.html.haml index 7de9126e14..dc3977266c 100644 --- a/app/views/spree/admin/reports/filters/_order_cycle_management.html.haml +++ b/app/views/admin/reports/filters/_order_cycle_management.html.haml @@ -1,13 +1,13 @@ -= render 'spree/admin/reports/date_range_form', f: f += render 'admin/reports/date_range_form', f: f .row .alpha.two.columns= label_tag nil, t(:report_hubs) - .omega.fourteen.columns= f.collection_select(:distributor_id_in, @distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) + .omega.fourteen.columns= f.collection_select(:distributor_id_in, @data.distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) .row .alpha.two.columns= label_tag nil, t(:report_customers_cycle) .omega.fourteen.columns - = f.select(:order_cycle_id_in, report_order_cycle_options(@order_cycles), {selected: params.dig(:q, :order_cycle_id_in)}, {class: "select2 fullwidth", multiple: true}) + = f.select(:order_cycle_id_in, report_order_cycle_options(@data.order_cycles), {selected: params.dig(:q, :order_cycle_id_in)}, {class: "select2 fullwidth", multiple: true}) .row .alpha.two.columns= label_tag nil, t(:report_payment) diff --git a/app/views/admin/reports/filters/_orders_and_distributors.html.haml b/app/views/admin/reports/filters/_orders_and_distributors.html.haml new file mode 100644 index 0000000000..35c5a74266 --- /dev/null +++ b/app/views/admin/reports/filters/_orders_and_distributors.html.haml @@ -0,0 +1 @@ += render 'admin/reports/date_range_form', f: f diff --git a/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml b/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml new file mode 100755 index 0000000000..0a9963544f --- /dev/null +++ b/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml @@ -0,0 +1,14 @@ += render 'admin/reports/date_range_form', f: f + +.row + .alpha.two.columns= label_tag nil, t(:report_hubs) + .omega.fourteen.columns= f.collection_select(:distributor_id_in, @data.orders_distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) + +.row + .alpha.two.columns= label_tag nil, t(:report_producers) + .omega.fourteen.columns= select_tag(:supplier_id_in, options_from_collection_for_select(@data.orders_suppliers, :id, :name, params[:supplier_id_in]), {class: "select2 fullwidth", multiple: true}) + +.row + .alpha.two.columns= label_tag nil, t(:report_customers_cycle) + .omega.fourteen.columns + = f.select(:order_cycle_id_in, report_order_cycle_options(@data.order_cycles), {selected: params.dig(:q, :order_cycle_id_in)}, {class: "select2 fullwidth", multiple: true}) diff --git a/app/views/admin/reports/filters/_packing.html.haml b/app/views/admin/reports/filters/_packing.html.haml index c7f54750ee..b15d3a5085 100755 --- a/app/views/admin/reports/filters/_packing.html.haml +++ b/app/views/admin/reports/filters/_packing.html.haml @@ -1,16 +1,16 @@ -= render partial: 'admin/reports/date_range_form' += render partial: 'admin/reports/date_range_form', locals: { f: f, field: 'order_completed_at' } .row .alpha.two.columns= label_tag nil, t(:report_hubs) .omega.fourteen.columns - = collection_select("q", "order_distributor_id_in", @distributors, :id, :name, {selected: params.dig(:q, :order_distributor_id_in)}, {class: "select2 fullwidth", multiple: true}) + = collection_select("q", "order_distributor_id_in", @data.orders_distributors, :id, :name, {selected: params.dig(:q, :order_distributor_id_in)}, {class: "select2 fullwidth", multiple: true}) .row .alpha.two.columns= label_tag nil, t(:report_producers) .omega.fourteen.columns - = select_tag("q[supplier_id_in]", options_from_collection_for_select(@suppliers, :id, :name, params.dig(:q, :supplier_id_in)), {class: "select2 fullwidth", multiple: true}) + = select_tag("q[supplier_id_in]", options_from_collection_for_select(@data.orders_suppliers, :id, :name, params.dig(:q, :supplier_id_in)), {class: "select2 fullwidth", multiple: true}) .row .alpha.two.columns= label_tag nil, t(:report_customers_cycle) .omega.fourteen.columns - = select_tag("q[order_cycle_id_in]", options_for_select(report_order_cycle_options(@order_cycles), params.dig(:q, :order_cycle_id_in)), {class: "select2 fullwidth", multiple: true}) \ No newline at end of file + = select_tag("q[order_cycle_id_in]", options_for_select(report_order_cycle_options(@data.order_cycles), params.dig(:q, :order_cycle_id_in)), {class: "select2 fullwidth", multiple: true}) \ No newline at end of file diff --git a/app/views/admin/reports/filters/_payments.html.haml b/app/views/admin/reports/filters/_payments.html.haml new file mode 100644 index 0000000000..91ec0c9e71 --- /dev/null +++ b/app/views/admin/reports/filters/_payments.html.haml @@ -0,0 +1,6 @@ += render 'admin/reports/date_range_form', f: f + +.row + .alpha.two.columns= label_tag nil, t(:report_distributor) + .omega.fourteen.columns + = f.collection_select(:distributor_id_eq, @data.distributors, :id, :name, {:include_blank => t(:report_all)}, {:class => "select2 fullwidth light"}) diff --git a/app/views/spree/admin/reports/filters/_products_and_inventory.html.haml b/app/views/admin/reports/filters/_products_and_inventory.html.haml similarity index 66% rename from app/views/spree/admin/reports/filters/_products_and_inventory.html.haml rename to app/views/admin/reports/filters/_products_and_inventory.html.haml index dbb56d76b7..921e55be0f 100644 --- a/app/views/spree/admin/reports/filters/_products_and_inventory.html.haml +++ b/app/views/admin/reports/filters/_products_and_inventory.html.haml @@ -2,19 +2,19 @@ .alpha.two.columns= label_tag nil, t(:report_distributor) .omega.fourteen.columns = select_tag(:distributor_id, - options_from_collection_for_select(@distributors, :id, :name, params[:distributor_id]), + options_from_collection_for_select(@data.distributors, :id, :name, params[:distributor_id]), {:include_blank => true, :class => "select2 fullwidth light"}) .row .alpha.two.columns= label_tag nil, t(:report_customers_supplier) .omega.fourteen.columns = select_tag(:supplier_id, - options_from_collection_for_select(@suppliers, :id, :name, params[:supplier_id]), + options_from_collection_for_select(@data.suppliers, :id, :name, params[:supplier_id]), {:include_blank => true, :class => "select2 fullwidth light"}) .row .alpha.two.columns= label_tag nil, t(:report_order_cycle) .omega.fourteen.columns = select_tag(:order_cycle_id, - options_for_select(report_order_cycle_options(@order_cycles), params[:order_cycle_id]), + options_for_select(report_order_cycle_options(@data.order_cycles), params[:order_cycle_id]), {:include_blank => true, :class => "select2 fullwidth light"}) diff --git a/app/views/admin/reports/filters/_sales_tax.html.haml b/app/views/admin/reports/filters/_sales_tax.html.haml new file mode 100644 index 0000000000..bdedb904c2 --- /dev/null +++ b/app/views/admin/reports/filters/_sales_tax.html.haml @@ -0,0 +1,6 @@ += render 'admin/reports/date_range_form', f: f + +.row + .alpha.two.columns= label_tag nil, t(:report_distributor) + .omega.fourteen.columns + = f.collection_select(:distributor_id_eq, @data.distributors, :id, :name, {:include_blank => t(:all)}, {:class => "select2 fullwidth light"}) diff --git a/app/views/spree/admin/reports/filters/_users_and_enterprises.html.haml b/app/views/admin/reports/filters/_users_and_enterprises.html.haml similarity index 100% rename from app/views/spree/admin/reports/filters/_users_and_enterprises.html.haml rename to app/views/admin/reports/filters/_users_and_enterprises.html.haml diff --git a/app/views/spree/admin/reports/filters/_xero_invoices.html.haml b/app/views/admin/reports/filters/_xero_invoices.html.haml similarity index 79% rename from app/views/spree/admin/reports/filters/_xero_invoices.html.haml rename to app/views/admin/reports/filters/_xero_invoices.html.haml index d6425ed0bd..c3c7f765cb 100644 --- a/app/views/spree/admin/reports/filters/_xero_invoices.html.haml +++ b/app/views/admin/reports/filters/_xero_invoices.html.haml @@ -1,12 +1,12 @@ -= render 'spree/admin/reports/date_range_form', f: f += render 'admin/reports/date_range_form', f: f .row .two.columns.alpha= label_tag nil, t(:report_hubs) - .fourteen.columns.omega= f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => 'All'}, {:class => "select2 fullwidth light"}) + .fourteen.columns.omega= f.collection_select(:distributor_id_eq, @data.distributors, :id, :name, {:include_blank => 'All'}, {:class => "select2 fullwidth light"}) .row .two.columns.alpha= label_tag nil, t(:report_order_cycle) .fourteen.columns.omega= f.select(:order_cycle_id_eq, - options_for_select(report_order_cycle_options(@order_cycles), params.dig(:q, :order_cycle_id_eq)), + options_for_select(report_order_cycle_options(@data.order_cycles), params.dig(:q, :order_cycle_id_eq)), {:include_blank => true}, {:class => "select2 fullwidth light"}) .row diff --git a/app/views/admin/reports/index.html.haml b/app/views/admin/reports/index.html.haml new file mode 100644 index 0000000000..4465695cd6 --- /dev/null +++ b/app/views/admin/reports/index.html.haml @@ -0,0 +1,25 @@ +- content_for :page_title do + = t(:listing_reports) + +.columns.twelve + %table.index + %thead + %tr + %th= t(:name) + %th= t(:description) + %tbody + - @reports.each do |report_type, report_subtypes| + %tr + %td + - name = I18n.t(:name, scope: [:admin, :reports, report_type]) + - url = main_app.admin_report_url(report_type: report_type) + = link_to name, url + %td + - begin + = I18n.t!(:description, scope: [:admin, :reports, report_type]) + - rescue I18n::MissingTranslationData + %ul{style: "margin-left: 12pt"} + - report_subtypes.each do |report_subtype| + %li + - url = main_app.admin_report_url(report_type: report_type, report_subtype: report_subtype[1]) + = link_to report_subtype[0], url \ No newline at end of file diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 714001f325..716edfab41 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -1,7 +1,7 @@ -= form_tag main_app.admin_reports_path, report_type: @report_type do += form_for @report.search, :url => url_for(only_path: false) do |f| %fieldset.no-border-bottom.print-hidden %legend{ align: 'center'}= t(:report_filters) - = render partial: "admin/reports/filters/#{@report_type}" + = render partial: "admin/reports/filters/#{@report_type}", locals: { f: f } %fieldset.print-hidden %legend{ align: 'center'}= t(:report_render_options) @@ -10,8 +10,10 @@ .actions.filter-actions = button t(:go), "report__submit-btn" -- if @report_message.present? - %p.report__message.print-hidden= @report_message +- if @report.message.present? + %p.report__message.print-hidden= @report.message -- if params[:q].present? +/ We don't want to render data unless search params are supplied. +/ Compiling data can take a long time. +- if request.post? = render "table" diff --git a/app/views/spree/admin/reports/_customer_names_message.html.haml b/app/views/spree/admin/reports/_customer_names_message.html.haml deleted file mode 100644 index 27f18cc93c..0000000000 --- a/app/views/spree/admin/reports/_customer_names_message.html.haml +++ /dev/null @@ -1,2 +0,0 @@ -%p.report__message - = t(".customer_names_tip") diff --git a/app/views/spree/admin/reports/_customers_description.html.haml b/app/views/spree/admin/reports/_customers_description.html.haml deleted file mode 100644 index 22d440725e..0000000000 --- a/app/views/spree/admin/reports/_customers_description.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_types.each do |report_type| - %li - = link_to report_type[0], "#{customers_admin_reports_url}?report_subtype=#{report_type[1]}" diff --git a/app/views/spree/admin/reports/_date_range_form.html.haml b/app/views/spree/admin/reports/_date_range_form.html.haml deleted file mode 100644 index 28d1385c51..0000000000 --- a/app/views/spree/admin/reports/_date_range_form.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -.row.date-range-filter - .alpha.two.columns= label_tag nil, t(:date_range) - .omega.fourteen.columns - = f.text_field :completed_at_gt, :class => 'datetimepicker datepicker-from', :placeholder => t(:start) - %span.range-divider - %i.icon-arrow-right - = f.text_field :completed_at_lt, :class => 'datetimepicker datepicker-to', :placeholder => t(:stop) diff --git a/app/views/spree/admin/reports/_link_order.html.haml b/app/views/spree/admin/reports/_link_order.html.haml deleted file mode 100644 index 1efecf6d25..0000000000 --- a/app/views/spree/admin/reports/_link_order.html.haml +++ /dev/null @@ -1,2 +0,0 @@ -- order_number = value -= link_to order_number, edit_admin_order_url(order_number), class: 'edit-order' diff --git a/app/views/spree/admin/reports/_order_cycle_management_description.html.haml b/app/views/spree/admin/reports/_order_cycle_management_description.html.haml deleted file mode 100644 index 7c3b39a69e..0000000000 --- a/app/views/spree/admin/reports/_order_cycle_management_description.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_types.each do |report_type| - %li - = link_to report_type[0], "#{order_cycle_management_admin_reports_url}?report_subtype=#{report_type[1]}" diff --git a/app/views/spree/admin/reports/_orders_and_fulfillment_description.html.haml b/app/views/spree/admin/reports/_orders_and_fulfillment_description.html.haml deleted file mode 100644 index d17874cd38..0000000000 --- a/app/views/spree/admin/reports/_orders_and_fulfillment_description.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_types.each do |report_type| - %li - = link_to report_type[0], "#{orders_and_fulfillment_admin_reports_url}?report_subtype=#{report_type[1]}" diff --git a/app/views/spree/admin/reports/_packing_description.html.haml b/app/views/spree/admin/reports/_packing_description.html.haml deleted file mode 100644 index d91e291909..0000000000 --- a/app/views/spree/admin/reports/_packing_description.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_subtypes("packing").each do |report_subtype| - %li - = link_to t("admin.reports.packing.#{report_subtype}_report"), - main_app.admin_reports_url(report_type: 'packing', report_subtype: report_subtype) diff --git a/app/views/spree/admin/reports/_products_and_inventory_description.html.haml b/app/views/spree/admin/reports/_products_and_inventory_description.html.haml deleted file mode 100644 index f65d13273e..0000000000 --- a/app/views/spree/admin/reports/_products_and_inventory_description.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_types.each do |report_type| - %li - = link_to report_type[0], "#{products_and_inventory_admin_reports_url}?report_subtype=#{report_type[1]}" diff --git a/app/views/spree/admin/reports/_sales_tax_description.html.haml b/app/views/spree/admin/reports/_sales_tax_description.html.haml deleted file mode 100644 index 169be2591e..0000000000 --- a/app/views/spree/admin/reports/_sales_tax_description.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%ul{style: "margin-left: 12pt"} - - report_types.each do |report_type| - %li - = link_to report_type[0], "#{sales_tax_admin_reports_url}?report_subtype=#{report_type[1]}" diff --git a/app/views/spree/admin/reports/_table.html.haml b/app/views/spree/admin/reports/_table.html.haml deleted file mode 100644 index b6b932b4e7..0000000000 --- a/app/views/spree/admin/reports/_table.html.haml +++ /dev/null @@ -1,19 +0,0 @@ -- column_partials ||= {} -%table.report__table - %thead - %tr - - @report.table_headers.each do |heading| - %th= heading - %tbody - - @report.table_rows.each do |row| - %tr - - row.each_with_index do |cell_value, column_index| - %td - - partial = column_partials[column_index] - - if partial - = render partial, value: cell_value - - else - = cell_value - - if @report.table_rows.empty? - %tr - %td{colspan: @report.table_headers.count}= t(:none) diff --git a/app/views/spree/admin/reports/filters/_bulk_coop.html.haml b/app/views/spree/admin/reports/filters/_bulk_coop.html.haml deleted file mode 100644 index f9ba0eb4f9..0000000000 --- a/app/views/spree/admin/reports/filters/_bulk_coop.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -= render 'spree/admin/reports/date_range_form', f: f - -.row - .alpha.two.columns= label_tag nil, t(:report_hubs) - .omega.fourteen.columns - = f.collection_select(:distributor_id_in, @distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) diff --git a/app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml b/app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml deleted file mode 100644 index 6d7296bbab..0000000000 --- a/app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render 'spree/admin/reports/date_range_form', f: f diff --git a/app/views/spree/admin/reports/filters/_orders_and_fulfillment.html.haml b/app/views/spree/admin/reports/filters/_orders_and_fulfillment.html.haml deleted file mode 100755 index e43204e60d..0000000000 --- a/app/views/spree/admin/reports/filters/_orders_and_fulfillment.html.haml +++ /dev/null @@ -1,14 +0,0 @@ -= render 'spree/admin/reports/date_range_form', f: f - -.row - .alpha.two.columns= label_tag nil, t(:report_hubs) - .omega.fourteen.columns= f.collection_select(:distributor_id_in, @distributors, :id, :name, {}, {class: "select2 fullwidth", multiple: true}) - -.row - .alpha.two.columns= label_tag nil, t(:report_producers) - .omega.fourteen.columns= select_tag(:supplier_id_in, options_from_collection_for_select(@suppliers, :id, :name, params[:supplier_id_in]), {class: "select2 fullwidth", multiple: true}) - -.row - .alpha.two.columns= label_tag nil, t(:report_customers_cycle) - .omega.fourteen.columns - = f.select(:order_cycle_id_in, report_order_cycle_options(@order_cycles), {selected: params.dig(:q, :order_cycle_id_in)}, {class: "select2 fullwidth", multiple: true}) diff --git a/app/views/spree/admin/reports/filters/_payments.html.haml b/app/views/spree/admin/reports/filters/_payments.html.haml deleted file mode 100644 index 5a208a876a..0000000000 --- a/app/views/spree/admin/reports/filters/_payments.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -= render 'spree/admin/reports/date_range_form', f: f - -.row - .alpha.two.columns= label_tag nil, t(:report_distributor) - .omega.fourteen.columns - = f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => t(:report_all)}, {:class => "select2 fullwidth light"}) diff --git a/app/views/spree/admin/reports/filters/_sales_tax.html.haml b/app/views/spree/admin/reports/filters/_sales_tax.html.haml deleted file mode 100644 index bdd7a00f97..0000000000 --- a/app/views/spree/admin/reports/filters/_sales_tax.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -= render 'spree/admin/reports/date_range_form', f: f - -.row - .alpha.two.columns= label_tag nil, t(:report_distributor) - .omega.fourteen.columns - = f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => t(:all)}, {:class => "select2 fullwidth light"}) diff --git a/app/views/spree/admin/reports/index.html.haml b/app/views/spree/admin/reports/index.html.haml deleted file mode 100644 index 533eab2de1..0000000000 --- a/app/views/spree/admin/reports/index.html.haml +++ /dev/null @@ -1,14 +0,0 @@ -- content_for :page_title do - = t(:listing_reports) - -.columns.twelve - %table.index - %thead - %tr - %th= t(:name) - %th= t(:description) - %tbody - - @reports.each do |key, value| - %tr - %td= link_to value[:name], value[:url] - %td= value[:description] diff --git a/app/views/spree/admin/reports/show.html.haml b/app/views/spree/admin/reports/show.html.haml deleted file mode 100644 index 5ea2ed207e..0000000000 --- a/app/views/spree/admin/reports/show.html.haml +++ /dev/null @@ -1,19 +0,0 @@ - -= form_for @report.search, :url => url_for(only_path: false) do |f| - - %fieldset.no-border-bottom.print-hidden - %legend{ align: 'center'}= t(:report_filters) - = render partial: "spree/admin/reports/filters/#{action_name}", locals: { f: f } - - %fieldset.print-hidden - %legend{ align: 'center'}= t(:report_render_options) - = render partial: "admin/reports/rendering_options" - - .actions.filter-actions - = button t(:go), "report__submit-btn" - -- if @report_message.present? - %p.report__message.print-hidden= @report_message - -- if render_content? - = render "table" diff --git a/app/views/spree/admin/shared/_tabs.html.haml b/app/views/spree/admin/shared/_tabs.html.haml index eb653c2500..696a3d29a6 100644 --- a/app/views/spree/admin/shared/_tabs.html.haml +++ b/app/views/spree/admin/shared/_tabs.html.haml @@ -2,7 +2,7 @@ = tab :products, :properties, :inventory, :product_import, :images, :variants, :product_properties, :group_buy_options, :seo, url: admin_products_path, icon: 'icon-th-large' = tab :order_cycles, url: main_app.admin_order_cycles_path, icon: 'icon-refresh' = tab :orders, :subscriptions, :customer_details, :adjustments, :payments, :return_authorizations, url: admin_orders_path('q[s]' => 'completed_at desc'), icon: 'icon-shopping-cart' -= tab :reports, icon: 'icon-file' += tab :reports, url: main_app.admin_reports_path, icon: 'icon-file' = tab :general_settings, :mail_methods, :tax_categories, :tax_rates, :tax_settings, :zones, :countries, :states, :payment_methods, :taxonomies, :shipping_methods, :shipping_categories, :enterprise_fees, :contents, :invoice_settings, :matomo_settings, :stripe_connect_settings, label: 'configuration', icon: 'icon-wrench', url: edit_admin_general_settings_path = tab :enterprises, :enterprise_relationships, url: main_app.admin_enterprises_path = tab :customers, url: main_app.admin_customers_path diff --git a/config/locales/en.yml b/config/locales/en.yml index cfa48a650c..bdd2bebcc3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1285,7 +1285,6 @@ en: name: Customers products_and_inventory: name: Products & Inventory - description: users_and_enterprises: name: Users & Enterprises description: Enterprise Ownership & Status @@ -1330,8 +1329,6 @@ en: hide_summary_rows: "Hide summary Rows" packing: name: "Packing Reports" - customer_report: "Pack By Customer" - supplier_report: "Pack By Supplier" subscriptions: index: title: "Subscriptions" diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 954b2861d4..7fc4afcd8f 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -115,6 +115,7 @@ Openfoodnetwork::Application.routes.draw do put :resume, on: :member, format: :json end - match '/reports/:report_type(/:report_subtype)', to: 'reports#show', via: [:get, :post], as: :reports + get '/reports', to: 'reports#index', as: :reports + match '/reports/:report_type(/:report_subtype)', to: 'reports#show', via: [:get, :post], as: :report end end diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 952b3a423f..eed705f5d6 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -31,19 +31,6 @@ Spree::Core::Engine.routes.draw do resource :account, :controller => 'users' - match '/admin/reports/orders_and_distributors' => 'admin/reports#orders_and_distributors', :as => "orders_and_distributors_admin_reports", :via => [:get, :post] - match '/admin/reports/order_cycle_management' => 'admin/reports#order_cycle_management', :as => "order_cycle_management_admin_reports", :via => [:get, :post] - match '/admin/reports/group_buys' => 'admin/reports#group_buys', :as => "group_buys_admin_reports", :via => [:get, :post] - match '/admin/reports/bulk_coop' => 'admin/reports#bulk_coop', :as => "bulk_coop_admin_reports", :via => [:get, :post] - match '/admin/reports/payments' => 'admin/reports#payments', :as => "payments_admin_reports", :via => [:get, :post] - match '/admin/reports/orders_and_fulfillment' => 'admin/reports#orders_and_fulfillment', :as => "orders_and_fulfillment_admin_reports", :via => [:get, :post] - match '/admin/reports/users_and_enterprises' => 'admin/reports#users_and_enterprises', :as => "users_and_enterprises_admin_reports", :via => [:get, :post] - match '/admin/reports/sales_tax' => 'admin/reports#sales_tax', :as => "sales_tax_admin_reports", :via => [:get, :post] - match '/admin/reports/products_and_inventory' => 'admin/reports#products_and_inventory', :as => "products_and_inventory_admin_reports", :via => [:get, :post] - match '/admin/reports/customers' => 'admin/reports#customers', :as => "customers_admin_reports", :via => [:get, :post] - match '/admin/reports/xero_invoices' => 'admin/reports#xero_invoices', :as => "xero_invoices_admin_reports", :via => [:get, :post] - match '/admin/reports/enterprise_fee_summary' => 'admin/reports#enterprise_fee_summary', :as => "enterprise_fee_summary_admin_reports", :via => [:get, :post] - match '/admin/orders/bulk_management' => 'admin/orders#bulk_management', :as => "admin_bulk_order_management", via: :get match '/admin/payment_methods/show_provider_preferences' => 'admin/payment_methods#show_provider_preferences', :via => :get put 'credit_cards/new_from_token', to: 'credit_cards#new_from_token' @@ -127,8 +114,6 @@ Spree::Core::Engine.routes.draw do end end - resources :reports, only: :index - resources :users do member do put :generate_api_key diff --git a/lib/reporting/frontend_data.rb b/lib/reporting/frontend_data.rb index aad748b1ff..a702a91de0 100644 --- a/lib/reporting/frontend_data.rb +++ b/lib/reporting/frontend_data.rb @@ -6,18 +6,39 @@ module Reporting @current_user = current_user end + # Load managed distributor enterprises of current user def distributors - permissions.visible_enterprises_for_order_reports.is_distributor. - select("enterprises.id, enterprises.name") + @distributors ||= Enterprise.is_distributor.managed_by(current_user) end def suppliers - permissions.visible_enterprises_for_order_reports.is_primary_producer. - select("enterprises.id, enterprises.name") + @suppliers ||= my_suppliers | suppliers_of_products_distributed_by(distributors) + end + + # Load managed producer enterprises of current user + def my_suppliers + Enterprise.is_primary_producer.managed_by(current_user) + end + + def suppliers_of_products_distributed_by(distributors) + supplier_ids = Spree::Product.in_distributors(distributors.select('enterprises.id')). + select('spree_products.supplier_id') + + Enterprise.where(id: supplier_ids) + end + + def orders_distributors + @orders_distributors ||= permissions.visible_enterprises_for_order_reports + .is_distributor.select("enterprises.id, enterprises.name") + end + + def orders_suppliers + @orders_suppliers ||= permissions.visible_enterprises_for_order_reports + .is_primary_producer.select("enterprises.id, enterprises.name") end def order_cycles - OrderCycle. + @order_cycles ||= OrderCycle. active_or_complete. visible_by(current_user). order('order_cycles.orders_close_at DESC') diff --git a/lib/reporting/report_loader.rb b/lib/reporting/report_loader.rb index b40f03b4ca..29dee2f794 100644 --- a/lib/reporting/report_loader.rb +++ b/lib/reporting/report_loader.rb @@ -2,21 +2,22 @@ module Reporting class ReportLoader - delegate :report_subtypes, to: :base_class - def initialize(report_type, report_subtype = nil) @report_type = report_type - @report_subtype = report_subtype + @report_subtype = report_subtype || "base" end + # We currently use two types of report : old report inheriting from ReportObjectReport + # And new ones inheriting gtom ReportQueryReport + # They use different namespace and we try to load them both def report_class - "#{report_module}::#{report_subtype_class}".constantize + "#{report_module}::#{report_type.camelize}Report".constantize rescue NameError - raise Reporting::Errors::ReportNotFound - end - - def default_report_subtype - report_subtypes.first || "base" + begin + "#{report_module}::#{report_subtype.camelize}".constantize + rescue NameError + raise Reporting::Errors::ReportNotFound + end end private @@ -26,15 +27,5 @@ module Reporting def report_module "Reporting::Reports::#{report_type.camelize}" end - - def report_subtype_class - (report_subtype || default_report_subtype).camelize - end - - def base_class - "#{report_module}::Base".constantize - rescue NameError - raise Reporting::Errors::ReportNotFound - end end end diff --git a/lib/reporting/report_object_template.rb b/lib/reporting/report_object_template.rb index 1eba55a0f9..e9d6e2bbb5 100644 --- a/lib/reporting/report_object_template.rb +++ b/lib/reporting/report_object_template.rb @@ -5,13 +5,6 @@ module Reporting class ReportObjectTemplate < ReportTemplate - attr_accessor :user, :params - - def initialize(user, params = {}) - @user = user - @params = params - end - def table_headers raise NotImplementedError end @@ -20,12 +13,6 @@ module Reporting 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 [] diff --git a/lib/reporting/report_query_template.rb b/lib/reporting/report_query_template.rb index f4b79d22aa..79cea83f86 100644 --- a/lib/reporting/report_query_template.rb +++ b/lib/reporting/report_query_template.rb @@ -3,20 +3,6 @@ # 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 @@ -33,32 +19,30 @@ module Reporting report_data.rows end - private - - attr_reader :current_user, :ransack_params - - def ransacked_orders_relation - visible_orders_relation.ransack(ransack_params).result + def search + visible_line_items_relation.ransack(ransack_params) end + private + def ransacked_line_items_relation - visible_line_items_relation.ransack(ransack_params).result + search.result end def visible_orders_relation - ::Permissions::Order.new(current_user). + ::Permissions::Order.new(user). visible_orders.complete.not_state(:canceled). select(:id).distinct end def visible_line_items_relation - ::Permissions::Order.new(current_user). + ::Permissions::Order.new(user). visible_line_items. select(:id).distinct end def managed_orders_relation - ::Enterprise.managed_by(current_user).select(:id).distinct + ::Enterprise.managed_by(user).select(:id).distinct end def i18n_scope diff --git a/lib/reporting/report_template.rb b/lib/reporting/report_template.rb index 91c3fb73b2..2e961331f4 100644 --- a/lib/reporting/report_template.rb +++ b/lib/reporting/report_template.rb @@ -7,6 +7,15 @@ module Reporting delegate :as_json, :as_arrays, :to_csv, :to_xlsx, :to_ods, :to_pdf, :to_json, to: :renderer + OPTIONAL_HEADERS = [].freeze + + def initialize(user, params) + @user = user + @params = params || {} + @params = @params.permit!.to_h unless @params.is_a? Hash + @ransack_params = @params[:q] || {} + end + def table_headers raise NotImplementedError end @@ -15,6 +24,18 @@ module Reporting raise NotImplementedError end + # Message to be displayed at the top of rendered table + def message + "" + end + + # Ransack search to get base ActiveRelation + # 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 + private def renderer diff --git a/lib/reporting/reports/bulk_coop/bulk_coop_report.rb b/lib/reporting/reports/bulk_coop/bulk_coop_report.rb index be15a9bdd4..62cb436868 100644 --- a/lib/reporting/reports/bulk_coop/bulk_coop_report.rb +++ b/lib/reporting/reports/bulk_coop/bulk_coop_report.rb @@ -4,7 +4,6 @@ module Reporting module Reports module BulkCoop class BulkCoopReport < ReportObjectTemplate - def initialize(user, params = {}) super(user, params) @@ -13,6 +12,10 @@ module Reporting @filter_canceled = false end + def message + I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + end + def table_headers case params[:report_subtype] when "bulk_coop_supplier_report" 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 4298533d8e..69f443aca8 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 @@ -19,6 +19,10 @@ module Reporting @parameters.authorize!(@permissions) end + def message + I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + end + def table_headers data_row_attributes.map do |attribute| header_label(attribute) diff --git a/lib/reporting/reports/list.rb b/lib/reporting/reports/list.rb index 917282b7db..e8055ca9d8 100644 --- a/lib/reporting/reports/list.rb +++ b/lib/reporting/reports/list.rb @@ -13,14 +13,14 @@ module Reporting 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, - enterprise_fee_summary: enterprise_fee_summary_report_types, + products_and_inventory: products_and_inventory_report_types, + users_and_enterprises: [], + enterprise_fee_summary: [], order_cycle_management: order_cycle_management_report_types, sales_tax: sales_tax_report_types, - packing: packing_report_types, xero_invoices: xero_report_types, - bulk_coop: bulk_coop_report_types + packing: packing_report_types } end @@ -59,12 +59,6 @@ module Reporting ] end - def enterprise_fee_summary_report_types - [ - [i18n_translate("enterprise_fee_summary"), :enterprise_fee_summary] - ] - end - def order_cycle_management_report_types [ [i18n_translate("payment_methods"), :payment_methods], @@ -81,14 +75,16 @@ module Reporting def packing_report_types [ - [i18n_translate("pack_by_customer"), :pack_by_customer], - [i18n_translate("pack_by_supplier"), :pack_by_supplier] + [i18n_translate("pack_by_customer"), :customer], + [i18n_translate("pack_by_supplier"), :supplier] ] end def xero_report_types - [[I18n.t(:summary), 'summary'], - [I18n.t(:detailed), 'detailed']] + [ + [I18n.t(:summary), 'summary'], + [I18n.t(:detailed), 'detailed'] + ] end def bulk_coop_report_types diff --git a/lib/reporting/reports/orders_and_fulfillment/customer_totals_report.rb b/lib/reporting/reports/orders_and_fulfillment/customer_totals_report.rb index ada0f6d533..b352fd338e 100644 --- a/lib/reporting/reports/orders_and_fulfillment/customer_totals_report.rb +++ b/lib/reporting/reports/orders_and_fulfillment/customer_totals_report.rb @@ -5,6 +5,8 @@ module Reporting module Reports module OrdersAndFulfillment class CustomerTotalsReport + include ReportsHelper + REPORT_TYPE = "order_cycle_customer_totals" attr_reader :context 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 d99767f474..7089dd911a 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 @@ -1,11 +1,11 @@ # frozen_string_literal: true -include Spree::ReportsHelper - module Reporting module Reports module OrdersAndFulfillment class OrdersAndFulfillmentReport < ReportObjectTemplate + include ReportsHelper + attr_reader :report_type delegate :table_headers, :rules, :columns, to: :report @@ -21,6 +21,10 @@ module Reporting } end + def message + I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + end + def search report_line_items.orders end diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index 5518906120..f5fb7515f3 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -4,7 +4,9 @@ module Reporting module Reports module Packing class Base < ReportQueryTemplate - SUBTYPES = ["customer", "supplier"] + def message + I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + end def primary_model Spree::LineItem diff --git a/lib/reporting/reports/sales_tax/sales_tax_report.rb b/lib/reporting/reports/sales_tax/sales_tax_report.rb index ad07846920..e2c44eb147 100644 --- a/lib/reporting/reports/sales_tax/sales_tax_report.rb +++ b/lib/reporting/reports/sales_tax/sales_tax_report.rb @@ -4,7 +4,7 @@ module Reporting module Reports module SalesTax class SalesTaxReport < ReportObjectTemplate - include Spree::ReportsHelper + include ReportsHelper def table_headers case params[:report_subtype] diff --git a/lib/reporting/reports/xero_invoices/xero_invoices_report.rb b/lib/reporting/reports/xero_invoices/xero_invoices_report.rb index 2da61f0f78..696857e918 100644 --- a/lib/reporting/reports/xero_invoices/xero_invoices_report.rb +++ b/lib/reporting/reports/xero_invoices/xero_invoices_report.rb @@ -45,10 +45,6 @@ module Reporting private - def report_options - params.merge(line_item_includes: line_item_includes) - end - def line_item_includes [:bill_address, :adjustments, { line_items: { variant: [{ option_values: :option_type }, { product: :supplier }] } }] diff --git a/spec/controllers/spree/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb similarity index 75% rename from spec/controllers/spree/admin/reports_controller_spec.rb rename to spec/controllers/admin/reports_controller_spec.rb index 50bb50cac4..3ad47e99d7 100644 --- a/spec/controllers/spree/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Spree::Admin::ReportsController, type: :controller do +describe Admin::ReportsController, type: :controller do # Given two distributors and two suppliers let(:bill_address) { create(:address) } let(:ship_address) { create(:address) } @@ -22,17 +22,20 @@ describe Spree::Admin::ReportsController, type: :controller do # Given two order cycles with both distributors let(:ocA) { create(:simple_order_cycle, coordinator: coordinator1, distributors: [distributor1, distributor2], - suppliers: [supplier1, supplier2, supplier3], variants: [product1.master, product3.master]) + suppliers: [supplier1, supplier2, supplier3], + variants: [product1.master, product3.master]) } let(:ocB) { create(:simple_order_cycle, coordinator: coordinator2, distributors: [distributor1, distributor2], - suppliers: [supplier1, supplier2, supplier3], variants: [product2.master]) + suppliers: [supplier1, supplier2, supplier3], + variants: [product2.master]) } # orderA1 can only be accessed by supplier1, supplier3 and distributor1 let(:orderA1) do order = create(:order, distributor: distributor1, bill_address: bill_address, - ship_address: ship_address, special_instructions: instructions, order_cycle: ocA) + ship_address: ship_address, special_instructions: instructions, + order_cycle: ocA) order.line_items << create(:line_item, variant: product1.master) order.line_items << create(:line_item, variant: product3.master) order.finalize! @@ -42,7 +45,8 @@ describe Spree::Admin::ReportsController, type: :controller do # orderA2 can only be accessed by supplier2 and distributor2 let(:orderA2) do order = create(:order, distributor: distributor2, bill_address: bill_address, - ship_address: ship_address, special_instructions: instructions, order_cycle: ocA) + ship_address: ship_address, special_instructions: instructions, + order_cycle: ocA) order.line_items << create(:line_item, variant: product2.master) order.finalize! order.save @@ -51,7 +55,8 @@ describe Spree::Admin::ReportsController, type: :controller do # orderB1 can only be accessed by supplier1, supplier3 and distributor1 let(:orderB1) do order = create(:order, distributor: distributor1, bill_address: bill_address, - ship_address: ship_address, special_instructions: instructions, order_cycle: ocB) + ship_address: ship_address, special_instructions: instructions, + order_cycle: ocB) order.line_items << create(:line_item, variant: product1.master) order.line_items << create(:line_item, variant: product3.master) order.finalize! @@ -61,7 +66,8 @@ describe Spree::Admin::ReportsController, type: :controller do # orderB2 can only be accessed by supplier2 and distributor2 let(:orderB2) do order = create(:order, distributor: distributor2, bill_address: bill_address, - ship_address: ship_address, special_instructions: instructions, order_cycle: ocB) + ship_address: ship_address, special_instructions: instructions, + order_cycle: ocB) order.line_items << create(:line_item, variant: product2.master) order.finalize! order.save @@ -81,8 +87,7 @@ describe Spree::Admin::ReportsController, type: :controller do describe 'Orders & Fulfillment' do it "shows all orders in order cycles I coordinate" do - spree_post :orders_and_fulfillment, q: {} - + spree_post :show, report_type: :orders_and_fulfillment, q: {} expect(resulting_orders).to include orderA1, orderA2 expect(resulting_orders).not_to include orderB1, orderB2 end @@ -97,7 +102,7 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [orderA1, orderA2, orderB1, orderB2] } it "only shows orders that I have access to" do - spree_post :orders_and_distributors + spree_post :show, report_type: :orders_and_distributors expect(assigns(:report).search.result).to include(orderA1, orderB1) expect(assigns(:report).search.result).not_to include(orderA2) @@ -109,7 +114,7 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [orderA1, orderA2, orderB1, orderB2] } it "only shows orders that I have access to" do - spree_post :payments + spree_post :show, report_type: :payments expect(resulting_orders_prelim).to include(orderA1, orderB1) expect(resulting_orders_prelim).not_to include(orderA2) @@ -122,7 +127,7 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [orderA1, orderA2, orderB1, orderB2] } it "only shows orders that I distribute" do - spree_post :orders_and_fulfillment, q: {} + spree_post :show, report_type: :orders_and_fulfillment, q: {} expect(resulting_orders).to include orderA1, orderB1 expect(resulting_orders).not_to include orderA2, orderB2 @@ -133,7 +138,8 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [orderA1, orderB1] } it "only shows the selected order cycle" do - spree_post :orders_and_fulfillment, q: { order_cycle_id_in: [ocA.id.to_s] } + spree_post :show, report_type: :orders_and_fulfillment, + q: { order_cycle_id_in: [ocA.id.to_s] } expect(resulting_orders).to include(orderA1) expect(resulting_orders).not_to include(orderB1) @@ -166,14 +172,14 @@ describe Spree::Admin::ReportsController, type: :controller do end it "only shows product line items that I am supplying" do - spree_post :orders_and_fulfillment, q: {} + spree_post :show, report_type: :orders_and_fulfillment, q: {} expect(resulting_products).to include product1 expect(resulting_products).not_to include product2, product3 end it "only shows the selected order cycle" do - spree_post :orders_and_fulfillment, q: { order_cycle_id_eq: ocA.id } + spree_post :show, report_type: :orders_and_fulfillment, q: { order_cycle_id_eq: ocA.id } expect(resulting_orders_prelim).to include(orderA1) expect(resulting_orders_prelim).not_to include(orderB1) @@ -183,7 +189,7 @@ describe Spree::Admin::ReportsController, type: :controller do before { orderA1.line_items.first.product.destroy } it "only shows product line items that I am supplying" do - spree_post :orders_and_fulfillment, q: {} + spree_post :show, report_type: :orders_and_fulfillment, q: {} table_items = assigns(:report).table_items variant = Spree::Variant.unscoped.find(table_items.first.variant_id) @@ -195,7 +201,7 @@ describe Spree::Admin::ReportsController, type: :controller do context "where I have not granted P-OC to the distributor" do it "does not show me line_items I supply" do - spree_post :orders_and_fulfillment + spree_post :show, report_type: :orders_and_fulfillment expect(resulting_products).not_to include product1, product2, product3 end @@ -212,13 +218,13 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [distributors, suppliers] } it "should build distributors for the current user" do - spree_get :products_and_inventory - expect(assigns(:distributors)).to match_array distributors + spree_get :show, report_type: :products_and_inventory + expect(assigns(:data).distributors).to match_array distributors end it "builds suppliers for the current user" do - spree_get :products_and_inventory - expect(assigns(:suppliers)).to match_array suppliers + spree_get :show, report_type: :products_and_inventory + expect(assigns(:data).suppliers).to match_array suppliers end end @@ -226,25 +232,22 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:order_cycles) { [ocA, ocB] } it "builds order cycles for the current user" do - spree_get :products_and_inventory - expect(assigns(:order_cycles)).to match_array order_cycles + spree_get :show, report_type: :products_and_inventory + expect(assigns(:data).order_cycles).to match_array order_cycles end end it "assigns report types" do - spree_get :products_and_inventory - expect(assigns(:report_subtypes)).to eq(subject.report_types[:products_and_inventory]) + spree_get :show, report_type: :products_and_inventory + expect(assigns(:report_subtypes)).to eq(subject.reports[:products_and_inventory]) end it "creates a ProductAndInventoryReport" do - expect(Reporting::Reports::ProductsAndInventory::ProductsAndInventoryReport).to receive(:new) - .with(@admin_user, - { "test" => "foo", "controller" => "spree/admin/reports", "report" => {}, - "action" => "products_and_inventory", "use_route" => "main_app" }, false) + allow(Reporting::Reports::ProductsAndInventory::ProductsAndInventoryReport).to receive(:new) .and_return(report = double(:report)) allow(report).to receive(:table_headers).and_return [] allow(report).to receive(:table_rows).and_return [] - spree_get :products_and_inventory, test: "foo" + spree_get :show, report_type: :products_and_inventory, test: "foo" expect(assigns(:report)).to eq(report) end end @@ -253,10 +256,10 @@ describe Spree::Admin::ReportsController, type: :controller do before { controller_login_as_admin } it "should have report types for customers" do - expect(subject.report_types[:customers]).to eq([ - ["Mailing List", :mailing_list], - ["Addresses", :addresses] - ]) + expect(subject.reports[:customers]).to eq([ + ["Mailing List", :mailing_list], + ["Addresses", :addresses] + ]) end context "with distributors and suppliers" do @@ -265,13 +268,13 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [distributors, suppliers] } it "should build distributors for the current user" do - spree_get :customers - expect(assigns(:distributors)).to match_array distributors + spree_get :show, report_type: :customers + expect(assigns(:data).distributors).to match_array distributors end it "builds suppliers for the current user" do - spree_get :customers - expect(assigns(:suppliers)).to match_array suppliers + spree_get :show, report_type: :customers + expect(assigns(:data).suppliers).to match_array suppliers end end @@ -279,25 +282,22 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:order_cycles) { [ocA, ocB] } it "builds order cycles for the current user" do - spree_get :customers - expect(assigns(:order_cycles)).to match_array order_cycles + spree_get :show, report_type: :customers + expect(assigns(:data).order_cycles).to match_array order_cycles end end it "assigns report types" do - spree_get :customers - expect(assigns(:report_subtypes)).to eq(subject.report_types[:customers]) + spree_get :show, report_type: :customers + expect(assigns(:report_subtypes)).to eq(subject.reports[:customers]) end it "creates a CustomersReport" do - expect(Reporting::Reports::Customers::CustomersReport).to receive(:new) - .with(@admin_user, { "test" => "foo", "controller" => "spree/admin/reports", - "action" => "customers", "use_route" => "main_app", - "report" => {} }, false) + allow(Reporting::Reports::Customers::CustomersReport).to receive(:new) .and_return(report = double(:report)) allow(report).to receive(:table_headers).and_return [] allow(report).to receive(:table_rows).and_return [] - spree_get :customers, test: "foo" + spree_get :show, report_type: :customers, test: "foo" expect(assigns(:report)).to eq(report) end end @@ -310,9 +310,10 @@ describe Spree::Admin::ReportsController, type: :controller do end it 'renders the delivery report' do - spree_post :order_cycle_management, { + spree_post :show, { q: { completed_at_lt: 1.day.ago }, shipping_method_in: ["123"], # We just need to search for shipping methods + report_type: :order_cycle_management, report_subtype: "delivery", } @@ -327,20 +328,20 @@ describe Spree::Admin::ReportsController, type: :controller do let!(:present_objects) { [coordinator1] } it "shows report search forms" do - spree_get :users_and_enterprises - expect(assigns(:report).table_rows).to eq [] + spree_get :show, report_type: :users_and_enterprises + expect(response).to have_http_status(:ok) end it "shows report data" do - spree_post :users_and_enterprises, q: {} + spree_post :show, report_type: :users_and_enterprises, q: {} expect(assigns(:report).table_rows.empty?).to be false end end describe "sales_tax" do it "shows report search forms" do - spree_get :sales_tax - expect(assigns(:report).table_rows).to eq [] + spree_get :show, report_type: :sales_tax + expect(response).to have_http_status(:ok) end end end diff --git a/spec/controllers/api/v0/reports_controller_spec.rb b/spec/controllers/api/v0/reports_controller_spec.rb index 23ddfabf21..09ba607f4b 100644 --- a/spec/controllers/api/v0/reports_controller_spec.rb +++ b/spec/controllers/api/v0/reports_controller_spec.rb @@ -63,7 +63,6 @@ describe Api::V0::ReportsController, type: :controller do it "returns an error" do api_get :show, report_type: "packing" - expect(response.status).to eq 422 expect(json_response["error"]).to eq( I18n.t('errors.missing_ransack_params', scope: i18n_scope) diff --git a/spec/helpers/spree/admin/reports_helper_spec.rb b/spec/helpers/admin/reports_helper_spec.rb similarity index 94% rename from spec/helpers/spree/admin/reports_helper_spec.rb rename to spec/helpers/admin/reports_helper_spec.rb index a4460e2b4a..9ea424bc48 100644 --- a/spec/helpers/spree/admin/reports_helper_spec.rb +++ b/spec/helpers/admin/reports_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Spree::ReportsHelper, type: :helper do +describe ReportsHelper, type: :helper do describe "#report_payment_method_options" do let(:order_with_payments) { create(:order_ready_to_ship) } let(:order_without_payments) { create(:order_with_line_items) } diff --git a/spec/helpers/navigation_helper_spec.rb b/spec/helpers/navigation_helper_spec.rb index 90199a1fe6..ee01b3204a 100644 --- a/spec/helpers/navigation_helper_spec.rb +++ b/spec/helpers/navigation_helper_spec.rb @@ -14,8 +14,8 @@ module Spree expect(helper.klass_for('lions')).to eq(:lion) end - it "returns Spree::Admin::ReportsController for reports" do - expect(helper.klass_for('reports')).to eq(Spree::Admin::ReportsController) + it "returns Admin::ReportsController for reports" do + expect(helper.klass_for('reports')).to eq(::Admin::ReportsController) end it "returns :overview for the dashboard" do diff --git a/spec/lib/reports/packing/packing_report_spec.rb b/spec/lib/reports/packing/packing_report_spec.rb index d661fbb586..9902d3ebe2 100644 --- a/spec/lib/reports/packing/packing_report_spec.rb +++ b/spec/lib/reports/packing/packing_report_spec.rb @@ -20,7 +20,7 @@ describe "Packing Reports" do let(:report_contents) { subject.report_data.rows.flatten } let(:row_count) { subject.report_data.rows.count } - subject { Reporting::Reports::Packing::Customer.new user, params } + subject { Reporting::Reports::Packing::Customer.new user, { q: params } } before do order.line_items << line_item diff --git a/spec/lib/reports/report_loader_spec.rb b/spec/lib/reports/report_loader_spec.rb index 5341ae641f..1bb8ff00ad 100644 --- a/spec/lib/reports/report_loader_spec.rb +++ b/spec/lib/reports/report_loader_spec.rb @@ -9,6 +9,10 @@ module Reporting class Green; end class Yellow; end end + + module Orange + class OrangeReport; end + end end end @@ -17,10 +21,6 @@ describe Reporting::ReportLoader do let(:report_base_class) { Reporting::Reports::Bananas::Base } let(:report_subtypes) { ["green", "yellow"] } - before do - allow(report_base_class).to receive(:report_subtypes).and_return(report_subtypes) - end - describe "#report_class" do describe "given report type and subtype" do let(:arguments) { ["bananas", "yellow"] } @@ -30,18 +30,17 @@ describe Reporting::ReportLoader do end end - describe "given report type only" do - context "when the report has multiple subtypes" do - let(:arguments) { ["bananas"] } + describe "given report type and subtype for old reports" do + let(:arguments) { ["orange", "subtype"] } - it "returns first listed report type" do - expect(service.report_class).to eq Reporting::Reports::Bananas::Green - end + it "returns a report class when given type and subtype" do + expect(service.report_class).to eq Reporting::Reports::Orange::OrangeReport end + end + describe "given report type only" do context "when the report has no subtypes" do let(:arguments) { ["bananas"] } - let(:report_subtypes) { [] } it "returns base class" do expect(service.report_class).to eq Reporting::Reports::Bananas::Base @@ -50,7 +49,6 @@ describe Reporting::ReportLoader do context "given a report type that does not exist" do let(:arguments) { ["apples"] } - let(:report_subtypes) { [] } it "raises an error" do expect{ service.report_class }.to raise_error(Reporting::Errors::ReportNotFound) @@ -58,51 +56,4 @@ describe Reporting::ReportLoader do end end end - - describe "#default_report_subtype" do - context "when the report has multiple subtypes" do - let(:arguments) { ["bananas"] } - - it "returns the first report type" do - expect(service.default_report_subtype).to eq report_base_class.report_subtypes.first - end - end - - context "when the report has no subtypes" do - let(:arguments) { ["bananas"] } - let(:report_subtypes) { [] } - - it "returns base" do - expect(service.default_report_subtype).to eq "base" - end - end - - context "given a report type that does not exist" do - let(:arguments) { ["apples"] } - let(:report_subtypes) { [] } - - it "raises an error" do - expect{ service.report_class }.to raise_error(Reporting::Errors::ReportNotFound) - end - end - end - - describe "#report_subtypes" do - context "when the report has multiple subtypes" do - let(:arguments) { ["bananas"] } - - it "returns a list of report subtypes for a given report" do - expect(service.report_subtypes).to eq report_subtypes - end - end - - context "when the report has no subtypes" do - let(:arguments) { ["bananas"] } - let(:report_subtypes) { [] } - - it "returns an empty array" do - expect(service.report_subtypes).to eq [] - end - end - end end diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 07805b6f9a..c4f754f6d9 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -5,8 +5,6 @@ require 'cancan/matchers' require 'support/ability_helpers' describe Spree::Ability do - include ::AbilityHelper - let(:user) { create(:user) } let(:subject) { Spree::Ability.new(user) } let(:token) { nil } @@ -441,8 +439,11 @@ describe Spree::Ability do it "should be able to read some reports" do is_expected.to have_ability( - [:admin, :index, :customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory, - :order_cycle_management], for: Spree::Admin::ReportsController + [:admin, :index, :show], for: Admin::ReportsController + ) + is_expected.to have_ability( + [:customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory, + :order_cycle_management], for: :report ) end @@ -451,7 +452,7 @@ describe Spree::Ability do it "should not be able to read other reports" do is_expected.not_to have_ability( [:group_buys, :payments, :orders_and_distributors, :users_and_enterprises, - :xero_invoices], for: Spree::Admin::ReportsController + :xero_invoices], for: :report ) end @@ -671,8 +672,12 @@ describe Spree::Ability do it "should be able to read some reports" do is_expected.to have_ability( - [:admin, :index, :customers, :sales_tax, :group_buys, :bulk_coop, :payments, - :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :xero_invoices], for: Spree::Admin::ReportsController + [:admin, :index, :show], for: Admin::ReportsController + ) + is_expected.to have_ability( + [:customers, :sales_tax, :group_buys, :bulk_coop, :payments, + :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, + :order_cycle_management, :xero_invoices], for: :report ) end @@ -680,7 +685,7 @@ describe Spree::Ability do it "should not be able to read other reports" do is_expected.not_to have_ability([:users_and_enterprises], - for: Spree::Admin::ReportsController) + for: :report) end it "should be able to access customer actions" do diff --git a/spec/support/ability_helper.rb b/spec/support/ability_helper.rb deleted file mode 100644 index aa1d17fd14..0000000000 --- a/spec/support/ability_helper.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module AbilityHelper - shared_examples "allows access to Enterprise Fee Summary" do - it "should be able to see link and read report" do - is_expected.to have_link_to_enterprise_fee_summary - is_expected.to have_direct_access_to_enterprise_fee_summary - end - - def have_link_to_enterprise_fee_summary - have_ability([:enterprise_fee_summary], for: Spree::Admin::ReportsController) - end - - def have_direct_access_to_enterprise_fee_summary - have_ability([:admin, :new, :create], for: :enterprise_fee_summary) - end - end -end diff --git a/spec/support/ability_helpers.rb b/spec/support/ability_helpers.rb index 4dcb840fe7..c474c8363d 100644 --- a/spec/support/ability_helpers.rb +++ b/spec/support/ability_helpers.rb @@ -105,3 +105,18 @@ shared_examples_for 'update only' do expect(subject).to_not be_able_to(:index, resource) end end + +shared_examples "allows access to Enterprise Fee Summary" do + it "should be able to see link and read report" do + is_expected.to have_link_to_enterprise_fee_summary + is_expected.to have_direct_access_to_enterprise_fee_summary + end + + def have_link_to_enterprise_fee_summary + have_ability([:enterprise_fee_summary], for: :report) + end + + def have_direct_access_to_enterprise_fee_summary + have_ability([:show, :index], for: Admin::ReportsController) + end +end diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index f2c372d129..9ffcccaa97 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -33,7 +33,7 @@ describe ' describe "Customers report" do before do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path end it "customers report" do @@ -64,7 +64,7 @@ describe ' describe "Order cycle management report" do before do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path end it "payment method report" do @@ -92,7 +92,7 @@ describe ' end it "orders and distributors report" do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Orders And Distributors' click_button 'Go' @@ -100,7 +100,7 @@ describe ' end it "payments reports" do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Payment Reports' click_button 'Go' @@ -162,7 +162,7 @@ describe ' payment_method: create(:payment_method, distributors: [distributor1])) break unless order1.next! until order1.complete? - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link "Sales Tax" select("Tax Types", from: "report_subtype") end @@ -198,7 +198,7 @@ describe ' describe "orders & fulfilment reports" do it "loads the report page" do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Orders & Fulfillment Reports' expect(page).to have_content 'Supplier' @@ -293,7 +293,7 @@ describe ' end it "shows products and inventory report" do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path expect(page).to have_content "All products" expect(page).to have_content "Inventory (on hand)" @@ -321,7 +321,7 @@ describe ' end it "shows the LettuceShare report" do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'LettuceShare' click_button "Go" @@ -341,7 +341,7 @@ describe ' before do enterprise3.enterprise_roles.build( user: enterprise1.owner ).save - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Users & Enterprises' end @@ -382,7 +382,7 @@ describe ' describe 'bulk coop report' do before do - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Bulk Co-Op' end @@ -539,7 +539,7 @@ describe ' order1.reload order1.create_tax_charge! - login_as_admin_and_visit spree.admin_reports_path + login_as_admin_and_visit admin_reports_path click_link 'Xero Invoices' end