Deprecate url_nesting configuration (4.6)

.rubocop.yml

@@ -9,17 +9,12 @@ AllCops:
     - 'alchemy_cms.gemspec'
     - 'Rakefile'
 
-  TargetRubyVersion: 2.3
+  TargetRubyVersion: 2.4
 
 # Really, rubocop?
 Bundler/OrderedGems:
   Enabled: false
 
-# Sometimes I believe this reads better
-# This also causes spacing issues on multi-line fixes
-Style/BracesAroundHashParameters:
-  Enabled: false
-
 Style/EmptyLiteral:
   Enabled: false
 
@@ -55,13 +50,10 @@ Style/MixinUsage:
   Exclude:
     - spec/**/*
 
-Performance/Count:
-  Enabled: false
-
-Layout/AlignHash:
+Layout/HashAlignment:
   Enabled: false
 
-Layout/AlignParameters:
+Layout/ParameterAlignment:
   Enabled: false
 
 Layout/ClosingParenthesisIndentation:
@@ -77,13 +69,13 @@ Layout/ElseAlignment:
 Layout/EmptyLineAfterMagicComment:
   Enabled: false
 
-Layout/IndentArray:
+Layout/FirstArrayElementIndentation:
   Enabled: false
 
-Layout/IndentHash:
+Layout/FirstHashElementIndentation:
   Enabled: false
 
-Layout/IndentHeredoc:
+Layout/HeredocIndentation:
   EnforcedStyle: active_support
 
 Layout/IndentationWidth:
@@ -115,7 +107,7 @@ Layout/SpaceInsideParens:
 Layout/EndAlignment:
   Enabled: false
 
-Lint/HandleExceptions:
+Lint/SuppressedException:
   Exclude:
     - 'config/initializers/mini_profiler.rb'
 

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,26 @@ 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

lib/tasks/alchemy/convert.rake

@@ -3,6 +3,7 @@ namespace :alchemy do
     namespace :urlnames do
       desc "Converts the urlname of all pages to nested url paths."
       task to_nested: [:environment] do
+        Alchemy::Deprecation.warn('alchemy:convert:urlnames:to_nested task is deprecated and will be removed from Alchemy 5.0')
         unless Alchemy::Config.get(:url_nesting)
           raise "\nURL nesting is disabled! Please enable url_nesting in `config/alchemy/config.yml` first.\n\n"
         end
@@ -18,6 +19,7 @@ namespace :alchemy do
 
       desc "Converts the urlname of all pages to contain the slug only."
       task to_slug: [:environment] do
+        Alchemy::Deprecation.warn('alchemy:convert:urlnames:to_slug task is deprecated and will be removed from Alchemy 5.0')
         if Alchemy::Config.get(:url_nesting)
           raise "\nURL nesting is enabled! Please disable url_nesting in `config/alchemy/config.yml` first.\n\n"
         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,67 @@ 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