diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 56ca48bed3..b9c9d3ba98 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -126,7 +126,8 @@ Spree::Admin::ReportsController.class_eval do @include_blank = I18n.t(:all) # -- Build Report with Order Grouper - @report = OpenFoodNetwork::OrdersAndFulfillmentsReport.new spree_current_user, params, render_content? + @report = OpenFoodNetwork::OrdersAndFulfillmentsReport.new(permissions, + params, render_content?) @table = order_grouper_table csv_file_name = "#{params[:report_type]}_#{timestamp}.csv" diff --git a/lib/open_food_network/orders_and_fulfillments_report.rb b/lib/open_food_network/orders_and_fulfillments_report.rb index 79d2c9e74e..0eaf26b54a 100644 --- a/lib/open_food_network/orders_and_fulfillments_report.rb +++ b/lib/open_food_network/orders_and_fulfillments_report.rb @@ -6,9 +6,9 @@ include Spree::ReportsHelper module OpenFoodNetwork class OrdersAndFulfillmentsReport attr_reader :params - def initialize(user, params = {}, render_table = false) + def initialize(permissions, params = {}, render_table = false) @params = params - @user = user + @permissions = permissions @render_table = render_table end @@ -288,6 +288,8 @@ module OpenFoodNetwork private + attr_reader :permissions + def supplier_name proc { |line_items| line_items.first.variant.product.supplier.name } end @@ -304,11 +306,6 @@ module OpenFoodNetwork proc { |line_items| line_items.first.variant.full_name } end - def permissions - return @permissions unless @permissions.nil? - @permissions = OpenFoodNetwork::Permissions.new(@user) - end - def total_units(line_items) return " " if not_all_have_unit?(line_items) diff --git a/lib/open_food_network/reports/line_items.rb b/lib/open_food_network/reports/line_items.rb index 86350a95e3..a7b8951fe6 100644 --- a/lib/open_food_network/reports/line_items.rb +++ b/lib/open_food_network/reports/line_items.rb @@ -12,11 +12,11 @@ module OpenFoodNetwork line_items = permissions.visible_line_items.merge(Spree::LineItem.where(order_id: orders)) line_items = line_items.supplied_by_any(params[:supplier_id_in]) if params[:supplier_id_in].present? - # If empty array is passed in, the where clause will return all line_items, which is bad - line_items_with_hidden_details = - permissions.editable_line_items.empty? ? line_items : line_items.where('"spree_line_items"."id" NOT IN (?)', permissions.editable_line_items) + hidden_line_items = line_items_with_hidden_details(permissions, line_items) - line_items.select{ |li| line_items_with_hidden_details.include? li }.each do |line_item| + line_items.select{ |li| + hidden_line_items.include? li + }.each do |line_item| # TODO We should really be hiding customer code here too, but until we # have an actual association between order and customer, it's a bit tricky line_item.order.bill_address.andand.assign_attributes(firstname: I18n.t('admin.reports.hidden'), lastname: "", phone: "", address1: "", address2: "", city: "", zipcode: "", state: nil) @@ -25,6 +25,16 @@ module OpenFoodNetwork end line_items end + + def self.line_items_with_hidden_details(permissions, line_items) + editable_line_items = permissions.editable_line_items.pluck(:id) + + if editable_line_items.empty? + line_items + else + line_items.where('"spree_line_items"."id" NOT IN (?)', editable_line_items) + end + end end end end diff --git a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb index 35fa8accc6..ce9a90ce29 100644 --- a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb +++ b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb @@ -23,7 +23,7 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do before { order.line_items << line_item } context "as a site admin" do - subject { described_class.new admin_user, {}, true } + subject { described_class.new OpenFoodNetwork::Permissions.new(admin_user), {}, true } it "fetches completed orders" do o2 = create(:order) @@ -39,7 +39,7 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do end context "as a manager of a supplier" do - subject { described_class.new user, {}, true } + subject { described_class.new OpenFoodNetwork::Permissions.new(user), {}, true } let(:s1) { create(:supplier_enterprise) } @@ -98,7 +98,7 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do end context "as a manager of a distributor" do - subject { described_class.new user, {}, true } + subject { described_class.new OpenFoodNetwork::Permissions.new(user), {}, true } before do distributor.enterprise_roles.create!(user: user) @@ -133,7 +133,8 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do ] report_types.each do |report_type| - report = described_class.new admin_user, report_type: report_type + report = described_class.new OpenFoodNetwork::Permissions.new(admin_user), + report_type: report_type expect(report.header.size).to eq(report.columns.size) end end @@ -149,7 +150,9 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do end let(:items) { - report = described_class.new(admin_user, { report_type: "order_cycle_customer_totals" }, true) + report = described_class.new(OpenFoodNetwork::Permissions.new(admin_user), + { report_type: "order_cycle_customer_totals" }, + true) OpenFoodNetwork::OrderGrouper.new(report.rules, report.columns).table(report.table_items) }