Merge pull request #4324 from Matt-Yorkley/reports_memoize

Reports memoization

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"
 

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)
 

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

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)
     }