diff --git a/app/assets/stylesheets/admin/reports.scss b/app/assets/stylesheets/admin/reports.scss index 1d105eb65b..a338c10f7a 100644 --- a/app/assets/stylesheets/admin/reports.scss +++ b/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; +} diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 0e60be9a14..407d0a71cd 100644 --- a/app/models/enterprise.rb +++ b/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 diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb index e3024fdc46..02eff79426 100644 --- a/app/serializers/api/admin/enterprise_serializer.rb +++ b/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 diff --git a/app/services/order_data_masker.rb b/app/services/order_data_masker.rb new file mode 100644 index 0000000000..0cca1c91d2 --- /dev/null +++ b/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 diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb index db7f31f10c..dc71d76d86 100644 --- a/app/services/permitted_attributes/enterprise.rb +++ b/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 diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index f7062ab07a..d24bb2d291 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/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 diff --git a/app/views/spree/admin/reports/_customer_names_message.html.haml b/app/views/spree/admin/reports/_customer_names_message.html.haml new file mode 100644 index 0000000000..191a3e1aa1 --- /dev/null +++ b/app/views/spree/admin/reports/_customer_names_message.html.haml @@ -0,0 +1,2 @@ +%p.customer-names-tip + = t(".customer_names_tip") diff --git a/app/views/spree/admin/reports/orders_and_distributors.html.haml b/app/views/spree/admin/reports/orders_and_distributors.html.haml index efd7ebbd07..4d99b181af 100644 --- a/app/views/spree/admin/reports/orders_and_distributors.html.haml +++ b/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) diff --git a/app/views/spree/admin/reports/orders_and_fulfillment.html.haml b/app/views/spree/admin/reports/orders_and_fulfillment.html.haml index 91921959ec..38b706da9e 100644 --- a/app/views/spree/admin/reports/orders_and_fulfillment.html.haml +++ b/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) diff --git a/app/views/spree/admin/reports/packing.html.haml b/app/views/spree/admin/reports/packing.html.haml index d8b881c6cd..ba1dcc7ff3 100644 --- a/app/views/spree/admin/reports/packing.html.haml +++ b/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) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8759f90d9b..db3866eb5c 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/engines/order_management/app/views/order_management/reports/bulk_coop/_filters.html.haml b/engines/order_management/app/views/order_management/reports/bulk_coop/_filters.html.haml index 012d910070..4d10fafb3b 100644 --- a/engines/order_management/app/views/order_management/reports/bulk_coop/_filters.html.haml +++ b/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" + diff --git a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml index 0aee85d71e..f63882d7de 100644 --- a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml +++ b/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" diff --git a/lib/open_food_network/order_and_distributor_report.rb b/lib/open_food_network/order_and_distributor_report.rb index 48435e4bd7..1bf78bb067 100644 --- a/lib/open_food_network/order_and_distributor_report.rb +++ b/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 diff --git a/lib/open_food_network/reports/line_items.rb b/lib/open_food_network/reports/line_items.rb index e086fe5c2b..7476dc85bf 100644 --- a/lib/open_food_network/reports/line_items.rb +++ b/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 diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index a51af80b36..b7077ba31e 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/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") } 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 f970c4b83a..fb1da5545f 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 @@ -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 diff --git a/spec/lib/open_food_network/packing_report_spec.rb b/spec/lib/open_food_network/packing_report_spec.rb index 462baf0b58..733f0b9cc9 100644 --- a/spec/lib/open_food_network/packing_report_spec.rb +++ b/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