Merge pull request #5769 from Matt-Yorkley/customer-names

Customer names in reports

app/assets/stylesheets/admin/reports.scss

@@ -3,6 +3,7 @@
 .report__table {
   margin-top: 2em;
 }
+
 .report__message {
   margin-top: 2em;
   border: 1px solid $pale-blue;
@@ -10,3 +11,7 @@
   padding: .5em;
   text-align: center;
 }
+
+.customer-names-tip {
+  margin-top: 1em;
+}

app/models/enterprise.rb

@@ -6,6 +6,7 @@ class Enterprise < ActiveRecord::Base
   preference :shopfront_closed_message, :text, default: ""
   preference :shopfront_taxon_order, :string, default: ""
   preference :shopfront_order_cycle_order, :string, default: "orders_close_at"
+  preference :show_customer_names_to_suppliers, :boolean, default: false
 
   # This is hopefully a temporary measure, pending the arrival of multiple named inventories
   # for shops. We need this here to allow hubs to restrict visible variants to only those in

app/serializers/api/admin/enterprise_serializer.rb

@@ -4,8 +4,8 @@ class Api::Admin::EnterpriseSerializer < ActiveModel::Serializer
              :preferred_shopfront_message, :preferred_shopfront_closed_message,
              :preferred_shopfront_taxon_order, :preferred_shopfront_order_cycle_order,
              :preferred_product_selection_from_inventory_only,
-             :owner, :contact, :users, :tag_groups, :default_tag_group,
-             :require_login, :allow_guest_orders, :allow_order_changes,
+             :preferred_show_customer_names_to_suppliers, :owner, :contact, :users, :tag_groups,
+             :default_tag_group, :require_login, :allow_guest_orders, :allow_order_changes,
              :logo, :promo_image
 
   has_one :owner, serializer: Api::Admin::UserSerializer

app/services/order_data_masker.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class OrderDataMasker
+  def initialize(order)
+    @order = order
+  end
+
+  def call
+    mask_customer_names unless customer_names_allowed?
+    mask_contact_data
+  end
+
+  private
+
+  attr_accessor :order
+
+  def customer_names_allowed?
+    order.distributor.preferences[:show_customer_names_to_suppliers]
+  end
+
+  def mask_customer_names
+    order.bill_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden'),
+                                          lastname: "")
+    order.ship_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden'),
+                                          lastname: "")
+  end
+
+  def mask_contact_data
+    order.bill_address&.assign_attributes(phone: "", address1: "", address2: "",
+                                          city: "", zipcode: "", state: nil)
+    order.ship_address&.assign_attributes(phone: "", address1: "", address2: "",
+                                          city: "", zipcode: "", state: nil)
+    order.assign_attributes(email: I18n.t('admin.reports.hidden'))
+  end
+end

app/services/permitted_attributes/enterprise.rb

@@ -30,7 +30,7 @@ module PermittedAttributes
         :abn, :acn, :charges_sales_tax, :display_invoice_logo, :invoice_text,
         :preferred_product_selection_from_inventory_only, :preferred_shopfront_message,
         :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order,
-        :preferred_shopfront_order_cycle_order
+        :preferred_shopfront_order_cycle_order, :preferred_show_customer_names_to_suppliers
       ]
     end
   end

app/views/admin/enterprises/form/_shop_preferences.html.haml

@@ -89,3 +89,16 @@
     .five.columns.omega
       = f.radio_button :enable_subscriptions, false
       = f.label :enable_subscriptions, t('.enable_subscriptions_false'), value: :false
+
+.row
+  .alpha.eleven.columns
+    .three.columns.alpha
+      %label= t '.customer_names_in_reports'
+      %div{'ofn-with-tip' => t('.customer_names_tip')}
+        %a= t 'admin.whats_this'
+    .three.columns
+      = radio_button :enterprise, :preferred_show_customer_names_to_suppliers, true
+      = label :enterprise_preferred_show_customer_names_to_suppliers, t('.customer_names_true'), value: :true
+    .five.columns.omega
+      = radio_button :enterprise, :preferred_show_customer_names_to_suppliers, false
+      = label :enterprise_preferred_show_customer_names_to_suppliers, t('.customer_names_false'), value: :false

app/views/spree/admin/reports/_customer_names_message.html.haml

@@ -0,0 +1,2 @@
+%p.customer-names-tip
+  = t(".customer_names_tip")

app/views/spree/admin/reports/orders_and_distributors.html.haml

@@ -6,4 +6,6 @@
   %br
   = button t(:search)
 
+= render partial: "customer_names_message"
+
 = render "table", id: "listing_orders", msg_option: t(:search)

app/views/spree/admin/reports/orders_and_fulfillment.html.haml

@@ -25,4 +25,6 @@
   .row
   = button t(:search)
 
+= render partial: "customer_names_message"
+
 = render "table", id: "listing_orders", msg_option: t(:search)

app/views/spree/admin/reports/packing.html.haml

@@ -25,4 +25,6 @@
   .row
   = button t(:search)
 
+= render partial: "customer_names_message"
+
 = render "table", id: "listing_orders", msg_option: t(:search)

config/locales/en.yml

@@ -778,6 +778,10 @@ en:
           enable_subscriptions_tip: "Enable subscriptions functionality?"
           enable_subscriptions_false: "Disabled"
           enable_subscriptions_true: "Enabled"
+          customer_names_in_reports: "Customer Names in Reports"
+          customer_names_tip: "Enable your suppliers to see your customers names in reports"
+          customer_names_false: "Disabled"
+          customer_names_true: "Enabled"
           shopfront_message: "Shopfront Message"
           shopfront_message_placeholder: >
             An optional message to welcome customers and explain how to shop with you. If text is entered here it will be displayed in a home tab when customers first arrive at your shopfront.
@@ -3403,6 +3407,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
           bulk_coop_allocation: 'Bulk Co-op - Allocation'
           bulk_coop_packing_sheets: 'Bulk Co-op - Packing Sheets'
           bulk_coop_customer_payments: 'Bulk Co-op - Customer Payments'
+        customer_names_message:
+          customer_names_tip: "If customer names are hidden for orders you have supplied, you can contact the distributor and ask if they can update their shop preferences to allow their suppliers to view customer names."
       users:
         index:
           listing_users: "Listing Users"

engines/order_management/app/views/order_management/reports/bulk_coop/_filters.html.haml

@@ -29,3 +29,6 @@
       = label_tag :report_format_csv, t(".report_format_csv")
 
   = button t(".generate_report")
+
+  = render partial: "spree/admin/reports/customer_names_message"
+

engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml

@@ -48,3 +48,5 @@
       = label_tag :report_format_csv, t(".report_format_csv")
 
   = button t(".generate_report")
+
+= render partial: "spree/admin/reports/customer_names_message"

lib/open_food_network/order_and_distributor_report.rb

@@ -45,17 +45,7 @@ module OpenFoodNetwork
       orders = search.result
 
       orders.select{ |order| orders_with_hidden_details(orders).include? order }.each do |order|
-        # 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
-        order.bill_address.andand.
-          assign_attributes(firstname: I18n.t('admin.reports.hidden'),
-                            lastname: "", phone: "", address1: "", address2: "",
-                            city: "", zipcode: "", state: nil)
-        order.ship_address.andand.
-          assign_attributes(firstname: I18n.t('admin.reports.hidden'),
-                            lastname: "", phone: "", address1: "", address2: "",
-                            city: "", zipcode: "", state: nil)
-        order.assign_attributes(email: I18n.t('admin.reports.hidden'))
+        OrderDataMasker.new(order).call
       end
 
       line_item_details orders

lib/open_food_network/reports/line_items.rb

@@ -27,17 +27,7 @@ module OpenFoodNetwork
         line_items.reject{ |li|
           editable_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)
-          line_item.order.ship_address.andand.
-            assign_attributes(firstname: I18n.t('admin.reports.hidden'),
-                              lastname: "", phone: "", address1: "", address2: "",
-                              city: "", zipcode: "", state: nil)
-          line_item.order.assign_attributes(email: I18n.t('admin.reports.hidden'))
+          OrderDataMasker.new(line_item.order).call
         end
 
         line_items

spec/controllers/admin/enterprises_controller_spec.rb

@@ -131,6 +131,15 @@ describe Admin::EnterprisesController, type: :controller do
         expect(distributor.users).to_not include user
       end
 
+      it "updates enterprise preferences" do
+        allow(controller).to receive_messages spree_current_user: distributor_manager
+        update_params = { id: distributor, enterprise: { preferred_show_customer_names_to_suppliers: "1" } }
+        spree_post :update, update_params
+
+        distributor.reload
+        expect(distributor.preferences[:show_customer_names_to_suppliers]).to eq true
+      end
+
       describe "enterprise properties" do
         let(:producer) { create(:enterprise) }
         let!(:property) { create(:property, name: "A nice name") }

spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb

@@ -75,6 +75,18 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do
           expect(subject.table_items).to eq([li2])
           expect(subject.table_items.first.order.bill_address.firstname).to eq("HIDDEN")
         end
+
+        context "where the distributor allows suppliers to see customer names" do
+          before do
+            distributor.preferred_show_customer_names_to_suppliers = true
+          end
+
+          it "shows line items supplied by my producers, with names shown" do
+            expect(subject.table_items).to eq([li2])
+            expect(subject.table_items.first.order.bill_address.firstname).
+              to eq(order.bill_address.firstname)
+          end
+        end
       end
 
       context "that has not granted P-OC to the distributor" do
@@ -96,6 +108,16 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do
         it "does not show line items supplied by my producers" do
           expect(subject.table_items).to eq([])
         end
+
+        context "where the distributor allows suppliers to see customer names" do
+          before do
+            distributor.preferred_show_customer_names_to_suppliers = true
+          end
+
+          it "does not show line items supplied by my producers" do
+            expect(subject.table_items).to eq([])
+          end
+        end
       end
     end
 

spec/lib/open_food_network/packing_report_spec.rb

@@ -6,27 +6,29 @@ include AuthenticationHelper
 module OpenFoodNetwork
   describe PackingReport do
     describe "fetching orders" do
-      let(:d1) { create(:distributor_enterprise) }
-      let(:oc1) { create(:simple_order_cycle) }
-      let(:o1) { create(:order, completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) }
-      let(:li1) { build(:line_item_with_shipment) }
+      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)
+      }
+      let(:line_item) { build(:line_item_with_shipment) }
 
-      before { o1.line_items << li1 }
+      before { order.line_items << line_item }
 
       context "as a site admin" do
         let(:user) { create(:admin_user) }
         subject { PackingReport.new user, {}, true }
 
         it "fetches completed orders" do
-          o2 = create(:order)
-          o2.line_items << build(:line_item)
-          expect(subject.table_items).to eq([li1])
+          order2 = create(:order)
+          order2.line_items << build(:line_item)
+          expect(subject.table_items).to eq([line_item])
         end
 
         it "does not show cancelled orders" do
-          o2 = create(:order, state: "canceled", completed_at: 1.day.ago)
-          o2.line_items << build(:line_item_with_shipment)
-          expect(subject.table_items).to eq([li1])
+          order2 = create(:order, state: "canceled", completed_at: 1.day.ago)
+          order2.line_items << build(:line_item_with_shipment)
+          expect(subject.table_items).to eq([line_item])
         end
       end
 
@@ -34,38 +36,71 @@ module OpenFoodNetwork
         let!(:user) { create(:user) }
         subject { PackingReport.new user, {}, true }
 
-        let(:s1) { create(:supplier_enterprise) }
+        let(:supplier) { create(:supplier_enterprise) }
 
         before do
-          s1.enterprise_roles.create!(user: user)
+          supplier.enterprise_roles.create!(user: user)
         end
 
         context "that has granted P-OC to the distributor" do
-          let(:o2) { create(:order, distributor: d1, completed_at: 1.day.ago, bill_address: create(:address), ship_address: create(:address)) }
-          let(:li2) { build(:line_item_with_shipment, product: create(:simple_product, supplier: s1)) }
+          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))
+          }
 
           before do
-            o2.line_items << li2
-            create(:enterprise_relationship, parent: s1, child: d1, permissions_list: [:add_to_order_cycle])
+            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.table_items).to eq([li2])
+            expect(subject.table_items).to eq([line_item2])
             expect(subject.table_items.first.order.bill_address.firstname).to eq("HIDDEN")
           end
+
+          context "where the distributor allows suppliers to see customer names" do
+            before do
+              distributor.preferred_show_customer_names_to_suppliers = true
+            end
+
+            it "shows line items supplied by my producers, with names shown" do
+              expect(subject.table_items).to eq([line_item2])
+              expect(subject.table_items.first.order.bill_address.firstname).
+                to eq(order2.bill_address.firstname)
+            end
+          end
         end
 
         context "that has not granted P-OC to the distributor" do
-          let(:o2) { create(:order, distributor: d1, completed_at: 1.day.ago, bill_address: create(:address), ship_address: create(:address)) }
-          let(:li2) { build(:line_item_with_shipment, product: create(:simple_product, supplier: s1)) }
+          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))
+          }
 
           before do
-            o2.line_items << li2
+            order2.line_items << line_item2
           end
 
           it "does not show line items supplied by my producers" do
             expect(subject.table_items).to eq([])
           end
+
+          context "where the distributor allows suppliers to see customer names" do
+            before do
+              distributor.preferred_show_customer_names_to_suppliers = true
+            end
+
+            it "does not show line items supplied by my producers" do
+              expect(subject.table_items).to eq([])
+            end
+          end
         end
       end
 
@@ -74,23 +109,23 @@ module OpenFoodNetwork
         subject { PackingReport.new user, {}, true }
 
         before do
-          d1.enterprise_roles.create!(user: user)
+          distributor.enterprise_roles.create!(user: user)
         end
 
         it "only shows line items distributed by enterprises managed by the current user" do
-          d2 = create(:distributor_enterprise)
-          d2.enterprise_roles.create!(user: create(:user))
-          o2 = create(:order, distributor: d2, completed_at: 1.day.ago)
-          o2.line_items << build(:line_item_with_shipment)
-          expect(subject.table_items).to eq([li1])
+          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.table_items).to eq([line_item])
         end
 
         it "only shows the selected order cycle" do
-          oc2 = create(:simple_order_cycle)
-          o2 = create(:order, distributor: d1, order_cycle: oc2)
-          o2.line_items << build(:line_item)
-          allow(subject).to receive(:params).and_return(order_cycle_id_in: oc1.id)
-          expect(subject.table_items).to eq([li1])
+          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.table_items).to eq([line_item])
         end
       end
     end