From 6f5cd98b1b489d0e3d6047524345a723fb3d8cfa Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 27 Oct 2021 15:28:57 +0100
Subject: [PATCH 1/5] Migrate preference to enterprises table

---
 ...140211_add_customer_names_to_enterprise.rb |  6 ++++
 .../20211027140313_migrate_customer_names.rb  | 28 +++++++++++++++++++
 db/schema.rb                                  |  1 +
 3 files changed, 35 insertions(+)
 create mode 100644 db/migrate/20211027140211_add_customer_names_to_enterprise.rb
 create mode 100644 db/migrate/20211027140313_migrate_customer_names.rb

diff --git a/db/migrate/20211027140211_add_customer_names_to_enterprise.rb b/db/migrate/20211027140211_add_customer_names_to_enterprise.rb
new file mode 100644
index 0000000000..a5f4c84028
--- /dev/null
+++ b/db/migrate/20211027140211_add_customer_names_to_enterprise.rb
@@ -0,0 +1,6 @@
+class AddCustomerNamesToEnterprise < ActiveRecord::Migration[6.1]
+  def change
+    add_column :enterprises, :show_customer_names_to_suppliers,
+               :boolean, null: false, default: false
+  end
+end
diff --git a/db/migrate/20211027140313_migrate_customer_names.rb b/db/migrate/20211027140313_migrate_customer_names.rb
new file mode 100644
index 0000000000..e87120b744
--- /dev/null
+++ b/db/migrate/20211027140313_migrate_customer_names.rb
@@ -0,0 +1,28 @@
+class MigrateCustomerNames < ActiveRecord::Migration[6.1]
+  class ApplicationRecord < ActiveRecord::Base
+    self.abstract_class = true
+  end
+  class Enterprise < ApplicationRecord; end
+  module Spree
+    class Preference < ApplicationRecord
+      self.table_name = "spree_preferences"
+      serialize :value
+    end
+  end
+
+  def up
+    migrate_customer_names_preferences!
+  end
+
+  def migrate_customer_names_preferences!
+    Enterprise.where(sells: ["own", "any"]).find_each do |enterprise|
+      return unless Spree::Preference.where(
+        value: true, key: "/enterprise/show_customer_names_to_suppliers/#{enterprise.id}"
+      ).exists?
+
+      enterprise.update_columns(
+        show_customer_names_to_suppliers: true
+      )
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 7a2eafbf15..fa328fbf16 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -210,6 +210,7 @@ ActiveRecord::Schema.define(version: 2021_10_29_174211) do
     t.integer "terms_and_conditions_file_size"
     t.datetime "terms_and_conditions_updated_at"
     t.integer "business_address_id"
+    t.boolean "show_customer_names_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

From 4f5c26635665653f67402338679a64d7b33a9d57 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 27 Oct 2021 16:50:36 +0100
Subject: [PATCH 2/5] Add migration unit test

---
 .../20211027140313_migrate_customer_names.rb  |  2 +-
 .../migrations/migrate_customer_names_spec.rb | 30 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 spec/migrations/migrate_customer_names_spec.rb

diff --git a/db/migrate/20211027140313_migrate_customer_names.rb b/db/migrate/20211027140313_migrate_customer_names.rb
index e87120b744..1b1a949c24 100644
--- a/db/migrate/20211027140313_migrate_customer_names.rb
+++ b/db/migrate/20211027140313_migrate_customer_names.rb
@@ -16,7 +16,7 @@ class MigrateCustomerNames < ActiveRecord::Migration[6.1]
 
   def migrate_customer_names_preferences!
     Enterprise.where(sells: ["own", "any"]).find_each do |enterprise|
-      return unless Spree::Preference.where(
+      next unless Spree::Preference.where(
         value: true, key: "/enterprise/show_customer_names_to_suppliers/#{enterprise.id}"
       ).exists?
 
diff --git a/spec/migrations/migrate_customer_names_spec.rb b/spec/migrations/migrate_customer_names_spec.rb
new file mode 100644
index 0000000000..73fb27d6b2
--- /dev/null
+++ b/spec/migrations/migrate_customer_names_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_relative '../../db/migrate/20211027140313_migrate_customer_names'
+
+describe MigrateCustomerNames do
+  subject { MigrateCustomerNames.new }
+
+  let!(:enterprise1) { create(:enterprise) }
+  let!(:enterprise2) { create(:enterprise) }
+  let!(:enterprise3) { create(:enterprise) }
+  let!(:enterprise4) { create(:enterprise) }
+
+  before do
+    Spree::Preference.create(value: true, value_type: "boolean", key: "/enterprise/show_customer_names_to_suppliers/#{enterprise1.id}")
+    Spree::Preference.create(value: false, value_type: "boolean", key: "/enterprise/show_customer_names_to_suppliers/#{enterprise2.id}")
+    Spree::Preference.create(value: true, value_type: "boolean", key: "/enterprise/show_customer_names_to_suppliers/#{enterprise4.id}")
+  end
+
+  describe '#migrate_customer_names_preferences!' do
+    it "migrates the preference to the enterprise" do
+      subject.migrate_customer_names_preferences!
+
+      expect(enterprise1.reload.show_customer_names_to_suppliers?).to be true
+      expect(enterprise2.reload.show_customer_names_to_suppliers?).to be false
+      expect(enterprise3.reload.show_customer_names_to_suppliers?).to be false # was nil
+      expect(enterprise4.reload.show_customer_names_to_suppliers?).to be true
+    end
+  end
+end

From cf5d964133437d44ba13a72ee1a8a21ea67c57fb Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 27 Oct 2021 17:01:49 +0100
Subject: [PATCH 3/5] Update usages of old preference getters and setters

---
 app/mailers/producer_mailer.rb                            | 2 +-
 app/serializers/api/admin/enterprise_serializer.rb        | 2 +-
 app/services/order_data_masker.rb                         | 2 +-
 app/services/permitted_attributes/enterprise.rb           | 2 +-
 .../admin/enterprises/form/_shop_preferences.html.haml    | 8 ++++----
 spec/controllers/admin/enterprises_controller_spec.rb     | 4 ++--
 .../orders_and_fulfillments_report_spec.rb                | 4 ++--
 spec/lib/open_food_network/packing_report_spec.rb         | 4 ++--
 spec/mailers/producer_mailer_spec.rb                      | 8 ++++----
 spec/services/order_data_masker_spec.rb                   | 4 ++--
 10 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb
index 4a22ef801d..b0abc8209e 100644
--- a/app/mailers/producer_mailer.rb
+++ b/app/mailers/producer_mailer.rb
@@ -77,7 +77,7 @@ class ProducerMailer < Spree::BaseMailer
   end
 
   def set_customer_data(line_items)
-    return unless @coordinator.preferred_show_customer_names_to_suppliers
+    return unless @coordinator.show_customer_names_to_suppliers?
 
     line_items.map do |line_item|
       {
diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb
index 6d45133e4c..75ffeeee55 100644
--- a/app/serializers/api/admin/enterprise_serializer.rb
+++ b/app/serializers/api/admin/enterprise_serializer.rb
@@ -8,7 +8,7 @@ module Api
                  :long_description, :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, :preferred_show_customer_names_to_suppliers,
+                 :preferred_shopfront_order_cycle_order, :show_customer_names_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/order_data_masker.rb b/app/services/order_data_masker.rb
index 0cca1c91d2..e5ae198332 100644
--- a/app/services/order_data_masker.rb
+++ b/app/services/order_data_masker.rb
@@ -15,7 +15,7 @@ class OrderDataMasker
   attr_accessor :order
 
   def customer_names_allowed?
-    order.distributor.preferences[:show_customer_names_to_suppliers]
+    order.distributor.show_customer_names_to_suppliers
   end
 
   def mask_customer_names
diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb
index dbae9dee32..6838448fb8 100644
--- a/app/services/permitted_attributes/enterprise.rb
+++ b/app/services/permitted_attributes/enterprise.rb
@@ -32,7 +32,7 @@ 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,
-        :preferred_show_customer_names_to_suppliers, :preferred_shopfront_product_sorting_method,
+        :show_customer_names_to_suppliers, :preferred_shopfront_product_sorting_method,
       ]
     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 527c85db1b..b10914bdff 100644
--- a/app/views/admin/enterprises/form/_shop_preferences.html.haml
+++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml
@@ -103,8 +103,8 @@
     %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
+    = radio_button :enterprise, :show_customer_names_to_suppliers, true
+    = label :enterprise_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
+    = radio_button :enterprise, :show_customer_names_to_suppliers, false
+    = label :enterprise_show_customer_names_to_suppliers, t('.customer_names_false'), value: :false
diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb
index d8b2e4f781..00bd934860 100644
--- a/spec/controllers/admin/enterprises_controller_spec.rb
+++ b/spec/controllers/admin/enterprises_controller_spec.rb
@@ -165,11 +165,11 @@ describe Admin::EnterprisesController, type: :controller do
       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" } }
+                          enterprise: { show_customer_names_to_suppliers: "1" } }
         spree_post :update, update_params
 
         distributor.reload
-        expect(distributor.preferences[:show_customer_names_to_suppliers]).to eq true
+        expect(distributor.show_customer_names_to_suppliers).to eq true
       end
 
       describe "enterprise properties" do
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 b87d872a0e..51343ddcab 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
@@ -82,7 +82,7 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do
 
         context "where the distributor allows suppliers to see customer names" do
           before do
-            distributor.preferred_show_customer_names_to_suppliers = true
+            distributor.update_columns show_customer_names_to_suppliers: true
           end
 
           it "shows line items supplied by my producers, with names shown" do
@@ -117,7 +117,7 @@ describe OpenFoodNetwork::OrdersAndFulfillmentsReport do
 
         context "where the distributor allows suppliers to see customer names" do
           before do
-            distributor.preferred_show_customer_names_to_suppliers = true
+            distributor.show_customer_names_to_suppliers = true
           end
 
           it "does not show line items supplied by my producers" do
diff --git a/spec/lib/open_food_network/packing_report_spec.rb b/spec/lib/open_food_network/packing_report_spec.rb
index 587f6d2d2a..baa282826b 100644
--- a/spec/lib/open_food_network/packing_report_spec.rb
+++ b/spec/lib/open_food_network/packing_report_spec.rb
@@ -66,7 +66,7 @@ module OpenFoodNetwork
 
           context "where the distributor allows suppliers to see customer names" do
             before do
-              distributor.preferred_show_customer_names_to_suppliers = true
+              distributor.update_columns show_customer_names_to_suppliers: true
             end
 
             it "shows line items supplied by my producers, with names shown" do
@@ -96,7 +96,7 @@ module OpenFoodNetwork
 
           context "where the distributor allows suppliers to see customer names" do
             before do
-              distributor.preferred_show_customer_names_to_suppliers = true
+              distributor.show_customer_names_to_suppliers = true
             end
 
             it "does not show line items supplied by my producers" do
diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb
index ad1480c754..6437b921a3 100644
--- a/spec/mailers/producer_mailer_spec.rb
+++ b/spec/mailers/producer_mailer_spec.rb
@@ -128,9 +128,9 @@ describe ProducerMailer, type: :mailer do
     expect(mail.body.encoded).to include(p1.name)
   end
 
-  context 'when flag preferred_show_customer_names_to_suppliers is true' do
+  context 'when flag show_customer_names_to_suppliers is true' do
     before do
-      order_cycle.coordinator.set_preference(:show_customer_names_to_suppliers, true)
+      order_cycle.coordinator.show_customer_names_to_suppliers = true
     end
 
     it "adds customer names table" do
@@ -160,9 +160,9 @@ describe ProducerMailer, type: :mailer do
     end
   end
 
-  context 'when flag preferred_show_customer_names_to_suppliers is false' do
+  context 'when flag show_customer_names_to_suppliers is false' do
     before do
-      order_cycle.coordinator.set_preference(:show_customer_names_to_suppliers, false)
+      order_cycle.coordinator.show_customer_names_to_suppliers = false
     end
 
     it "does not add customer names table" do
diff --git a/spec/services/order_data_masker_spec.rb b/spec/services/order_data_masker_spec.rb
index 9c98f967e2..edbbad930b 100644
--- a/spec/services/order_data_masker_spec.rb
+++ b/spec/services/order_data_masker_spec.rb
@@ -8,7 +8,7 @@ describe OrderDataMasker do
     let(:order) { create(:order, distributor: distributor, ship_address: create(:address)) }
 
     context 'when displaying customer names is allowed' do
-      before { distributor.preferences[:show_customer_names_to_suppliers] = true }
+      before { distributor.show_customer_names_to_suppliers = true }
 
       it 'masks personal addresses and email' do
         described_class.new(order).call
@@ -49,7 +49,7 @@ describe OrderDataMasker do
     end
 
     context 'when displaying customer names is not allowed' do
-      before { distributor.preferences[:show_customer_names_to_suppliers] = false }
+      before { distributor.show_customer_names_to_suppliers = false }
 
       it 'masks personal addresses and email' do
         described_class.new(order).call

From d46ed5969900a0795795063dd3ba1398091d3fb3 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 28 Oct 2021 14:15:29 +0100
Subject: [PATCH 4/5] Remove preference from Enterprise class

---
 app/models/enterprise.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index 4b8ea17e78..77b49a9110 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -15,7 +15,6 @@ class Enterprise < ApplicationRecord
   preference :shopfront_taxon_order, :string, default: ""
   preference :shopfront_producer_order, :string, default: ""
   preference :shopfront_order_cycle_order, :string, default: "orders_close_at"
-  preference :show_customer_names_to_suppliers, :boolean, default: false
   preference :shopfront_product_sorting_method, :string, default: "by_category"
 
   # Allow hubs to restrict visible variants to only those in their inventory

From 21bf5797acf3d4fbc5cef121f0d61b5e49337e1a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 3 Nov 2021 10:56:15 +0000
Subject: [PATCH 5/5] Improve data migration

---
 .../20211027140313_migrate_customer_names.rb  | 28 ++++++++-----------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/db/migrate/20211027140313_migrate_customer_names.rb b/db/migrate/20211027140313_migrate_customer_names.rb
index 1b1a949c24..c361720dfd 100644
--- a/db/migrate/20211027140313_migrate_customer_names.rb
+++ b/db/migrate/20211027140313_migrate_customer_names.rb
@@ -1,12 +1,14 @@
 class MigrateCustomerNames < ActiveRecord::Migration[6.1]
-  class ApplicationRecord < ActiveRecord::Base
-    self.abstract_class = true
-  end
-  class Enterprise < ApplicationRecord; end
-  module Spree
-    class Preference < ApplicationRecord
-      self.table_name = "spree_preferences"
-      serialize :value
+  class Enterprise < ActiveRecord::Base
+    scope :showing_customer_names, -> do
+      joins(
+        <<-SQL
+          JOIN spree_preferences ON (
+            value LIKE '--- true\n%'
+            AND spree_preferences.key = CONCAT('/enterprise/show_customer_names_to_suppliers/', enterprises.id)
+          )
+        SQL
+      )
     end
   end
 
@@ -15,14 +17,6 @@ class MigrateCustomerNames < ActiveRecord::Migration[6.1]
   end
 
   def migrate_customer_names_preferences!
-    Enterprise.where(sells: ["own", "any"]).find_each do |enterprise|
-      next unless Spree::Preference.where(
-        value: true, key: "/enterprise/show_customer_names_to_suppliers/#{enterprise.id}"
-      ).exists?
-
-      enterprise.update_columns(
-        show_customer_names_to_suppliers: true
-      )
-    end
+    Enterprise.showing_customer_names.update_all(show_customer_names_to_suppliers: true)
   end
 end