Move frontend preferences out of spree core (#11354)

* Move `always_put_site_name_in_title` and `title_site_name_separator` preference to frontend gem and deprecate `ControllerHelpers::Common`

* Remove logo/mailer_logo configs

Store logos can be uploaded via Admin UI for a long time

* Move `layout` config into frontend gem

* Deprecate max_level_in_taxons_menu config

* Move `show_raw_product_description` preference to frontend gem

* Move `:show_store_selector` preference to frontend gem

* Move `shipping_instructions` preference from core to frontend

* Don't use `storefront_*_path` preferences in frontend gems

* Move WYSIWYG editor preferences from core to backend

* Do not notify about all deprecated preferences

api/lib/spree/api/engine.rb

@@ -17,7 +17,8 @@ class Engine < Rails::Engine
 
       initializer 'spree.api.checking_deprecated_preferences' do
         Spree::Api::Config.deprecated_preferences.each do |pref|
-          warn "[DEPRECATION] Spree::Api::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
+          # FIXME: we should only notify about deprecated preferences that are in use, not all of them
+          # warn "[DEPRECATION] Spree::Api::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
         end
       end
 

backend/app/controllers/spree/admin/general_settings_controller.rb

@@ -26,11 +26,6 @@ def clear_cache
         head :no_content
       end
 
-      def render(*args)
-        @preferences_currency |= [:show_store_selector]
-        super
-      end
-
       private
 
       def update_currency_settings

backend/app/helpers/spree/admin/base_helper.rb

@@ -285,6 +285,14 @@ def flatpickr_local_fallback
           'default'
         end
       end
+
+      def product_wysiwyg_editor_enabled?
+        Spree::Backend::Config[:product_wysiwyg_editor_enabled]
+      end
+
+      def taxon_wysiwyg_editor_enabled?
+        Spree::Backend::Config[:taxon_wysiwyg_editor_enabled]
+      end
     end
   end
 end

backend/app/models/spree/backend_configuration.rb

@@ -3,6 +3,8 @@ class BackendConfiguration < Preferences::Configuration
     preference :locale, :string, default: Rails.application.config.i18n.default_locale
     preference :variants_per_page, :integer, default: Kaminari.config.default_per_page
     preference :menus_per_page, :integer, default: Kaminari.config.default_per_page
+    preference :product_wysiwyg_editor_enabled, :boolean, default: true
+    preference :taxon_wysiwyg_editor_enabled, :boolean, default: true
 
     ORDER_TABS         ||= [:orders, :payments, :creditcard_payments,
                             :shipments, :credit_cards, :return_authorizations,

backend/app/views/spree/admin/general_settings/edit.html.erb

@@ -75,27 +75,6 @@
               </div>
             </div>
           </div>
-
-          <div class="card mb-3" id="general-settings-stores" data-hook="general_settings_stores">
-            <div class="card-header">
-              <h1 class="card-title mb-0 h5">
-                <%= Spree.t(:multiple_stores) %>
-              </h1>
-            </div>
-            <div class="card-body">
-              <div class="alert alert-warning col-12">
-                <%= Spree.t('show_store_selector.long') %>
-              </div>
-              <div class="form-group">
-                <div class="checkbox">
-                  <label>
-                    <%= check_box('', :show_store_selector, { checked: Spree::Config.show_store_selector }) %>
-                    <%= Spree.t('show_store_selector.short') %>
-                  </label>
-                </div>
-              </div>
-            </div>
-          </div>
         </div>
       </div>
 

backend/lib/spree/backend/engine.rb

@@ -14,7 +14,8 @@ class Engine < ::Rails::Engine
 
       initializer 'spree.backend.checking_deprecated_preferences' do
         Spree::Backend::Config.deprecated_preferences.each do |pref|
-          warn "[DEPRECATION] Spree::Backend::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
+          # FIXME: we should only notify about deprecated preferences that are in use, not all of them
+          # warn "[DEPRECATION] Spree::Backend::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
         end
       end
     end

backend/spec/features/admin/general_settings/prices_settings_updating_spec.rb

@@ -4,50 +4,6 @@
   let!(:store) { create(:store, default: true) }
   stub_authorization!
 
-  describe 'enabling currency settings' do
-    before do
-      reset_spree_preferences do |config|
-        config.show_store_selector = false
-      end
-    end
-
-    it 'allows to enable currency settings' do
-      visit spree.edit_admin_general_settings_path
-
-      # Test initial state
-      expect(page).to have_unchecked_field('show_store_selector')
-
-      # Interact with the form
-      check('show_store_selector')
-      click_button 'Update'
-
-      # Test final state
-      expect(page).to have_content 'General Settings has been successfully updated!'
-      expect(page).to have_checked_field('show_store_selector')
-      assert_admin_flash_alert_success('General Settings has been successfully updated!')
-    end
-  end
-
-  describe 'disabling currency settings' do
-    before do
-      reset_spree_preferences do |config|
-        config.show_store_selector = true
-      end
-    end
-
-    it 'allows to disable currency settings' do
-      visit spree.edit_admin_general_settings_path
-
-      expect(page).to have_checked_field('show_store_selector')
-
-      uncheck('show_store_selector')
-      click_button 'Update'
-
-      expect(page).to have_content 'General Settings has been successfully updated!'
-      expect(page).to have_unchecked_field('show_store_selector')
-    end
-  end
-
   describe 'clearing cache' do
     it 'returns flash alert of type success' do
       visit spree.edit_admin_general_settings_path

backend/spec/features/admin/products/edit/products_spec.rb

@@ -7,11 +7,11 @@
 
   context 'editing a product with WYSIWYG disabled' do
     before do
-      Spree::Config.product_wysiwyg_editor_enabled = false
+      Spree::Backend::Config.product_wysiwyg_editor_enabled = false
       visit spree.admin_product_path(product)
     end
 
-    after { Spree::Config.product_wysiwyg_editor_enabled = true }
+    after { Spree::Backend::Config.product_wysiwyg_editor_enabled = true }
 
     it 'displays the product description as a standard input field' do
       expect(page).to have_field(id: 'product_description', with: product.description)
@@ -21,7 +21,7 @@
 
   context 'editing a product with WYSIWYG editer enabled' do
     before do
-      Spree::Config.product_wysiwyg_editor_enabled = true
+      Spree::Backend::Config.product_wysiwyg_editor_enabled = true
       visit spree.admin_product_path(product)
     end
 

backend/spec/features/admin/taxons_spec.rb

@@ -16,9 +16,9 @@
   end
 
   context 'when WYSIWYG editor is disabled' do
-    before { Spree::Config.taxon_wysiwyg_editor_enabled = false }
+    before { Spree::Backend::Config.taxon_wysiwyg_editor_enabled = false }
 
-    after { Spree::Config.taxon_wysiwyg_editor_enabled = true }
+    after { Spree::Backend::Config.taxon_wysiwyg_editor_enabled = true }
 
     it 'displays the taxon description as a standard input field' do
       visit spree.edit_admin_taxonomy_taxon_path(taxonomy, taxonomy.root.id)

core/app/helpers/spree/base_helper.rb

@@ -65,7 +65,7 @@ def logo(image_path = nil, options = {})
                      elsif current_store.logo.attached? && current_store.logo.image?
                        main_app.url_for(current_store.logo)
                      else
-                       Spree::Config[:logo]
+                      'logo/spree_50.png'
                      end
 
       path = spree.respond_to?(:root_path) ? spree.root_path : main_app.root_path
@@ -269,9 +269,5 @@ def meta_robots
 
       tag('meta', name: 'robots', content: current_store.seo_robots)
     end
-
-    def taxon_wysiwyg_editor_enabled?
-      Spree::Config[:taxon_wysiwyg_editor_enabled]
-    end
   end
 end

core/app/helpers/spree/products_helper.rb

@@ -44,20 +44,6 @@ def used_variants_options(variants, product)
       variants_option_types_presenter(variants, product).options
     end
 
-    def product_wysiwyg_editor_enabled?
-      Spree::Config[:product_wysiwyg_editor_enabled]
-    end
-
-    # converts line breaks in product description into <p> tags (for html display purposes)
-    def product_description(product)
-      description = if Spree::Config[:show_raw_product_description] || product_wysiwyg_editor_enabled?
-                      product.description
-                    else
-                      product.description.to_s.gsub(/(.*?)\r?\n\r?\n/m, '<p>\1</p>')
-                    end
-      description.blank? ? Spree.t(:product_has_no_description) : description
-    end
-
     def line_item_description_text(description_text)
       if description_text.present?
         truncate(strip_tags(description_text.gsub('&nbsp;', ' ').squish), length: 100)

core/app/models/spree/app_configuration.rb

@@ -35,8 +35,8 @@ class AppConfiguration < Preferences::Configuration
     preference :allow_guest_checkout, :boolean, default: true
     preference :alternative_shipping_phone, :boolean, default: false # Request extra phone for ship addr
     preference :always_include_confirm_step, :boolean, default: false # Ensures confirmation step is always in checkout_progress bar, but does not force a confirm step if your payment methods do not support it.
-    preference :always_put_site_name_in_title, :boolean, default: true
-    preference :title_site_name_separator, :string, default: '-' # When always_put_site_name_in_title is true, insert a separator character before the site name in the title
+    preference :always_put_site_name_in_title, :boolean, deprecated: true
+    preference :title_site_name_separator, :string, deprecated: true
     preference :auto_capture, :boolean, default: false # automatically capture the credit card (as opposed to just authorize and capture later)
     preference :auto_capture_on_dispatch, :boolean, default: false # Captures payment for each shipment in Shipment#after_ship callback, and makes Shipment.ready when payment authorized.
     preference :binary_inventory_cache, :boolean, default: false # only invalidate product cache when a stock item changes whether it is in_stock
@@ -48,31 +48,29 @@ class AppConfiguration < Preferences::Configuration
     preference :disable_store_presence_validation, :boolean, default: false # when turned off disables Store presence validation for Products and Payment Methods
     preference :expedited_exchanges, :boolean, default: false # NOTE this requires payment profiles to be supported on your gateway of choice as well as a delayed job handler to be configured with activejob. kicks off an exchange shipment upon return authorization save. charge customer if they do not return items within timely manner.
     preference :expedited_exchanges_days_window, :integer, default: 14 # the amount of days the customer has to return their item after the expedited exchange is shipped in order to avoid being charged
-    preference :layout, :string, default: 'spree/layouts/spree_application'
-    preference :logo, :string, default: 'logo/spree_50.png'
-    preference :mailer_logo, :string, default: 'logo/spree_50.png'
-    preference :max_level_in_taxons_menu, :integer, default: 1 # maximum nesting level in taxons menu
+    preference :layout, :string, deprecated: 'Please use Spree::Frontend::Config[:layout] instead'
+    preference :logo, :string, deprecated: true
+    preference :mailer_logo, :string, deprecated: true
+    preference :max_level_in_taxons_menu, :integer, deprecated: true
     preference :products_per_page, :integer, default: 12
-    preference :product_wysiwyg_editor_enabled, :boolean, default: true
     preference :require_master_price, :boolean, default: true
     preference :restock_inventory, :boolean, default: true # Determines if a return item is restocked automatically once it has been received
     preference :return_eligibility_number_of_days, :integer, default: 365
     preference :send_core_emails, :boolean, default: true # Default mail headers settings
-    preference :shipping_instructions, :boolean, default: false # Request instructions/info for shipping
+    preference :shipping_instructions, :boolean, deprecated: true
     preference :show_only_complete_orders_by_default, :boolean, default: true
     preference :show_variant_full_price, :boolean, default: false # Displays variant full price or difference with product price. Default false to be compatible with older behavior
     preference :show_products_without_price, :boolean, default: false
-    preference :show_raw_product_description, :boolean, default: false
+    preference :show_raw_product_description, :boolean, deprecated: true
     preference :tax_using_ship_address, :boolean, default: true
-    preference :taxon_wysiwyg_editor_enabled, :boolean, default: true
     preference :track_inventory_levels, :boolean, default: true # Determines whether to track on_hand values for variants / products.
 
     # Store credits configurations
     preference :non_expiring_credit_types, :array, default: []
     preference :credit_to_new_allocation, :boolean, default: false
 
     # Multi store configurations
-    preference :show_store_selector, :boolean, default: false
+    preference :show_store_selector, :boolean, deprecated: true
 
     # searcher_class allows spree extension writers to provide their own Search class
     def searcher_class

core/lib/spree/core/controller_helpers/common.rb

@@ -17,6 +17,9 @@ module Common
           attr_writer :title
 
           def title
+            ActiveSupport::Deprecation.warn(<<-DEPRECATION, caller)
+              ControllerHelpers::Common is deprecated and will be removed in Spree 5.0.
+            DEPRECATION
             title_string = @title.present? ? @title : accurate_title
             if title_string.present?
               if Spree::Config[:always_put_site_name_in_title] && !title_string.include?(default_title)
@@ -30,11 +33,17 @@ def title
           end
 
           def default_title
+            ActiveSupport::Deprecation.warn(<<-DEPRECATION, caller)
+              ControllerHelpers::Common is deprecated and will be removed in Spree 5.0.
+            DEPRECATION
             current_store.name
           end
 
           # this is a hook for subclasses to provide title
           def accurate_title
+            ActiveSupport::Deprecation.warn(<<-DEPRECATION, caller)
+              ControllerHelpers::Common is deprecated and will be removed in Spree 5.0.
+            DEPRECATION
             current_store.seo_title
           end
 
@@ -56,7 +65,12 @@ def set_user_language
           # Default layout is: +app/views/spree/layouts/spree_application+
           #
           def get_layout
-            layout ||= Spree::Config[:layout]
+            ActiveSupport::Deprecation.warn(<<-DEPRECATION, caller)
+              ControllerHelpers::Common is deprecated and will be removed in Spree 5.0.
+            DEPRECATION
+            return unless defined?(Spree::Frontend)
+
+            layout ||= Spree::Frontend::Config[:layout]
           end
         end
       end

core/lib/spree/core/engine.rb

@@ -131,7 +131,8 @@ class Engine < ::Rails::Engine
 
       initializer 'spree.core.checking_deprecated_preferences' do
         Spree::Config.deprecated_preferences.each do |pref|
-          warn "[DEPRECATION] Spree::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
+          # FIXME: we should only notify about deprecated preferences that are in use, not all of them
+          # warn "[DEPRECATION] Spree::Config[:#{pref[:name]}] is deprecated. #{pref[:message]}"
         end
       end
 

core/spec/helpers/products_helper_spec.rb

@@ -117,59 +117,6 @@ module Spree
       end
     end
 
-    context '#product_description' do
-      context 'when configuration is set to sanitize output' do
-        before { Spree::Config.product_wysiwyg_editor_enabled = false }
-
-        after { Spree::Config.product_wysiwyg_editor_enabled = true }
-
-        # Regression test for #1607
-        it 'renders a product description without excessive paragraph breaks' do
-          product.description = %Q{
-<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus a ligula leo. Proin eu arcu at ipsum dapibus ullamcorper. Pellentesque egestas orci nec magna condimentum luctus. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Ut ac ante et mauris bibendum ultricies non sed massa. Fusce facilisis dui eget lacus scelerisque eget aliquam urna ultricies. Duis et rhoncus quam. Praesent tellus nisi, ultrices sed iaculis quis, euismod interdum ipsum.</p>
-<ul>
-<li>Lorem ipsum dolor sit amet</li>
-<li>Lorem ipsum dolor sit amet</li>
-</ul>
-          }
-          description = product_description(product)
-          expect(description.strip).to eq(product.description.strip)
-        end
-
-        it 'renders a product description with automatic paragraph breaks' do
-          product.description = %Q{
-THIS IS THE BEST PRODUCT EVER!
-
-"IT CHANGED MY LIFE" - Sue, MD}
-
-          description = product_description(product)
-          expect(description.strip).to eq(%Q{<p>\nTHIS IS THE BEST PRODUCT EVER!</p>"IT CHANGED MY LIFE" - Sue, MD})
-        end
-
-        it 'renders a product description without any formatting based on configuration' do
-          initial_description = %Q{
-              <p>hello world</p>
-
-              <p>tihs is completely awesome and it works</p>
-
-              <p>why so many spaces in the code. and why some more formatting afterwards?</p>
-          }
-
-          product.description = initial_description
-
-          Spree::Config[:show_raw_product_description] = true
-          description = product_description(product)
-          expect(description).to eq(initial_description)
-        end
-
-        context 'renders a product description default description incase description is blank' do
-          before { product.description = '' }
-
-          it { expect(product_description(product)).to eq(Spree.t(:product_has_no_description)) }
-        end
-      end
-    end
-
     shared_examples_for 'line item descriptions' do
       context 'variant has a blank description' do
         let(:description) { nil }