Deprecate url_nesting configuration

config/alchemy/config.yml

@@ -44,19 +44,6 @@ sitemap:
   show_root: true
   show_flag: false
 
-# === URL nesting
-#
-# Since Alchemy 2.6.0, page urls are nested, respectively to their tree position.
-#
-# Disable +url_nesting+ to get slug only urls.
-#
-# NOTE: After changing the url_nesting, you should run one of these convert rake tasks:
-#
-#   rake alchemy:convert:urlnames:to_nested
-#   rake alchemy:convert:urlnames:to_slug
-#
-url_nesting: true
-
 # === Default items per page in admin views
 #
 # In Alchemy's Admin, change how many items you would get shown per page by Kaminari

lib/alchemy/config.rb

@@ -8,8 +8,10 @@ class << self
       # @param name [String]
       #
       def get(name)
+        check_deprecation(name)
         show[name.to_s]
       end
+
       alias_method :parameter, :get
 
       # Returns a merged configuration of the following files
@@ -25,11 +27,20 @@ def show
         @config ||= merge_configs!(alchemy_config, main_app_config, env_specific_config)
       end
 
+      # A list of deprecated configurations
+      # a value of nil means there is no new default
+      # any not nil value is the new default
+      def deprecated_configs
+        {
+          url_nesting: true,
+        }
+      end
+
       private
 
       # Alchemy default configuration
       def alchemy_config
-        read_file(File.join(File.dirname(__FILE__), '..', '..', 'config/alchemy/config.yml'))
+        read_file(File.join(File.dirname(__FILE__), "..", "..", "config/alchemy/config.yml"))
       end
 
       # Application specific configuration
@@ -54,11 +65,25 @@ def read_file(file)
       # Merges all given configs together
       #
       def merge_configs!(*config_files)
-        raise LoadError, 'No Alchemy config file found!' if config_files.map(&:blank?).all?
+        raise LoadError, "No Alchemy config file found!" if config_files.map(&:blank?).all?
         config = {}
         config_files.each { |h| config.merge!(h.stringify_keys!) }
         config
       end
+
+      def check_deprecation(name)
+        if deprecated_configs.key?(name.to_sym)
+          config = deprecated_configs[name.to_sym]
+          if config.nil?
+            Alchemy::Deprecation.warn("#{name} configuration is deprecated and will be removed from Alchemy 5.0")
+          else
+            value = show[name.to_s]
+            if value != config
+              Alchemy::Deprecation.warn("Setting #{name} configuration to #{value} is deprecated and will be always #{config} in Alchemy 5.0")
+            end
+          end
+        end
+      end
     end
   end
 end

spec/libraries/config_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require 'rails_helper'
+require "rails_helper"
 
 module Alchemy
   describe Config do
@@ -11,26 +11,57 @@ module Alchemy
       end
 
       it "should return the requested part of the config" do
-        expect(Config).to receive(:show).and_return({'mailer' => {'setting' => 'true'}})
-        expect(Config.get(:mailer)).to eq({'setting' => 'true'})
+        expect(Config).to receive(:show).and_return({ "mailer" => { "setting" => "true" } })
+        expect(Config.get(:mailer)).to eq({ "setting" => "true" })
+      end
+
+      context "if config is deprecated" do
+        context "without a new default" do
+          before do
+            expect(described_class).to receive(:deprecated_configs).at_least(:once) do
+              { url_nesting: nil }
+            end
+          end
+
+          it "warns" do
+            expect(Alchemy::Deprecation).to receive(:warn)
+            Config.get(:url_nesting)
+          end
+        end
+
+        context "with a new default" do
+          context "and current value is not the default" do
+            before do
+              expect(described_class).to receive(:show).at_least(:once) do
+                { "url_nesting" => false }
+              end
+            end
+
+            it "warns about new default" do
+              expect(Alchemy::Deprecation).to \
+                receive(:warn).with("Setting url_nesting configuration to false is deprecated and will be always true in Alchemy 5.0")
+              Config.get(:url_nesting)
+            end
+          end
+        end
       end
     end
 
-    describe '.main_app_config' do
+    describe ".main_app_config" do
       let(:main_app_config_path) { "#{Rails.root}/config/alchemy/config.yml" }
 
       it "should call and return .read_file with the correct config path" do
-        expect(Config).to receive(:read_file).with(main_app_config_path).once.and_return({setting: 'true'})
-        expect(Config.send(:main_app_config)).to eq({setting: 'true'})
+        expect(Config).to receive(:read_file).with(main_app_config_path).once.and_return({ setting: "true" })
+        expect(Config.send(:main_app_config)).to eq({ setting: "true" })
       end
     end
 
-    describe '.env_specific_config' do
+    describe ".env_specific_config" do
       let(:env_specific_config_path) { "#{Rails.root}/config/alchemy/#{Rails.env}.config.yml" }
 
       it "should call and return .read_file with the correct config path" do
-        expect(Config).to receive(:read_file).with(env_specific_config_path).once.and_return({setting: 'true'})
-        expect(Config.send(:env_specific_config)).to eq({setting: 'true'})
+        expect(Config).to receive(:read_file).with(env_specific_config_path).once.and_return({ setting: "true" })
+        expect(Config.send(:env_specific_config)).to eq({ setting: "true" })
       end
     end
 
@@ -39,65 +70,65 @@ module Alchemy
         before { Config.instance_variable_set("@config", nil) }
 
         it "should call and return .merge_configs!" do
-          expect(Config).to receive(:merge_configs!).once.and_return({setting: 'true'})
-          expect(Config.show).to eq({setting: 'true'})
+          expect(Config).to receive(:merge_configs!).once.and_return({ setting: "true" })
+          expect(Config.show).to eq({ setting: "true" })
         end
       end
 
       context "when ivar @config was already set" do
-        before { Config.instance_variable_set("@config", {setting: 'true'}) }
+        before { Config.instance_variable_set("@config", { setting: "true" }) }
         after { Config.instance_variable_set("@config", nil) }
 
         it "should have memoized the return value of .merge_configs!" do
-          expect(Config.send(:show)).to eq({setting: 'true'})
+          expect(Config.send(:show)).to eq({ setting: "true" })
         end
       end
     end
 
-    describe '.read_file' do
-      context 'when given path to yml file exists' do
-        context 'and file is empty' do
+    describe ".read_file" do
+      context "when given path to yml file exists" do
+        context "and file is empty" do
           before do
             # YAML.safe_load returns nil if file is empty.
             allow(YAML).to receive(:safe_load) { nil }
           end
 
           it "should return an empty Hash" do
-            expect(Config.send(:read_file, 'empty_file.yml')).to eq({})
+            expect(Config.send(:read_file, "empty_file.yml")).to eq({})
           end
         end
       end
 
-      context 'when given path to yml file does not exist' do
-        it 'should return an empty Hash' do
-          expect(Config.send(:read_file, 'does/not/exist.yml')).to eq({})
+      context "when given path to yml file does not exist" do
+        it "should return an empty Hash" do
+          expect(Config.send(:read_file, "does/not/exist.yml")).to eq({})
         end
       end
     end
 
-    describe '.merge_configs!' do
+    describe ".merge_configs!" do
       let(:config_1) do
-        {setting_1: 'same', other_setting: 'something'}
+        { setting_1: "same", other_setting: "something" }
       end
 
       let(:config_2) do
-        {setting_1: 'same', setting_2: 'anything'}
+        { setting_1: "same", setting_2: "anything" }
       end
 
       it "should stringify the keys" do
         expect(Config.send(:merge_configs!, config_1)).to eq(config_1.stringify_keys!)
       end
 
-      context 'when all passed configs are empty' do
+      context "when all passed configs are empty" do
         it "should raise an error" do
           expect { Config.send(:merge_configs!, {}) }.to raise_error(LoadError)
         end
       end
 
-      context 'when configs containing same keys' do
+      context "when configs containing same keys" do
         it "should merge them together" do
           expect(Config.send(:merge_configs!, config_1, config_2)).to eq(
-            {'setting_1' => 'same', 'other_setting' => 'something', 'setting_2' => 'anything'}
+            { "setting_1" => "same", "other_setting" => "something", "setting_2" => "anything" }
           )
         end
       end