Remove redirect_to_public_child flag and feature (#1910)

* Remove redirect_to_public_child flag and feature

This has been deprecated in 5.0 and is not supported anymore.

* Include locale redirect modules in pages controller

Now that we do not have to take care of the public child redirect anymore we can move the rather simple logic into the controller again.

app/controllers/alchemy/pages_controller.rb

@@ -13,19 +13,24 @@ class PagesController < Alchemy::BaseController
 
     # Redirecting concerns. Order is important here!
     include SiteRedirects
-    include LocaleRedirects
+
+    before_action :enforce_no_locale,
+      if: :locale_prefix_not_allowed?,
+      only: [:index, :show]
 
     before_action :load_index_page, only: [:index]
     before_action :load_page, only: [:show]
 
     # Legacy page redirects need to run after the page was loaded and before we render 404.
     include LegacyPageRedirects
 
-    # From here on, we need a +@page+ to work with!
-    before_action :page_not_found!, if: -> { @page.blank? }, only: [:index, :show]
+    # From here on, we need a published +@page+ to work with!
+    before_action :page_not_found!, unless: -> { @page&.public? }, only: [:index, :show]
 
-    # Page redirects need to run after the page was loaded and we're sure to have a +@page+ set.
-    include PageRedirects
+    # Page redirects need to run after the page was loaded and we're sure to have a public +@page+ set.
+    before_action :enforce_locale,
+      if: :locale_prefix_missing?,
+      only: [:index, :show]
 
     # We only need to set the +@root_page+ if we are sure that no more redirects happen.
     before_action :set_root_page, only: [:index, :show]
@@ -66,12 +71,8 @@ def index
     # descendant it finds. If no public page can be found it renders a 404 error.
     #
     def show
-      if redirect_url.present?
-        redirect_permanently_to redirect_url
-      else
-        authorize! :show, @page
-        render_page if render_fresh_page?
-      end
+      authorize! :show, @page
+      render_page if render_fresh_page?
     end
 
     # Renders a search engine compatible xml sitemap.
@@ -84,13 +85,25 @@ def sitemap
 
     private
 
+    # Redirects to requested action without locale prefixed
+    def enforce_no_locale
+      redirect_permanently_to additional_params.merge(locale: nil)
+    end
+
+    # Is the requested locale allowed?
+    #
+    # If Alchemy is not in multi language mode or the requested locale is the default locale,
+    # then we want to redirect to a non prefixed url.
+    #
+    def locale_prefix_not_allowed?
+      params[:locale].present? && !multi_language? ||
+        params[:locale].presence == ::I18n.default_locale.to_s
+    end
+
     # == Loads index page
     #
     # Loads the current public language root page.
     #
-    # If the root page is not public it redirects to the first published child.
-    # This can be configured via +redirect_to_public_child+ [default: true]
-    #
     # If no index page and no admin users are present we show the "Welcome to Alchemy" page.
     #
     def load_index_page
@@ -116,6 +129,28 @@ def load_page
       )
     end
 
+    def enforce_locale
+      redirect_permanently_to page_locale_redirect_url(locale: Language.current.code)
+    end
+
+    def locale_prefix_missing?
+      multi_language? && params[:locale].blank? && !default_locale?
+    end
+
+    def default_locale?
+      Language.current.code.to_sym == ::I18n.default_locale.to_sym
+    end
+
+    # Page url with or without locale while keeping all additional params
+    def page_locale_redirect_url(options = {})
+      options = {
+        locale: prefix_locale? ? @page.language_code : nil,
+        urlname: @page.urlname,
+      }.merge(options)
+
+      alchemy.show_page_path additional_params.merge(options)
+    end
+
     # Redirects to given url with 301 status
     def redirect_permanently_to(url)
       redirect_to url, status: :moved_permanently

config/alchemy/config.yml

@@ -9,12 +9,6 @@
 #
 auto_logout_time: 30
 
-# === Redirect Options
-#
-#   redirect_to_public_child  [Boolean]   # Alchemy redirects to the first public child page found, if a page is not public.
-#
-redirect_to_public_child: true
-
 # === Page caching
 #
 # Enable/Disable page caching globally.

lib/alchemy/config.rb

@@ -31,9 +31,7 @@ def show
       # a value of nil means there is no new default
       # any not nil value is the new default
       def deprecated_configs
-        {
-          redirect_to_public_child: nil,
-        }
+        {}
       end
 
       private

spec/controllers/alchemy/pages_controller_spec.rb

@@ -62,76 +62,24 @@ module Alchemy
               default_language_root.update!(public_on: nil)
             end
 
-            context "and redirect_to_public_child is set to false" do
-              before do
-                stub_alchemy_config(:redirect_to_public_child, false)
-              end
-
-              it "raises routing error (404)" do
-                expect {
-                  get :index
-                }.to raise_error(ActionController::RoutingError)
-              end
-
-              context "when a page layout callback is set" do
-                before do
-                  ApplicationController.extend Alchemy::OnPageLayout
-                  ApplicationController.class_eval do
-                    on_page_layout("index") { "do something" }
-                  end
-                end
-
-                it 'raises routing error (404) and no "undefined method for nil" error' do
-                  expect {
-                    get :index
-                  }.to raise_error(ActionController::RoutingError)
-                end
-              end
+            it "raises routing error (404)" do
+              expect {
+                get :index
+              }.to raise_error(ActionController::RoutingError)
             end
 
-            context "and redirect_to_public_child is set to true" do
+            context "when a page layout callback is set" do
               before do
-                stub_alchemy_config(:redirect_to_public_child, true)
-              end
-
-              context "that has a public child" do
-                let!(:public_child) do
-                  create(:alchemy_page, :public, parent: default_language_root)
-                end
-
-                it "loads this page" do
-                  get :index
-                  expect(assigns(:page)).to eq(public_child)
+                ApplicationController.extend Alchemy::OnPageLayout
+                ApplicationController.class_eval do
+                  on_page_layout("index") { "do something" }
                 end
               end
 
-              context "that has a non public child" do
-                let!(:non_public_child) do
-                  create(:alchemy_page, parent: default_language_root)
-                end
-
-                context "that has a public child" do
-                  let!(:public_child) do
-                    create(:alchemy_page, :public, parent: non_public_child)
-                  end
-
-                  it "loads this page" do
-                    get :index
-                    expect(assigns(:page)).to eq(public_child)
-                  end
-                end
-
-                context "that has a non public child" do
-                  before do
-                    create(:alchemy_page, parent: non_public_child)
-                  end
-
-                  it "raises routing error (404)" do
-                    expect {
-                      get :index
-                    }.to raise_error(ActionController::RoutingError)
-                  end
-                end
+              it 'raises routing error (404) and no "undefined method for nil" error' do
+                expect {
+                  get :index
+                }.to raise_error(ActionController::RoutingError)
               end
             end
           end