From 02db30202bb002985278e337aa123f5599fb23e9 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 5 Mar 2025 11:04:53 +1100 Subject: [PATCH 01/12] Use rails form helper methods This helps ensure the labels are attached to the radio buttons, so you can click on the words to select the option. --- .../admin/enterprises/form/_shop_preferences.html.haml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 5b75001514..7a650382cb 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -106,8 +106,8 @@ %div{'ofn-with-tip' => t('.customer_names_tip')} %a= t 'admin.whats_this' .three.columns - = radio_button :enterprise, :show_customer_names_to_suppliers, true - = label :enterprise_show_customer_names_to_suppliers, t('.customer_names_true'), value: :true + = f.radio_button :show_customer_names_to_suppliers, true + = f.label :show_customer_names_to_suppliers, t('.customer_names_true'), value: :true .five.columns.omega - = radio_button :enterprise, :show_customer_names_to_suppliers, false - = label :enterprise_show_customer_names_to_suppliers, t('.customer_names_false'), value: :false + = f.radio_button :show_customer_names_to_suppliers, false + = f.label :show_customer_names_to_suppliers, t('.customer_names_false'), value: :false From 4402854a2a67e1e5dc5c1e0f4f1913607873c7c0 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 5 Mar 2025 12:43:20 +1100 Subject: [PATCH 02/12] AddEnableProducersToEditOrdersToEnterprises With system spec for setting the preference. The enterprise edit page seems under-tested.. --- app/serializers/api/admin/enterprise_serializer.rb | 1 + app/services/permitted_attributes/enterprise.rb | 3 ++- .../enterprises/form/_shop_preferences.html.haml | 12 ++++++++++++ config/locales/en.yml | 4 ++++ ..._customer_contacts_to_suppliers_to_enterprises.rb | 8 ++++++++ db/schema.rb | 4 +++- spec/system/admin/enterprises_spec.rb | 6 ++++++ 7 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20250304234657_add_show_customer_contacts_to_suppliers_to_enterprises.rb diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb index 7c29b41e70..eef391286f 100644 --- a/app/serializers/api/admin/enterprise_serializer.rb +++ b/app/serializers/api/admin/enterprise_serializer.rb @@ -9,6 +9,7 @@ module Api :preferred_shopfront_message, :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order, :preferred_shopfront_producer_order, :preferred_shopfront_order_cycle_order, :show_customer_names_to_suppliers, + :show_customer_contacts_to_suppliers, :preferred_shopfront_product_sorting_method, :owner, :contact, :users, :tag_groups, :default_tag_group, :require_login, :allow_guest_orders, :allow_order_changes, :logo, :promo_image, :terms_and_conditions, diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb index 1dd5b5a838..c25c82a585 100644 --- a/app/services/permitted_attributes/enterprise.rb +++ b/app/services/permitted_attributes/enterprise.rb @@ -33,7 +33,8 @@ module PermittedAttributes :preferred_product_selection_from_inventory_only, :preferred_shopfront_message, :preferred_shopfront_closed_message, :preferred_shopfront_taxon_order, :preferred_shopfront_producer_order, :preferred_shopfront_order_cycle_order, - :show_customer_names_to_suppliers, :preferred_shopfront_product_sorting_method, + :show_customer_names_to_suppliers, :show_customer_contacts_to_suppliers, + :preferred_shopfront_product_sorting_method, :preferred_invoice_order_by_supplier, :preferred_product_low_stock_display, :hide_ofn_navigation, :white_label_logo, :white_label_logo_link, diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 7a650382cb..33f4a5f792 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -111,3 +111,15 @@ .five.columns.omega = f.radio_button :show_customer_names_to_suppliers, false = f.label :show_customer_names_to_suppliers, t('.customer_names_false'), value: :false + +.row + .three.columns.alpha + %label= t '.customer_contacts_in_reports' + %div{'ofn-with-tip' => t('.customer_contacts_tip')} + %a= t 'admin.whats_this' + .three.columns + = f.radio_button :show_customer_contacts_to_suppliers, true + = f.label :show_customer_contacts_to_suppliers, t('.customer_contacts_true'), value: :true + .five.columns.omega + = f.radio_button :show_customer_contacts_to_suppliers, false + = f.label :show_customer_contacts_to_suppliers, t('.customer_contacts_false'), value: :false diff --git a/config/locales/en.yml b/config/locales/en.yml index bc312c91ba..ee18a4014e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1350,6 +1350,10 @@ en: customer_names_tip: "Enable your suppliers to see your customers names in reports" customer_names_false: "Disabled" customer_names_true: "Enabled" + customer_contacts_in_reports: "Customer contact details in reports" + customer_contacts_tip: "Enable your suppliers to see your customer email and phone numbers in reports" + customer_contacts_false: "Disabled" + customer_contacts_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. diff --git a/db/migrate/20250304234657_add_show_customer_contacts_to_suppliers_to_enterprises.rb b/db/migrate/20250304234657_add_show_customer_contacts_to_suppliers_to_enterprises.rb new file mode 100644 index 0000000000..d0d6e5931b --- /dev/null +++ b/db/migrate/20250304234657_add_show_customer_contacts_to_suppliers_to_enterprises.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddShowCustomerContactsToSuppliersToEnterprises < ActiveRecord::Migration[7.0] + def change + add_column :enterprises, :show_customer_contacts_to_suppliers, :boolean, default: false, + null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index a20327a48a..69728aa208 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2025_01_28_031518) do +ActiveRecord::Schema[7.0].define(version: 2025_03_04_234657) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "plpgsql" @@ -230,6 +230,8 @@ ActiveRecord::Schema[7.0].define(version: 2025_01_28_031518) do t.text "white_label_logo_link" t.boolean "hide_groups_tab", default: false t.string "external_billing_id", limit: 128 + t.boolean "enable_producers_to_edit_orders", default: false, null: false + t.boolean "show_customer_contacts_to_suppliers", default: false, null: false t.index ["address_id"], name: "index_enterprises_on_address_id" t.index ["is_primary_producer", "sells"], name: "index_enterprises_on_is_primary_producer_and_sells" t.index ["name"], name: "index_enterprises_on_name", unique: true diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 86d007a46b..acaf29ecd4 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -100,6 +100,10 @@ RSpec.describe ' expect(page).not_to have_checked_field "enterprise_require_login_false" # expect(page).to have_checked_field "enterprise_enable_subscriptions_false" + choose('enterprise[show_customer_contacts_to_suppliers]', option: true) + + # See also "setting ordering preferences" tested separately. + scroll_to(:bottom) accept_alert do scroll_to(:bottom) @@ -216,6 +220,7 @@ RSpec.describe ' "enterprise_preferred_product_selection_from_inventory_only_false" ) + # Save changes click_button 'Update' expect(flash_message).to eq('Enterprise "Eaterprises" has been successfully updated!') @@ -246,6 +251,7 @@ RSpec.describe ' ) expect(page).to have_checked_field "enterprise_require_login_true" expect(page).to have_checked_field "enterprise_enable_subscriptions_true" + expect(page).to have_checked_field 'enterprise[show_customer_contacts_to_suppliers]', with: true # Back navigation loads the tab content page.execute_script('window.history.back()') From 98ab910fb41d9a467f047a208e2613b5ce4d3f4a Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 5 Mar 2025 15:58:45 +1100 Subject: [PATCH 03/12] Refactor --- app/services/orders/mask_data_service.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/services/orders/mask_data_service.rb b/app/services/orders/mask_data_service.rb index 35cbc7482b..3ce4612b79 100644 --- a/app/services/orders/mask_data_service.rb +++ b/app/services/orders/mask_data_service.rb @@ -9,6 +9,7 @@ module Orders def call mask_customer_names unless customer_names_allowed? mask_contact_data + mask_address end private @@ -27,11 +28,16 @@ module Orders 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.bill_address&.assign_attributes(phone: "") + order.ship_address&.assign_attributes(phone: "") order.assign_attributes(email: I18n.t('admin.reports.hidden')) end + + def mask_address + order.bill_address&.assign_attributes(address1: "", address2: "", + city: "", zipcode: "", state: nil) + order.ship_address&.assign_attributes(address1: "", address2: "", + city: "", zipcode: "", state: nil) + end end end From b05a42f3bb4dae186a5061f192774fa33353d875 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 5 Mar 2025 16:13:42 +1100 Subject: [PATCH 04/12] Refactor spec --- .../services/orders/mask_data_service_spec.rb | 82 +++++++++---------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/spec/services/orders/mask_data_service_spec.rb b/spec/services/orders/mask_data_service_spec.rb index aed4ba48a8..17c41fc3a8 100644 --- a/spec/services/orders/mask_data_service_spec.rb +++ b/spec/services/orders/mask_data_service_spec.rb @@ -7,14 +7,37 @@ RSpec.describe Orders::MaskDataService do let(:distributor) { create(:enterprise) } let(:order) { create(:order, distributor:, ship_address: create(:address)) } - context 'when displaying customer names is allowed' do - before { distributor.show_customer_names_to_suppliers = true } - - it 'masks personal addresses and email' do + shared_examples "mask customer name" do + it 'masks the full name' do + described_class.new(order).call + + expect(order.bill_address.attributes).to include( + 'firstname' => 'HIDDEN', + 'lastname' => '' + ) + expect(order.ship_address.attributes).to include( + 'firstname' => 'HIDDEN', + 'lastname' => '' + ) + end + end + + shared_examples "mask customer contact data" do + it 'masks personal phone and email' do + described_class.new(order).call + + expect(order.bill_address.attributes).to include('phone' => '') + expect(order.ship_address.attributes).to include('phone' => '') + + expect(order.email).to eq('HIDDEN') + end + end + + shared_examples "mask customer address" do + it 'masks personal addresses' do described_class.new(order).call expect(order.bill_address.attributes).to include( - 'phone' => '', 'address1' => '', 'address2' => '', 'city' => '', @@ -23,16 +46,20 @@ RSpec.describe Orders::MaskDataService do ) expect(order.ship_address.attributes).to include( - 'phone' => '', 'address1' => '', 'address2' => '', 'city' => '', 'zipcode' => '', 'state_id' => nil ) - - expect(order.email).to eq('HIDDEN') end + end + + context 'when displaying customer names is allowed' do + before { distributor.show_customer_names_to_suppliers = true } + + include_examples "mask customer contact data" + include_examples "mask customer address" it 'does not mask the full name' do described_class.new(order).call @@ -51,42 +78,9 @@ RSpec.describe Orders::MaskDataService do context 'when displaying customer names is not allowed' do before { distributor.show_customer_names_to_suppliers = false } - it 'masks personal addresses and email' do - described_class.new(order).call - - expect(order.bill_address.attributes).to include( - 'phone' => '', - 'address1' => '', - 'address2' => '', - 'city' => '', - 'zipcode' => '', - 'state_id' => nil - ) - - expect(order.ship_address.attributes).to include( - 'phone' => '', - 'address1' => '', - 'address2' => '', - 'city' => '', - 'zipcode' => '', - 'state_id' => nil - ) - - expect(order.email).to eq('HIDDEN') - end - - it 'masks the full name' do - described_class.new(order).call - - expect(order.bill_address.attributes).to include( - 'firstname' => 'HIDDEN', - 'lastname' => '' - ) - expect(order.ship_address.attributes).to include( - 'firstname' => 'HIDDEN', - 'lastname' => '' - ) - end + include_examples "mask customer name" + include_examples "mask customer contact data" + include_examples "mask customer address" end end end From 47b6888fe63ae492aedbdaf17a2fcb6c09177633 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 5 Mar 2025 16:36:31 +1100 Subject: [PATCH 05/12] Display customer emails & phone numbers to suppliers when permitted. The MaskDataService is used by the report framework, so this should affect all reports. It would be nice to test all reports, but I figured it wasn't worth it (already we only test one report for masking names). --- app/services/orders/mask_data_service.rb | 6 ++++- .../orders_and_distributors_report_spec.rb | 11 +++++++++ .../services/orders/mask_data_service_spec.rb | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/services/orders/mask_data_service.rb b/app/services/orders/mask_data_service.rb index 3ce4612b79..78761d3087 100644 --- a/app/services/orders/mask_data_service.rb +++ b/app/services/orders/mask_data_service.rb @@ -8,7 +8,7 @@ module Orders def call mask_customer_names unless customer_names_allowed? - mask_contact_data + mask_contact_data unless cutomer_contacts_allowed? mask_address end @@ -27,6 +27,10 @@ module Orders lastname: "") end + def cutomer_contacts_allowed? + order.distributor.show_customer_contacts_to_suppliers + end + def mask_contact_data order.bill_address&.assign_attributes(phone: "") order.ship_address&.assign_attributes(phone: "") diff --git a/spec/lib/reports/orders_and_distributors_report_spec.rb b/spec/lib/reports/orders_and_distributors_report_spec.rb index c4cd520f15..b8a6e4e50c 100644 --- a/spec/lib/reports/orders_and_distributors_report_spec.rb +++ b/spec/lib/reports/orders_and_distributors_report_spec.rb @@ -134,6 +134,17 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do expect(row[2..5]).to eq [bill_address.full_name, "HIDDEN", "", ""] end end + + context "where the distributor allows suppliers to see customer phone and email" do + before do + distributor.update_columns show_customer_contacts_to_suppliers: true + end + + it "shows line items supplied by my producers, with only contact details shown" do + expect(row).not_to include("FirstName LastName", "City") + expect(row[2..5]).to eq ["HIDDEN", order.email, "123-456", ""] + end + end end it "minimises database queries" do diff --git a/spec/services/orders/mask_data_service_spec.rb b/spec/services/orders/mask_data_service_spec.rb index 17c41fc3a8..c4ad1d74b0 100644 --- a/spec/services/orders/mask_data_service_spec.rb +++ b/spec/services/orders/mask_data_service_spec.rb @@ -82,5 +82,29 @@ RSpec.describe Orders::MaskDataService do include_examples "mask customer contact data" include_examples "mask customer address" end + + context 'when displaying customer contact data is allowed' do + before { distributor.show_customer_contacts_to_suppliers = true } + + include_examples "mask customer name" + include_examples "mask customer address" + + it 'does not mask the phone or email' do + described_class.new(order).call + + expect(order.bill_address.attributes).not_to include('phone' => '') + expect(order.ship_address.attributes).not_to include('phone' => '') + + expect(order.email).not_to eq('HIDDEN') + end + end + + context 'when displaying customer contact data is not allowed' do + before { distributor.show_customer_contacts_to_suppliers = false } + + include_examples "mask customer name" + include_examples "mask customer contact data" + include_examples "mask customer address" + end end end From f40a00dde38f177af9534be6a4f6d2d121b11dce Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 15:10:36 +1100 Subject: [PATCH 06/12] Expand spec to cover existing behaviour And use let to avoid extra db query --- .../reports/packing/packing_report_spec.rb | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/spec/lib/reports/packing/packing_report_spec.rb b/spec/lib/reports/packing/packing_report_spec.rb index dd43bdd37d..de7d20f5c5 100644 --- a/spec/lib/reports/packing/packing_report_spec.rb +++ b/spec/lib/reports/packing/packing_report_spec.rb @@ -89,20 +89,26 @@ RSpec.describe "Packing Reports" do permissions_list: [:add_to_order_cycle]) end - it "shows line items supplied by my producers, with names hidden" do + it "shows line items supplied by my producers, with names and contacts hidden" do expect(report_contents).to include line_item2.product.name - expect(report_data.first["first_name"]).to eq( - '< Hidden >' - ) + row = report_data.first + expect(row["customer_code"]).to eq '< Hidden >' + expect(row["first_name"]).to eq '< Hidden >' + expect(row["last_name"]).to eq '< Hidden >' + expect(row["phone"]).to eq '< Hidden >' end context "where the distributor allows suppliers to see customer names" do - before do - distributor.update_columns show_customer_names_to_suppliers: true - end + let(:distributor) { + create(:distributor_enterprise, show_customer_names_to_suppliers: true) + } - it "shows line items supplied by my producers, with names shown" do - expect(report_data.first["first_name"]).to eq(order2.bill_address.firstname) + it "shows line items supplied by my producers, with names and contacts shown" do + row = report_data.first + expect(row["customer_code"]).to eq order2.customer.code + expect(row["first_name"]).to eq order2.bill_address.firstname + expect(row["last_name"]).to eq order2.bill_address.lastname + expect(row["phone"]).to eq order2.bill_address.phone end end From 38fca53e91e43060c801888644cea9429afd1f5c Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 15:37:03 +1100 Subject: [PATCH 07/12] Refactor Using a more specific name for the mask rule, and making way for a second standardised rule. --- lib/reporting/queries/query_builder.rb | 11 ++++++++--- lib/reporting/reports/packing/base.rb | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/reporting/queries/query_builder.rb b/lib/reporting/queries/query_builder.rb index 23697a6824..3f6de43c43 100644 --- a/lib/reporting/queries/query_builder.rb +++ b/lib/reporting/queries/query_builder.rb @@ -49,9 +49,13 @@ module Reporting reflect query.order(*instance_exec(&ordering_fields)) end + def mask_customer_name(field) + masked(field, nil, managed_order_mask_rule(:show_customer_names_to_suppliers)) + end + def masked(field, message = nil, mask_rule = nil) Case.new. - when(mask_rule || default_mask_rule). + when(mask_rule). then(field). else(quoted(message || I18n.t("hidden_field", scope: i18n_scope))) end @@ -80,10 +84,11 @@ module Reporting private - def default_mask_rule + # Show unmasked data if order is managed by user, or if distributor allows suppliers + def managed_order_mask_rule(condition_name) id = raw("#{managed_orders_alias.name}.id") # rubocop:disable Rails/OutputSafety line_item_table[:order_id].in(id). - or(distributor_alias[:show_customer_names_to_suppliers].eq(true)) + or(distributor_alias[condition_name].eq(true)) end def summary_row_title diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index 1035991f5b..b6c1f4efed 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -42,10 +42,10 @@ module Reporting lambda do { hub: distributor_alias[:name], - customer_code: masked(customer_table[:code]), - last_name: masked(bill_address_alias[:lastname]), - first_name: masked(bill_address_alias[:firstname]), - phone: masked(bill_address_alias[:phone]), + customer_code: mask_customer_name(customer_table[:code]), + last_name: mask_customer_name(bill_address_alias[:lastname]), + first_name: mask_customer_name(bill_address_alias[:firstname]), + phone: mask_customer_name(bill_address_alias[:phone]), supplier: supplier_alias[:name], product: product_table[:name], variant: variant_full_name, From 9c296b691ff5b38121afa7f876a99f99b1f2df75 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 15:38:40 +1100 Subject: [PATCH 08/12] Remove unnecessary parameter We've never needed it and still don't need it for the next feature --- lib/reporting/queries/query_builder.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/reporting/queries/query_builder.rb b/lib/reporting/queries/query_builder.rb index 3f6de43c43..848442d9d2 100644 --- a/lib/reporting/queries/query_builder.rb +++ b/lib/reporting/queries/query_builder.rb @@ -50,14 +50,14 @@ module Reporting end def mask_customer_name(field) - masked(field, nil, managed_order_mask_rule(:show_customer_names_to_suppliers)) + masked(field, managed_order_mask_rule(:show_customer_names_to_suppliers)) end - def masked(field, message = nil, mask_rule = nil) + def masked(field, mask_rule = nil) Case.new. when(mask_rule). then(field). - else(quoted(message || I18n.t("hidden_field", scope: i18n_scope))) + else(quoted(I18n.t("hidden_field", scope: i18n_scope))) end def distinct_results(fields = nil) From 19b6cbcc9bd92ad11e0a5294b7dc8392189930d1 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 15:47:16 +1100 Subject: [PATCH 09/12] Packing reports: Display phone numbers to suppliers --- lib/reporting/queries/query_builder.rb | 4 ++++ lib/reporting/reports/packing/base.rb | 2 +- spec/lib/reports/packing/packing_report_spec.rb | 13 +++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/reporting/queries/query_builder.rb b/lib/reporting/queries/query_builder.rb index 848442d9d2..4e580cbb2c 100644 --- a/lib/reporting/queries/query_builder.rb +++ b/lib/reporting/queries/query_builder.rb @@ -53,6 +53,10 @@ module Reporting masked(field, managed_order_mask_rule(:show_customer_names_to_suppliers)) end + def mask_contact_data(field) + masked(field, managed_order_mask_rule(:show_customer_contacts_to_suppliers)) + end + def masked(field, mask_rule = nil) Case.new. when(mask_rule). diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index b6c1f4efed..a063a84a21 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -45,7 +45,7 @@ module Reporting customer_code: mask_customer_name(customer_table[:code]), last_name: mask_customer_name(bill_address_alias[:lastname]), first_name: mask_customer_name(bill_address_alias[:firstname]), - phone: mask_customer_name(bill_address_alias[:phone]), + phone: mask_contact_data(bill_address_alias[:phone]), supplier: supplier_alias[:name], product: product_table[:name], variant: variant_full_name, diff --git a/spec/lib/reports/packing/packing_report_spec.rb b/spec/lib/reports/packing/packing_report_spec.rb index de7d20f5c5..62188b764e 100644 --- a/spec/lib/reports/packing/packing_report_spec.rb +++ b/spec/lib/reports/packing/packing_report_spec.rb @@ -108,6 +108,19 @@ RSpec.describe "Packing Reports" do expect(row["customer_code"]).to eq order2.customer.code expect(row["first_name"]).to eq order2.bill_address.firstname expect(row["last_name"]).to eq order2.bill_address.lastname + expect(row["phone"]).to eq '< Hidden >' + end + end + + context "where the distributor allows suppliers to see customer contact details" do + let(:distributor) { + create(:distributor_enterprise, show_customer_contacts_to_suppliers: true) + } + + it "shows line items supplied by my producers, with names and contacts shown" do + row = report_data.first + expect(row["first_name"]).to eq '< Hidden >' + expect(row["last_name"]).to eq '< Hidden >' expect(row["phone"]).to eq order2.bill_address.phone end end From 729dc9d658e445bbea267610981f34b5f5674cd4 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 15:58:49 +1100 Subject: [PATCH 10/12] Move mask logic to separate module And cross-reference similar files so we don't miss it next time\! --- app/services/orders/mask_data_service.rb | 3 ++ lib/reporting/queries/mask_data.rb | 36 ++++++++++++++++++++++++ lib/reporting/queries/query_builder.rb | 23 +-------------- 3 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 lib/reporting/queries/mask_data.rb diff --git a/app/services/orders/mask_data_service.rb b/app/services/orders/mask_data_service.rb index 78761d3087..9ec6962dca 100644 --- a/app/services/orders/mask_data_service.rb +++ b/app/services/orders/mask_data_service.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +# Mask user data from suppliers, unless explicitly allowed +# See also: lib/reporting/queries/mask_data.rb +# module Orders class MaskDataService def initialize(order) diff --git a/lib/reporting/queries/mask_data.rb b/lib/reporting/queries/mask_data.rb new file mode 100644 index 0000000000..5ef7ccf1cb --- /dev/null +++ b/lib/reporting/queries/mask_data.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# Mask user data from suppliers, unless explicitly allowed +# See also: app/services/orders/mask_data_service.rb +# +module Reporting + module Queries + module MaskData + include Tables + + def mask_customer_name(field) + masked(field, managed_order_mask_rule(:show_customer_names_to_suppliers)) + end + + def mask_contact_data(field) + masked(field, managed_order_mask_rule(:show_customer_contacts_to_suppliers)) + end + + def masked(field, mask_rule = nil) + Arel::Nodes::Case.new. + when(mask_rule). + then(field). + else(quoted(I18n.t("hidden_field", scope: i18n_scope))) + end + + private + + # Show unmasked data if order is managed by user, or if distributor allows suppliers + def managed_order_mask_rule(condition_name) + id = raw("#{managed_orders_alias.name}.id") # rubocop:disable Rails/OutputSafety + line_item_table[:order_id].in(id). + or(distributor_alias[condition_name].eq(true)) + end + end + end +end diff --git a/lib/reporting/queries/query_builder.rb b/lib/reporting/queries/query_builder.rb index 4e580cbb2c..c659bcecc7 100644 --- a/lib/reporting/queries/query_builder.rb +++ b/lib/reporting/queries/query_builder.rb @@ -5,6 +5,7 @@ module Reporting class QueryBuilder < QueryInterface include Joins include Tables + include MaskData attr_reader :grouping_fields @@ -49,21 +50,6 @@ module Reporting reflect query.order(*instance_exec(&ordering_fields)) end - def mask_customer_name(field) - masked(field, managed_order_mask_rule(:show_customer_names_to_suppliers)) - end - - def mask_contact_data(field) - masked(field, managed_order_mask_rule(:show_customer_contacts_to_suppliers)) - end - - def masked(field, mask_rule = nil) - Case.new. - when(mask_rule). - then(field). - else(quoted(I18n.t("hidden_field", scope: i18n_scope))) - end - def distinct_results(fields = nil) return reflect query.distinct if fields.blank? @@ -88,13 +74,6 @@ module Reporting private - # Show unmasked data if order is managed by user, or if distributor allows suppliers - def managed_order_mask_rule(condition_name) - id = raw("#{managed_orders_alias.name}.id") # rubocop:disable Rails/OutputSafety - line_item_table[:order_id].in(id). - or(distributor_alias[condition_name].eq(true)) - end - def summary_row_title I18n.t("total", scope: i18n_scope) end From e02ef08b0641ab316230bee00a5b932321809376 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 31 Mar 2025 16:24:07 +1100 Subject: [PATCH 11/12] Consolidate translations for hidden field The string '< Hidden >' was agreed on as a good default, so we will use the hidden_field key. I also moved the definition in en.yml up to the more general area at the start of admin.reports section (before it was hidden between report-specific keys. --- app/services/orders/mask_data_service.rb | 6 +++--- config/locales/en.yml | 3 +-- spec/lib/reports/bulk_coop_report_spec.rb | 2 +- spec/lib/reports/line_items_spec.rb | 2 +- .../reports/orders_and_distributors_report_spec.rb | 6 +++--- spec/services/orders/mask_data_service_spec.rb | 12 ++++++------ 6 files changed, 15 insertions(+), 16 deletions(-) diff --git a/app/services/orders/mask_data_service.rb b/app/services/orders/mask_data_service.rb index 9ec6962dca..b3e358c33e 100644 --- a/app/services/orders/mask_data_service.rb +++ b/app/services/orders/mask_data_service.rb @@ -24,9 +24,9 @@ module Orders end def mask_customer_names - order.bill_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden'), + order.bill_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden_field'), lastname: "") - order.ship_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden'), + order.ship_address&.assign_attributes(firstname: I18n.t('admin.reports.hidden_field'), lastname: "") end @@ -37,7 +37,7 @@ module Orders def mask_contact_data order.bill_address&.assign_attributes(phone: "") order.ship_address&.assign_attributes(phone: "") - order.assign_attributes(email: I18n.t('admin.reports.hidden')) + order.assign_attributes(email: I18n.t('admin.reports.hidden_field')) end def mask_address diff --git a/config/locales/en.yml b/config/locales/en.yml index ee18a4014e..3e02ffc334 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1765,7 +1765,7 @@ en: not_visible: "%{enterprise} is not visible and so cannot be found on the map or in searches" reports: deprecated: "This report is deprecated and will be removed in a future release." - hidden: HIDDEN + hidden_field: "< Hidden >" unitsize: UNITSIZE total: TOTAL total_items: TOTAL ITEMS @@ -1843,7 +1843,6 @@ en: no_report_type: "Please specify a report type" report_not_found: "Report not found" missing_ransack_params: "Please supply Ransack search params in the request" - hidden_field: "< Hidden >" summary_row: total: "TOTAL" table: diff --git a/spec/lib/reports/bulk_coop_report_spec.rb b/spec/lib/reports/bulk_coop_report_spec.rb index fb6b64a7aa..b07643f9d9 100644 --- a/spec/lib/reports/bulk_coop_report_spec.rb +++ b/spec/lib/reports/bulk_coop_report_spec.rb @@ -133,7 +133,7 @@ module Reporting it "shows line items supplied by my producers, with names hidden" do expect(subject.table_items).to eq([li2]) - expect(subject.table_items.first.order.bill_address.firstname).to eq("HIDDEN") + expect(subject.table_items.first.order.bill_address.firstname).to eq("< Hidden >") end end diff --git a/spec/lib/reports/line_items_spec.rb b/spec/lib/reports/line_items_spec.rb index 6619634ac2..2fc293973f 100644 --- a/spec/lib/reports/line_items_spec.rb +++ b/spec/lib/reports/line_items_spec.rb @@ -51,7 +51,7 @@ RSpec.describe Reporting::LineItems do it 'returns masked data' do line_items = reports_line_items.list - expect(line_items.first.order.email).to eq('HIDDEN') + expect(line_items.first.order.email).to eq("< Hidden >") end context "when filtering by product" do diff --git a/spec/lib/reports/orders_and_distributors_report_spec.rb b/spec/lib/reports/orders_and_distributors_report_spec.rb index b8a6e4e50c..98f4943f29 100644 --- a/spec/lib/reports/orders_and_distributors_report_spec.rb +++ b/spec/lib/reports/orders_and_distributors_report_spec.rb @@ -120,7 +120,7 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do it "shows line items supplied by my producers, with contact details hidden" do expect(row).not_to include("FirstName LastName") expect(row).not_to include("123-456", "City", order.email) - expect(row[2..5]).to eq ["HIDDEN", "HIDDEN", "", ""] + expect(row[2..5]).to eq ["< Hidden >", "< Hidden >", "", ""] end context "where the distributor allows suppliers to see customer names" do @@ -131,7 +131,7 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do it "shows line items supplied by my producers, with only contact names shown" do expect(row).to include("FirstName LastName") expect(row).not_to include("123-456", "City", order.email) - expect(row[2..5]).to eq [bill_address.full_name, "HIDDEN", "", ""] + expect(row[2..5]).to eq [bill_address.full_name, "< Hidden >", "", ""] end end @@ -142,7 +142,7 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do it "shows line items supplied by my producers, with only contact details shown" do expect(row).not_to include("FirstName LastName", "City") - expect(row[2..5]).to eq ["HIDDEN", order.email, "123-456", ""] + expect(row[2..5]).to eq ["< Hidden >", order.email, "123-456", ""] end end end diff --git a/spec/services/orders/mask_data_service_spec.rb b/spec/services/orders/mask_data_service_spec.rb index c4ad1d74b0..658b0c03ab 100644 --- a/spec/services/orders/mask_data_service_spec.rb +++ b/spec/services/orders/mask_data_service_spec.rb @@ -12,11 +12,11 @@ RSpec.describe Orders::MaskDataService do described_class.new(order).call expect(order.bill_address.attributes).to include( - 'firstname' => 'HIDDEN', + 'firstname' => "< Hidden >", 'lastname' => '' ) expect(order.ship_address.attributes).to include( - 'firstname' => 'HIDDEN', + 'firstname' => "< Hidden >", 'lastname' => '' ) end @@ -29,7 +29,7 @@ RSpec.describe Orders::MaskDataService do expect(order.bill_address.attributes).to include('phone' => '') expect(order.ship_address.attributes).to include('phone' => '') - expect(order.email).to eq('HIDDEN') + expect(order.email).to eq("< Hidden >") end end @@ -65,11 +65,11 @@ RSpec.describe Orders::MaskDataService do described_class.new(order).call expect(order.bill_address.attributes).not_to include( - firstname: 'HIDDEN', + firstname: "< Hidden >", lastname: '' ) expect(order.ship_address.attributes).not_to include( - firstname: 'HIDDEN', + firstname: "< Hidden >", lastname: '' ) end @@ -95,7 +95,7 @@ RSpec.describe Orders::MaskDataService do expect(order.bill_address.attributes).not_to include('phone' => '') expect(order.ship_address.attributes).not_to include('phone' => '') - expect(order.email).not_to eq('HIDDEN') + expect(order.email).not_to eq("< Hidden >") end end From 369b93f0dd465eb9e99e1227df3722e093f717a9 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 7 Apr 2025 16:33:39 +1000 Subject: [PATCH 12/12] Update tip By updating the translation key, all other locales will be notified to update their translations too. --- config/locales/en.yml | 3 +-- lib/reporting/reports/bulk_coop/base.rb | 2 +- lib/reporting/reports/enterprise_fee_summary/fee_summary.rb | 2 +- lib/reporting/reports/orders_and_fulfillment/base.rb | 2 +- lib/reporting/reports/packing/base.rb | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 3e02ffc334..55aeb2a19a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4655,8 +4655,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using reports: table: select_and_search: "Select filters and click on %{option} to access your data." - 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." + hidden_customer_details_tip: "If customer names and/or contact details are hidden, you can ask the distributor to update their shop preferences to allow their suppliers to view customer details in reports." products_and_inventory: all_products: message: "Note that stock levels reported are from supplier product lists only. If you are using Inventory to manage your stock quantities these values will be ignored in this report." diff --git a/lib/reporting/reports/bulk_coop/base.rb b/lib/reporting/reports/bulk_coop/base.rb index bd6b3d03e7..4ebfefdc2d 100644 --- a/lib/reporting/reports/bulk_coop/base.rb +++ b/lib/reporting/reports/bulk_coop/base.rb @@ -5,7 +5,7 @@ module Reporting module BulkCoop class Base < ReportTemplate def message - I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + I18n.t("spree.admin.reports.hidden_customer_details_tip") end def search diff --git a/lib/reporting/reports/enterprise_fee_summary/fee_summary.rb b/lib/reporting/reports/enterprise_fee_summary/fee_summary.rb index 61679e445e..08447fdb85 100644 --- a/lib/reporting/reports/enterprise_fee_summary/fee_summary.rb +++ b/lib/reporting/reports/enterprise_fee_summary/fee_summary.rb @@ -23,7 +23,7 @@ module Reporting end def message - I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + I18n.t("spree.admin.reports.hidden_customer_details_tip") end def query_result diff --git a/lib/reporting/reports/orders_and_fulfillment/base.rb b/lib/reporting/reports/orders_and_fulfillment/base.rb index 0aa6ff3128..f3a9052610 100644 --- a/lib/reporting/reports/orders_and_fulfillment/base.rb +++ b/lib/reporting/reports/orders_and_fulfillment/base.rb @@ -5,7 +5,7 @@ module Reporting module OrdersAndFulfillment class Base < ReportTemplate def message - I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + I18n.t("spree.admin.reports.hidden_customer_details_tip") end def default_params diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index a063a84a21..93105ec0be 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -5,7 +5,7 @@ module Reporting module Packing class Base < ReportQueryTemplate def message - I18n.t("spree.admin.reports.customer_names_message.customer_names_tip") + I18n.t("spree.admin.reports.hidden_customer_details_tip") end def report_query