From e20c2e3ced13970751c049dcf82ece858fbb94cd Mon Sep 17 00:00:00 2001 From: Sebastian Castro Date: Sun, 27 Mar 2022 10:05:14 +0000 Subject: [PATCH] Report Refactor 1: Orders & Distributors --- .rubocop_todo.yml | 4 ++-- .../spree/admin/reports_controller.rb | 9 ++------- .../filters/_orders_and_distributors.html.haml | 1 + .../reports/orders_and_distributors.html.haml | 12 ------------ ...ort.rb => orders_and_distributors_report.rb} | 8 ++++---- .../spree/admin/reports_controller_spec.rb | 6 +++--- ...b => orders_and_distributors_report_spec.rb} | 17 ++++++++--------- spec/system/admin/reports_spec.rb | 2 +- 8 files changed, 21 insertions(+), 38 deletions(-) create mode 100644 app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml delete mode 100644 app/views/spree/admin/reports/orders_and_distributors.html.haml rename lib/open_food_network/{order_and_distributor_report.rb => orders_and_distributors_report.rb} (96%) rename spec/lib/open_food_network/{order_and_distributor_report_spec.rb => orders_and_distributors_report_spec.rb} (87%) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index ac1cfe8ee6..657102531e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -423,7 +423,7 @@ Metrics/AbcSize: - 'lib/discourse/single_sign_on.rb' - 'lib/open_food_network/customers_report.rb' - 'lib/open_food_network/group_buy_report.rb' - - 'lib/open_food_network/order_and_distributor_report.rb' + - 'lib/open_food_network/orders_and_distributors_report.rb' - 'lib/open_food_network/order_cycle_form_applicator.rb' - 'lib/open_food_network/order_cycle_permissions.rb' - 'lib/open_food_network/payments_report.rb' @@ -1135,7 +1135,7 @@ Style/OptionalBooleanParameter: - 'engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb' - 'engines/order_management/app/services/order_management/stock/estimator.rb' - 'lib/open_food_network/customers_report.rb' - - 'lib/open_food_network/order_and_distributor_report.rb' + - 'lib/open_food_network/orders_and_distributors_report.rb' - 'lib/open_food_network/order_cycle_management_report.rb' - 'lib/open_food_network/orders_and_fulfillment_report.rb' - 'lib/open_food_network/payments_report.rb' diff --git a/app/controllers/spree/admin/reports_controller.rb b/app/controllers/spree/admin/reports_controller.rb index c12a37f942..231095fa75 100644 --- a/app/controllers/spree/admin/reports_controller.rb +++ b/app/controllers/spree/admin/reports_controller.rb @@ -3,7 +3,7 @@ require 'csv' require 'open_food_network/reports/list' -require 'open_food_network/order_and_distributor_report' +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/group_buy_report' @@ -54,12 +54,7 @@ module Spree end def orders_and_distributors - @report = OpenFoodNetwork::OrderAndDistributorReport.new spree_current_user, - raw_params, - render_content? - @search = @report.search - csv_file_name = "orders_and_distributors_#{timestamp}.csv" - render_report(@report.header, @report.table, params[:csv], csv_file_name) + render_report2 end def sales_tax 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 new file mode 100644 index 0000000000..6d7296bbab --- /dev/null +++ b/app/views/spree/admin/reports/filters/_orders_and_distributors.html.haml @@ -0,0 +1 @@ += render 'spree/admin/reports/date_range_form', f: f diff --git a/app/views/spree/admin/reports/orders_and_distributors.html.haml b/app/views/spree/admin/reports/orders_and_distributors.html.haml deleted file mode 100644 index fae9b26d99..0000000000 --- a/app/views/spree/admin/reports/orders_and_distributors.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -= form_for @search, :url => spree.orders_and_distributors_admin_reports_path do |f| - = render 'date_range_form', f: f - - = check_box_tag :csv - = label_tag :csv, t(:report_customers_csv) - %br - = button t(:search) - -- if render_content? - = render partial: "customer_names_message" - -= render "table", id: "listing_orders", msg_option: t(:search) diff --git a/lib/open_food_network/order_and_distributor_report.rb b/lib/open_food_network/orders_and_distributors_report.rb similarity index 96% rename from lib/open_food_network/order_and_distributor_report.rb rename to lib/open_food_network/orders_and_distributors_report.rb index 9764184b02..252f33f3e0 100644 --- a/lib/open_food_network/order_and_distributor_report.rb +++ b/lib/open_food_network/orders_and_distributors_report.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module OpenFoodNetwork - class OrderAndDistributorReport + class OrdersAndDistributorsReport def initialize(user, params = {}, render_table = false) @params = params @user = user @@ -10,7 +10,7 @@ module OpenFoodNetwork @permissions = ::Permissions::Order.new(user, @params[:q]) end - def header + def table_headers [ I18n.t(:report_header_order_date), I18n.t(:report_header_order_id), @@ -41,7 +41,7 @@ module OpenFoodNetwork ransack(@params[:q]) end - def table + def table_rows return [] unless @render_table orders = search.result @@ -104,7 +104,7 @@ module OpenFoodNetwork order.distributor.address.address1, order.distributor.address.city, order.distributor.address.zipcode, - order.shipping_method.name, + order.shipping_method&.name, order.special_instructions ] end diff --git a/spec/controllers/spree/admin/reports_controller_spec.rb b/spec/controllers/spree/admin/reports_controller_spec.rb index 0c84612e47..f263bba98d 100644 --- a/spec/controllers/spree/admin/reports_controller_spec.rb +++ b/spec/controllers/spree/admin/reports_controller_spec.rb @@ -99,9 +99,9 @@ describe Spree::Admin::ReportsController, type: :controller do it "only shows orders that I have access to" do spree_post :orders_and_distributors - expect(assigns(:search).result).to include(orderA1, orderB1) - expect(assigns(:search).result).not_to include(orderA2) - expect(assigns(:search).result).not_to include(orderB2) + expect(assigns(:report).search.result).to include(orderA1, orderB1) + expect(assigns(:report).search.result).not_to include(orderA2) + expect(assigns(:report).search.result).not_to include(orderB2) end end diff --git a/spec/lib/open_food_network/order_and_distributor_report_spec.rb b/spec/lib/open_food_network/orders_and_distributors_report_spec.rb similarity index 87% rename from spec/lib/open_food_network/order_and_distributor_report_spec.rb rename to spec/lib/open_food_network/orders_and_distributors_report_spec.rb index 80bc46f913..4f97d9d848 100644 --- a/spec/lib/open_food_network/order_and_distributor_report_spec.rb +++ b/spec/lib/open_food_network/orders_and_distributors_report_spec.rb @@ -1,16 +1,15 @@ # frozen_string_literal: true require 'spec_helper' -require 'open_food_network/order_and_distributor_report' +require 'open_food_network/orders_and_distributors_report' module OpenFoodNetwork - describe OrderAndDistributorReport do + describe OrdersAndDistributorsReport do describe 'orders and distributors report' do it 'should return a header row describing the report' do - subject = OrderAndDistributorReport.new nil + subject = OrdersAndDistributorsReport.new nil - header = subject.header - expect(header).to eq( + expect(subject.table_headers).to eq( [ 'Order date', 'Order Id', 'Customer Name', 'Customer Email', 'Customer Phone', 'Customer City', @@ -45,9 +44,9 @@ module OpenFoodNetwork end it 'should denormalise order and distributor details for display as csv' do - subject = OrderAndDistributorReport.new create(:admin_user), {}, true + subject = OrdersAndDistributorsReport.new create(:admin_user), {}, true - table = subject.table + table = subject.table_rows expect(table.size).to eq 1 expect(table[0]).to eq([ @@ -77,9 +76,9 @@ module OpenFoodNetwork it "prints one row per line item" do create(:line_item_with_shipment, order: order) - subject = OrderAndDistributorReport.new(create(:admin_user), {}, true) + subject = OrdersAndDistributorsReport.new(create(:admin_user), {}, true) - table = subject.table + table = subject.table_rows expect(table.size).to eq 2 end end diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 3c8b6f3122..e9551b6935 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -80,7 +80,7 @@ describe ' it "delivery report" do click_link "Delivery Report" - click_button "Search" + click_button "Go" rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([