From 97b22ec65de3851f02a9d091a4b40cc7e3ad29c2 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 5 Sep 2021 10:54:08 +0100
Subject: [PATCH] Update related specs

---
 .../api/v0/reports/packing_report_spec.rb     |  20 +--
 spec/features/admin/reports_spec.rb           |  77 ---------
 .../reports/packing/packing_report_spec.rb    | 149 ++++++++++++------
 spec/lib/reports/report_renderer_spec.rb      |  37 ++---
 .../admin/reports/packing_report_spec.rb      | 102 +++++++-----
 5 files changed, 176 insertions(+), 209 deletions(-)

diff --git a/spec/controllers/api/v0/reports/packing_report_spec.rb b/spec/controllers/api/v0/reports/packing_report_spec.rb
index 25bb8eba91..193992dcf1 100644
--- a/spec/controllers/api/v0/reports/packing_report_spec.rb
+++ b/spec/controllers/api/v0/reports/packing_report_spec.rb
@@ -12,19 +12,10 @@ describe Api::V0::ReportsController, type: :controller do
 
   before do
     allow(controller).to receive(:spree_current_user) { current_user }
+    order.finalize!
   end
 
   describe "packing report" do
-    context "as a regular user" do
-      let(:current_user) { create(:user) }
-
-      it "does not show reports" do
-        api_get :show, params
-
-        assert_unauthorized!
-      end
-    end
-
     context "as an enterprise user with full order permissions (distributor)" do
       let!(:distributor) { create(:distributor_enterprise) }
       let!(:order) { create(:completed_order_with_totals, distributor: distributor) }
@@ -78,7 +69,7 @@ describe Api::V0::ReportsController, type: :controller do
       "product" => line_item.product.name,
       "variant" => line_item.full_name,
       "quantity" => line_item.quantity,
-      "is_temperature_controlled" =>
+      "temp_controlled" =>
         line_item.product.shipping_category&.temperature_controlled ? I18n.t(:yes) : I18n.t(:no)
     }
   end
@@ -93,23 +84,22 @@ describe Api::V0::ReportsController, type: :controller do
       "product" => line_item.product.name,
       "variant" => line_item.full_name,
       "quantity" => line_item.quantity,
-      "is_temperature_controlled" =>
+      "temp_controlled" =>
         line_item.product.shipping_category&.temperature_controlled ? I18n.t(:yes) : I18n.t(:no)
     }
   end
 
   def summary_row(order)
     {
-      "summary_row_title" => I18n.t("summary_row.total", scope: i18n_scope),
       "hub" => "",
       "customer_code" => "",
       "first_name" => "",
       "last_name" => "",
       "supplier" => "",
-      "product" => "",
+      "product" => I18n.t("total_items", scope: i18n_scope),
       "variant" => "",
       "quantity" => order.line_items.sum(&:quantity),
-      "is_temperature_controlled" => "",
+      "temp_controlled" => "",
     }
   end
 
diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index b714f7337e..f18383f2db 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -91,83 +91,6 @@ describe '
     end
   end
 
-  describe "Packing reports" do
-    before do
-      login_as_admin_and_visit spree.admin_reports_path
-    end
-
-    let(:bill_address1) { create(:address, lastname: "MULLER") }
-    let(:bill_address2) { create(:address, lastname: "Mistery") }
-    let(:distributor_address) {
-      create(:address, address1: "distributor address", city: 'The Shire', zipcode: "1234")
-    }
-    let(:distributor) { create(:distributor_enterprise, address: distributor_address) }
-    let(:order1) { create(:order, distributor: distributor, bill_address: bill_address1) }
-    let(:order2) { create(:order, distributor: distributor, bill_address: bill_address2) }
-    let(:supplier) { create(:supplier_enterprise, name: "Supplier") }
-    let(:product_1) { create(:simple_product, name: "Product 1", supplier: supplier ) }
-    let(:variant_1) { create(:variant, product: product_1, unit_description: "Big") }
-    let(:variant_2) { create(:variant, product: product_1, unit_description: "Small") }
-    let(:product_2) { create(:simple_product, name: "Product 2", supplier: supplier) }
-
-    before do
-      Timecop.travel(Time.zone.local(2013, 4, 25, 14, 0, 0)) { order1.finalize! }
-      Timecop.travel(Time.zone.local(2013, 4, 25, 15, 0, 0)) { order2.finalize! }
-
-      create(:line_item_with_shipment, variant: variant_1, quantity: 1, order: order1)
-      create(:line_item_with_shipment, variant: variant_2, quantity: 3, order: order1)
-      create(:line_item_with_shipment, variant: product_2.master, quantity: 3, order: order2)
-    end
-
-    it "Pack By Customer" do
-      click_link "Pack By Customer"
-      fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
-      fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
-      # select 'Pack By Customer', from: 'report_type'
-      click_button 'Search'
-
-      rows = find("table#listing_orders").all("thead tr")
-      table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
-      expect(table.sort).to eq([
-        ["Hub", "Code", "First Name", "Last Name", "Supplier", "Product", "Variant", "Quantity",
-         "TempControlled?"]
-      ].sort)
-      expect(page).to have_selector 'table#listing_orders tbody tr', count: 5 # Totals row per order
-    end
-
-    it "Alphabetically Sorted Pack by Customer" do
-      click_link "Pack By Customer"
-      click_button 'Search'
-
-      rows = find("table#listing_orders").all("tr")
-      table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[3] }
-      expect(table).to eq([
-                            "Last Name",
-                            order2.bill_address.lastname,
-                            "",
-                            order1.bill_address.lastname,
-                            order1.bill_address.lastname,
-                            ""
-                          ])
-    end
-
-    it "Pack By Supplier" do
-      click_link "Pack By Supplier"
-      fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
-      fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
-      # select 'Pack By Customer', from: 'report_type'
-      click_button 'Search'
-
-      rows = find("table#listing_orders").all("thead tr")
-      table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
-      expect(table.sort).to eq([
-        ["Hub", "Supplier", "Code", "First Name", "Last Name", "Product", "Variant", "Quantity",
-         "TempControlled?"]
-      ].sort)
-      expect(all('table#listing_orders tbody tr').count).to eq(4) # Totals row per supplier
-    end
-  end
-
   it "orders and distributors report" do
     login_as_admin_and_visit spree.admin_reports_path
     click_link 'Orders And Distributors'
diff --git a/spec/lib/reports/packing/packing_report_spec.rb b/spec/lib/reports/packing/packing_report_spec.rb
index 7087bacfdf..89a706e783 100644
--- a/spec/lib/reports/packing/packing_report_spec.rb
+++ b/spec/lib/reports/packing/packing_report_spec.rb
@@ -9,103 +9,148 @@ describe "Packing Reports" do
     let(:distributor) { create(:distributor_enterprise) }
     let(:order_cycle) { create(:simple_order_cycle) }
     let(:order) {
-      create(:order, completed_at: 1.day.ago, order_cycle: order_cycle, distributor: distributor)
+      create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor,
+                                           line_items_count: 0)
     }
     let(:line_item) { build(:line_item_with_shipment) }
+    let(:user) { create(:admin_user) }
+    let(:params) { {} }
 
-    before { order.line_items << line_item }
+    let(:report_data) { subject.report_data.as_json }
+    let(:report_contents) { subject.report_data.rows.flatten }
+    let(:row_count) { subject.report_data.rows.count }
+
+    subject { Reporting::Reports::Packing::Customer.new user, params }
+
+    before do
+      order.line_items << line_item
+      order.finalize!
+    end
 
     context "as a site admin" do
-      let(:user) { create(:admin_user) }
-      subject { Reports::Packing::Customer.new user, {} }
+      let(:cancelled_order) { create(:completed_order_with_totals, line_items_count: 0) }
+      let(:line_item2) { build(:line_item_with_shipment) }
 
-      it "fetches completed orders" do
-        order2 = create(:order)
-        order2.line_items << build(:line_item)
-        expect(subject.collection).to eq([line_item])
+      before do
+        cancelled_order.line_items << line_item2
+        cancelled_order.finalize!
+        cancelled_order.cancel!
       end
 
-      it "does not show cancelled orders" do
-        order2 = create(:order, state: "canceled", completed_at: 1.day.ago)
-        order2.line_items << build(:line_item_with_shipment)
-        expect(subject.collection).to eq([line_item])
+      it "fetches line items for completed orders" do
+        expect(report_contents).to include line_item.product.name
+      end
+
+      it "does not fetch line items for cancelled orders" do
+        expect(report_contents).to_not include line_item2.product.name
       end
     end
 
     context "as a manager of a supplier" do
       let!(:user) { create(:user) }
-      subject { Reports::Packing::Customer.new user, {} }
-
       let(:supplier) { create(:supplier_enterprise) }
+      let(:order2) {
+        create(:completed_order_with_totals, distributor: distributor,
+                                             bill_address: create(:address),
+                                             ship_address: create(:address))
+      }
+      let(:line_item2) {
+        build(:line_item_with_shipment, product: create(:simple_product, supplier: supplier))
+      }
 
       before do
+        order2.line_items << line_item2
+        order2.finalize!
         supplier.enterprise_roles.create!(user: user)
       end
 
-      context "that has granted P-OC to the distributor" do
-        let(:order2) {
-          create(:order, distributor: distributor, completed_at: 1.day.ago,
-                         bill_address: create(:address), ship_address: create(:address))
-        }
-        let(:line_item2) {
-          build(:line_item_with_shipment, product: create(:simple_product, supplier: supplier))
-        }
+      context "which has not granted P-OC to the distributor" do
+        it "does not show line items supplied by my producers" do
+          expect(row_count).to eq 0
+        end
+      end
 
+      context "which has granted P-OC to the distributor" do
         before do
-          order2.line_items << line_item2
           create(:enterprise_relationship, parent: supplier, child: distributor,
                                            permissions_list: [:add_to_order_cycle])
         end
 
         it "shows line items supplied by my producers, with names hidden" do
-          expect(subject.collection).to eq([line_item2])
-          expect(subject.as_json.first[:first_name]).to eq(
+          expect(report_contents).to include line_item2.product.name
+          expect(report_data.first["first_name"]).to eq(
             I18n.t('admin.reports.hidden_field')
           )
         end
-      end
 
-      context "that has not granted P-OC to the distributor" do
-        let(:order2) {
-          create(:order, distributor: distributor, completed_at: 1.day.ago,
-                         bill_address: create(:address), ship_address: create(:address))
-        }
-        let(:line_item2) {
-          build(:line_item_with_shipment, product: create(:simple_product, supplier: supplier))
-        }
+        context "where the distributor allows suppliers to see customer names" do
+          before do
+            distributor.update_columns show_customer_names_to_suppliers: true
+          end
 
-        before do
-          order2.line_items << line_item2
-        end
-
-        it "does not show line items supplied by my producers" do
-          expect(subject.collection).to eq([])
+          it "shows line items supplied by my producers, with names shown" do
+            expect(report_data.first["first_name"]).to eq(order2.bill_address.firstname)
+          end
         end
       end
     end
 
     context "as a manager of a distributor" do
       let!(:user) { create(:user) }
-      subject { Reports::Packing::Customer.new user, {} }
+      let(:distributor2) { create(:distributor_enterprise) }
+      let(:order3) {
+        create(:completed_order_with_totals, distributor: distributor2,
+                                             line_items_count: 0)
+      }
+      let(:line_item3) { build(:line_item_with_shipment) }
 
       before do
+        order3.line_items << line_item3
+        order3.finalize!
         distributor.enterprise_roles.create!(user: user)
       end
 
       it "only shows line items distributed by enterprises managed by the current user" do
-        distributor2 = create(:distributor_enterprise)
-        distributor2.enterprise_roles.create!(user: create(:user))
-        order2 = create(:order, distributor: distributor2, completed_at: 1.day.ago)
-        order2.line_items << build(:line_item_with_shipment)
-        expect(subject.collection).to eq([line_item])
+        expect(report_contents).to include line_item.product.name
+        expect(report_contents).to_not include line_item3.product.name
       end
 
-      it "only shows the selected order cycle" do
-        order_cycle2 = create(:simple_order_cycle)
-        order2 = create(:order, distributor: distributor, order_cycle: order_cycle2)
-        order2.line_items << build(:line_item)
-        allow(subject).to receive(:params).and_return(order_cycle_id_in: order_cycle.id)
-        expect(subject.collection).to eq([line_item])
+      context "filtering by order cycle" do
+        let(:order_cycle2) { create(:simple_order_cycle) }
+        let(:order4) {
+          create(:completed_order_with_totals, distributor: distributor, order_cycle: order_cycle2,
+                                               line_items_count: 0)
+        }
+        let(:line_item4) { build(:line_item_with_shipment) }
+        let(:params) { { order_cycle_id_in: order_cycle.id } }
+
+        before do
+          order4.line_items << line_item4
+          order4.finalize!
+        end
+
+        it "only shows results from the selected order cycle" do
+          expect(report_contents).to include line_item.product.name
+          expect(report_contents).to_not include line_item4.product.name
+        end
+      end
+    end
+
+    describe "ordering and grouping" do
+      let(:distributor2) { create(:distributor_enterprise) }
+      let(:order2) {
+        create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor2,
+                                             line_items_count: 2)
+      }
+
+      before do
+        order2.finalize!
+      end
+
+      it "groups and orders by distributor and order" do
+        expect(subject.report_data.rows.map(&:first)).to eq(
+          [order.distributor.name, "", order2.distributor.name, order2.distributor.name, ""]
+        )
       end
     end
   end
diff --git a/spec/lib/reports/report_renderer_spec.rb b/spec/lib/reports/report_renderer_spec.rb
index 2566ccf30a..2377c96950 100644
--- a/spec/lib/reports/report_renderer_spec.rb
+++ b/spec/lib/reports/report_renderer_spec.rb
@@ -2,19 +2,21 @@
 
 require 'spec_helper'
 
-describe Reports::ReportRenderer do
-  let(:report_rows) {
+describe Reporting::ReportRenderer do
+  let(:data) {
     [
-      { id: 1, name: 'carrots', quantity: 3 },
-      { id: 2, name: 'onions', quantity: 6 }
+      { "id" => 1, "name" => "carrots", "quantity" => 3 },
+      { "id" => 2, "name" => "onions", "quantity" => 6 }
     ]
   }
-  let(:report) { OpenStruct.new(report_rows: report_rows) }
+  let(:report_data) { ActiveRecord::Result.new(data.first.keys, data.map(&:values)) }
+  let(:report) { OpenStruct.new(report_data: report_data)
+  }
   let(:service) { described_class.new(report) }
 
   describe "#table_headers" do
     it "returns the report's table headers" do
-      expect(service.table_headers).to eq [:id, :name, :quantity]
+      expect(service.table_headers).to eq ["id", "name", "quantity"]
     end
   end
 
@@ -29,37 +31,18 @@ describe Reports::ReportRenderer do
 
   describe "#as_json" do
     it "returns the report's data as hashes" do
-      expect(service.as_json).to eq report_rows
+      expect(service.as_json).to eq data.as_json
     end
   end
 
   describe "#as_arrays" do
     it "returns the report's data as arrays" do
       expect(service.as_arrays).to eq [
-        [:id, :name, :quantity],
+        ["id", "name", "quantity"],
         [1, "carrots", 3],
         [2, "onions", 6]
       ]
     end
-
-    context "with summary rows" do
-      let(:report_rows) {
-        [
-          { id: 1, name: 'carrots', quantity: 3 },
-          { id: 2, name: 'onions', quantity: 6 },
-          { id: nil, name: nil, quantity: 9, summary_row_title: "TOTAL" }
-        ]
-      }
-
-      it "returns the report's data as arrays" do
-        expect(service.as_arrays).to eq [
-          [:id, :name, :quantity],
-          [1, "carrots", 3],
-          [2, "onions", 6],
-          ["TOTAL", nil, 9]
-        ]
-      end
-    end
   end
 
   describe "exporting to different formats" do
diff --git a/spec/system/admin/reports/packing_report_spec.rb b/spec/system/admin/reports/packing_report_spec.rb
index 3ca76997ef..dcd5660641 100644
--- a/spec/system/admin/reports/packing_report_spec.rb
+++ b/spec/system/admin/reports/packing_report_spec.rb
@@ -12,59 +12,83 @@ describe "Packing Reports", js: true do
       visit spree.admin_reports_path
     end
 
-    let(:bill_address1) { create(:address, lastname: "Aman") }
-    let(:bill_address2) { create(:address, lastname: "Bman") }
+    let(:bill_address1) { create(:address, lastname: "MULLER") }
+    let(:bill_address2) { create(:address, lastname: "Mistery") }
     let(:distributor_address) {
       create(:address, address1: "distributor address", city: 'The Shire', zipcode: "1234")
     }
     let(:distributor) { create(:distributor_enterprise, address: distributor_address) }
-    let(:order1) { create(:order, distributor: distributor, bill_address: bill_address1) }
-    let(:order2) { create(:order, distributor: distributor, bill_address: bill_address2) }
+    let(:order1) {
+      create(:completed_order_with_totals, line_items_count: 0, distributor: distributor,
+                                           bill_address: bill_address1)
+    }
+    let(:order2) {
+      create(:completed_order_with_totals, line_items_count: 0, distributor: distributor,
+                                           bill_address: bill_address2)
+    }
     let(:supplier) { create(:supplier_enterprise, name: "Supplier") }
-    let(:product_1) { create(:simple_product, name: "Product 1", supplier: supplier ) }
-    let(:variant_1) { create(:variant, product: product_1, unit_description: "Big") }
-    let(:variant_2) { create(:variant, product: product_1, unit_description: "Small") }
-    let(:product_2) { create(:simple_product, name: "Product 2", supplier: supplier) }
+    let(:product1) { create(:simple_product, name: "Product 1", supplier: supplier ) }
+    let(:variant1) { create(:variant, product: product1, unit_description: "Big") }
+    let(:variant2) { create(:variant, product: product1, unit_description: "Small") }
+    let(:product2) { create(:simple_product, name: "Product 2", supplier: supplier) }
 
     before do
       Timecop.travel(Time.zone.local(2013, 4, 25, 14, 0, 0)) { order1.finalize! }
       Timecop.travel(Time.zone.local(2013, 4, 25, 15, 0, 0)) { order2.finalize! }
 
-      create(:line_item_with_shipment, variant: variant_1, quantity: 1, order: order1)
-      create(:line_item_with_shipment, variant: variant_2, quantity: 3, order: order1)
-      create(:line_item_with_shipment, variant: product_2.master, quantity: 3, order: order2)
+      create(:line_item_with_shipment, variant: variant1, quantity: 1, order: order1)
+      create(:line_item_with_shipment, variant: variant2, quantity: 3, order: order1)
+      create(:line_item_with_shipment, variant: product2.master, quantity: 3, order: order2)
     end
 
-    scenario "Pack By Customer" do
-      click_link "Pack By Customer"
-      fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
-      fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
-      # select 'Pack By Customer', from: 'report_type'
-      click_button 'Search'
+    describe "Pack By Customer" do
+      it "displays the report" do
+        click_link "Pack By Customer"
+        fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
+        fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
+        click_button 'Search'
 
-      rows = find("table#listing_orders").all("thead tr")
-      table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
-      expect(table).to eq([
-                            ["Hub", "Code", "First Name", "Last Name", "Supplier",
-                             "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
-                          ])
-      expect(page).to have_selector 'table#listing_orders tbody tr', count: 5 # Totals row per order
+        rows = find("table#listing_orders").all("thead tr")
+        table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
+        expect(table).to eq([
+                              ["Hub", "Code", "First Name", "Last Name", "Supplier",
+                               "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
+                            ])
+        expect(page).to have_selector 'table#listing_orders tbody tr', count: 5 # Totals row per order
+      end
+
+      it "sorts alphabetically" do
+        click_link "Pack By Customer"
+        click_button 'Search'
+
+        rows = find("table#listing_orders").all("tr")
+        table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[3] }
+        expect(table).to eq([
+                              "LAST NAME",
+                              order2.bill_address.lastname,
+                              "",
+                              order1.bill_address.lastname,
+                              order1.bill_address.lastname,
+                              ""
+                            ])
+      end
     end
 
-    scenario "Pack By Supplier" do
-      click_link "Pack By Supplier"
-      fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
-      fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
-      # select 'Pack By Customer', from: 'report_type'
-      click_button 'Search'
+    describe "Pack By Supplier" do
+      it "displays the report" do
+        click_link "Pack By Supplier"
+        fill_in 'q_completed_at_gt', with: '2013-04-25 13:00:00'
+        fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
+        click_button 'Search'
 
-      rows = find("table#listing_orders").all("thead tr")
-      table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
-      expect(table).to eq([
-                            ["Hub", "Supplier", "Code", "First Name", "Last Name",
-                             "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
-                          ])
-      expect(all('table#listing_orders tbody tr').count).to eq(4) # Totals row per supplier
+        rows = find("table#listing_orders").all("thead tr")
+        table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
+        expect(table).to eq([
+                              ["Hub", "Supplier", "Code", "First Name", "Last Name",
+                               "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
+                            ])
+        expect(all('table#listing_orders tbody tr').count).to eq(4) # Totals row per supplier
+      end
     end
   end
 
@@ -72,7 +96,8 @@ describe "Packing Reports", js: true do
     let(:distributor) { create(:distributor_enterprise) }
     let(:oc) { create(:simple_order_cycle) }
     let(:order) {
-      create(:order, completed_at: 1.day.ago, order_cycle: oc, distributor: distributor)
+      create(:completed_order_with_totals, line_items_count: 0, completed_at: 1.day.ago,
+                                           order_cycle: oc, distributor: distributor)
     }
     let(:li1) { build(:line_item_with_shipment) }
     let(:li2) { build(:line_item_with_shipment) }
@@ -80,6 +105,7 @@ describe "Packing Reports", js: true do
     before do
       order.line_items << li1
       order.line_items << li2
+      order.finalize!
       login_as_admin
     end