Merge pull request #2244 from tf/custom-404

tf · web-flow · commit 7a2310b40dfb · 2025-06-24T12:51:03.000+02:00
Custom 404 pages
diff --git a/admins/pageflow/sites.rb b/admins/pageflow/sites.rb
@@ -30,7 +30,8 @@ module Pageflow
         :copyright_link_label,
         :privacy_link_url,
         :home_url,
-        :cutoff_mode_name
+        :cutoff_mode_name,
+        :custom_404_entry_id
       ] + permitted_admin_form_input_params
     end
 
diff --git a/app/controllers/concerns/pageflow/controller_delegation.rb b/app/controllers/concerns/pageflow/controller_delegation.rb
@@ -8,7 +8,7 @@ module ControllerDelegation
     # the response.
     def delegate_to_rack_app!(app)
       result = app.call(request.env)
-      yield(*result) if block_given?
+      result = yield(result) if block_given?
 
       throw(:delegate, result)
     end
diff --git a/app/controllers/pageflow/application_controller.rb b/app/controllers/pageflow/application_controller.rb
@@ -22,15 +22,9 @@ class ApplicationController < ActionController::Base
 
     rescue_from ActiveRecord::RecordNotFound do |exception|
       debug_log_with_backtrace(exception)
+
       respond_to do |format|
-        format.html do
-          begin
-            render file: Rails.public_path.join('pageflow', 'error_pages', '404.html'), status: :not_found
-          rescue ActionView::MissingTemplate => exception
-            debug_log_with_backtrace(exception)
-            head :not_found
-          end
-        end
+        format.html { render_static_404_error_page }
         format.any { head :not_found }
       end
     end
@@ -61,6 +55,14 @@ def locale_from_accept_language_header
       http_accept_language.compatible_language_from(I18n.available_locales)
     end
 
+    def render_static_404_error_page
+      render file: Rails.public_path.join('pageflow', 'error_pages', '404.html'),
+             status: :not_found
+    rescue ActionView::MissingTemplate => e
+      debug_log_with_backtrace(e)
+      head :not_found
+    end
+
     def debug_log_with_backtrace(exception)
       Rails.logger.debug exception
       backtrace = ''
diff --git a/app/controllers/pageflow/entries_controller.rb b/app/controllers/pageflow/entries_controller.rb
@@ -25,6 +25,8 @@ def show
           return unless check_entry_password_protection(entry)
 
           delegate_to_entry_type_frontend_app!(entry)
+        rescue ActiveRecord::RecordNotFound
+          render_custom_or_static_404_error_page
         end
       end
     end
@@ -100,15 +102,18 @@ def entry_redirect(entry)
       Pageflow.config.public_entry_redirect.call(entry, request)
     end
 
-    def delegate_to_entry_type_frontend_app!(entry)
+    def delegate_to_entry_type_frontend_app!(entry, override_status: nil)
       EntriesControllerEnvHelper.add_entry_info_to_env(request.env, entry: entry, mode: :published)
 
-      delegate_to_rack_app!(entry.entry_type.frontend_app) do |_status, headers, _body|
+      delegate_to_rack_app!(entry.entry_type.frontend_app) do |result|
+        status, headers, body = result
         config = Pageflow.config_for(entry)
 
         allow_iframe_for_embed(headers)
         apply_additional_headers(entry, config, headers)
         apply_cache_control(entry, config, headers)
+
+        [override_status || status, headers, body]
       end
     end
 
@@ -128,5 +133,17 @@ def apply_additional_headers(entry, config, headers)
         config.additional_public_entry_headers.for(entry, request)
       )
     end
+
+    def render_custom_or_static_404_error_page
+      site = Site.for_request(request).first
+
+      if site&.custom_404_entry&.published_without_password_protection?
+        entry = PublishedEntry.new(site.custom_404_entry)
+        delegate_to_entry_type_frontend_app!(entry, override_status: 404)
+      else
+        # Fallback to ApplicationController's handler method
+        render_static_404_error_page
+      end
+    end
   end
 end
diff --git a/app/models/concerns/pageflow/entry_publication_states.rb b/app/models/concerns/pageflow/entry_publication_states.rb
@@ -38,6 +38,10 @@ def published_with_password_protection?
       published? && published_revision.password_protected?
     end
 
+    def published_without_password_protection?
+      published? && !published_revision.password_protected?
+    end
+
     def published?
       published_revision.present?
     end
diff --git a/app/models/pageflow/site.rb b/app/models/pageflow/site.rb
@@ -1,6 +1,7 @@
 module Pageflow
   class Site < ApplicationRecord
     belongs_to :account
+    belongs_to :custom_404_entry, class_name: 'Entry', optional: true
 
     has_many :entry_templates, dependent: :destroy
     has_many :entries
@@ -11,6 +12,7 @@ class Site < ApplicationRecord
 
     validates :account, presence: true
     validates_inclusion_of :cutoff_mode_name, in: :available_cutoff_mode_names, allow_blank: true
+    validate :custom_404_entry_belongs_to_same_site
 
     delegate :enabled_feature_names, to: :account
 
@@ -74,5 +76,11 @@ def self.ransackable_associations(_auth_object = nil)
     def available_cutoff_mode_names
       Pageflow.config_for(account).cutoff_modes.names
     end
+
+    def custom_404_entry_belongs_to_same_site
+      return unless custom_404_entry.present?
+
+      errors.add(:custom_404_entry, :must_belong_to_same_site) if custom_404_entry.site_id != id
+    end
   end
 end
diff --git a/app/views/admin/sites/_fields.html.erb b/app/views/admin/sites/_fields.html.erb
@@ -23,6 +23,12 @@
   <%= f.input :sitemap_enabled, hint: t('pageflow.admin.sites.sitemap_hint',
                                         site_host: @site&.persisted? ? @site.host : '<host>') %>
 
+  <%= f.input :custom_404_entry_id,
+              as: :select,
+              collection: @site&.persisted? ? @site.entries.published_without_password_protection.pluck(:title, :id) : [],
+              include_blank: t('pageflow.admin.sites.custom_404_entry.none'),
+              hint: t('pageflow.admin.sites.custom_404_entry.hint') %>
+
   <% account_config.admin_form_inputs.find_all_for(:site).each do |form_input| %>
     <%= form_input.build(f) %>
   <% end %>
diff --git a/config/locales/new/custom_404_entry.de.yml b/config/locales/new/custom_404_entry.de.yml
@@ -0,0 +1,11 @@
+de:
+  pageflow:
+    admin:
+      sites:
+        custom_404_entry:
+          none: "(Standard 404-Seite verwenden)"
+          hint: "Wählen Sie einen Eintrag aus, der als benutzerdefinierte 404-Seite angezeigt werden soll. Nur veröffentlichte Einträge ohne Passwortschutz sind verfügbar."
+  activerecord:
+    attributes:
+      pageflow/site:
+        custom_404_entry_id: "Benutzerdefinierte 404-Seite"
diff --git a/config/locales/new/custom_404_entry.en.yml b/config/locales/new/custom_404_entry.en.yml
@@ -0,0 +1,11 @@
+en:
+  pageflow:
+    admin:
+      sites:
+        custom_404_entry:
+          none: "(Use default 404 page)"
+          hint: "Select an entry to display as the custom 404 page. Only published entries without password protection are available."
+  activerecord:
+    attributes:
+      pageflow/site:
+        custom_404_entry_id: "Custom 404 page"
diff --git a/db/migrate/20250617090048_add_custom404_entry_to_sites.rb b/db/migrate/20250617090048_add_custom404_entry_to_sites.rb
@@ -0,0 +1,5 @@
+class AddCustom404EntryToSites < ActiveRecord::Migration[7.1]
+  def change
+    add_reference :pageflow_sites, :custom_404_entry, null: true
+  end
+end
diff --git a/spec/controllers/admin/sites_controller_spec.rb b/spec/controllers/admin/sites_controller_spec.rb
@@ -216,6 +216,15 @@ def self.name
 
         expect(response.body).to have_select('Cutoff mode', options: ['(None)', 'Some Cutoff Mode'])
       end
+
+      it 'displays empty custom 404 entry select for new sites' do
+        account = create(:account)
+
+        sign_in(create(:user, :admin), scope: :user)
+        get(:new, params: {account_id: account})
+
+        expect(response.body).to have_select('Custom 404 page', options: ['(Use default 404 page)'])
+      end
     end
 
     describe '#edit' do
@@ -240,6 +249,28 @@ def self.name
 
         expect(response.body).to have_selector('[name="site[custom_field]"]')
       end
+
+      it 'displays custom 404 entry select for existing sites' do
+        site = create(:site)
+        entry = create(:entry, :published, site: site)
+
+        sign_in(create(:user, :admin), scope: :user)
+        get(:edit, params: {account_id: site.account, id: site})
+
+        expect(response.body).to have_select('Custom 404 page')
+      end
+
+      it 'displays only published entries without password protection in custom 404 entry select' do
+        site = create(:site)
+        published_entry = create(:entry, :published, site: site, title: 'Published Entry')
+        unpublished_entry = create(:entry, site: site, title: 'Unpublished Entry')
+        password_protected_entry = create(:entry, :published_with_password, site: site, password: 'secret', title: 'Protected Entry')
+
+        sign_in(create(:user, :admin), scope: :user)
+        get(:edit, params: {account_id: site.account, id: site})
+
+        expect(response.body).to have_select('Custom 404 page', options: ['(Use default 404 page)', 'Published Entry'])
+      end
     end
 
     describe '#create' do
@@ -342,6 +373,40 @@ def self.name
 
         expect(account.sites.last.custom_field).to eq(nil)
       end
+
+      it 'sets custom_404_entry_id when provided' do
+        site = create(:site)
+        entry = create(:entry, :published, site: site)
+
+        sign_in(create(:user, :admin), scope: :user)
+        patch(:update,
+              params: {
+                account_id: site.account,
+                id: site,
+                site: {
+                  custom_404_entry_id: entry.id
+                }
+              })
+
+        expect(site.reload.custom_404_entry_id).to eq(entry.id)
+      end
+
+      it 'allows nil custom_404_entry_id' do
+        account = create(:account)
+
+        sign_in(create(:user, :admin), scope: :user)
+        post(:create,
+             params: {
+               account_id: account,
+               site: {
+                 name: 'test site',
+                 custom_404_entry_id: ''
+               }
+             })
+
+        site = account.sites.last
+        expect(site.custom_404_entry_id).to be_nil
+      end
     end
 
     describe '#update' do
@@ -438,6 +503,41 @@ def self.name
 
         expect(site.reload.custom_field).to eq(nil)
       end
+
+      it 'updates custom_404_entry_id when provided' do
+        site = create(:site)
+        entry = create(:entry, :published, site: site)
+
+        sign_in(create(:user, :admin), scope: :user)
+        patch(:update,
+              params: {
+                account_id: site.account,
+                id: site,
+                site: {
+                  custom_404_entry_id: entry.id
+                }
+              })
+
+        expect(site.reload.custom_404_entry_id).to eq(entry.id)
+      end
+
+      it 'clears custom_404_entry_id when set to blank' do
+        site = create(:site)
+        entry = create(:entry, :published, site: site)
+        site.update!(custom_404_entry: entry)
+
+        sign_in(create(:user, :admin), scope: :user)
+        patch(:update,
+              params: {
+                account_id: site.account,
+                id: site,
+                site: {
+                  custom_404_entry_id: ''
+                }
+              })
+
+        expect(site.reload.custom_404_entry_id).to be_nil
+      end
     end
   end
 end
diff --git a/spec/models/concerns/pageflow/entry_publication_states_spec.rb b/spec/models/concerns/pageflow/entry_publication_states_spec.rb
@@ -249,5 +249,45 @@ module Pageflow
         end
       end
     end
+
+    describe '#published_with_password_protection?' do
+      it 'returns true for entry published with password protection' do
+        entry = create(:entry, :published_with_password)
+
+        expect(entry.published_with_password_protection?).to eq(true)
+      end
+
+      it 'returns false for entry published without password protection' do
+        entry = create(:entry, :published)
+
+        expect(entry.published_with_password_protection?).to eq(false)
+      end
+
+      it 'returns false for non-published entry' do
+        entry = create(:entry)
+
+        expect(entry.published_with_password_protection?).to eq(false)
+      end
+    end
+
+    describe '#published_without_password_protection?' do
+      it 'returns true for entry published without password protection' do
+        entry = create(:entry, :published)
+
+        expect(entry.published_without_password_protection?).to eq(true)
+      end
+
+      it 'returns false for entry published with password protection' do
+        entry = create(:entry, :published_with_password)
+
+        expect(entry.published_without_password_protection?).to eq(false)
+      end
+
+      it 'returns false for non-published entry' do
+        entry = create(:entry)
+
+        expect(entry.published_without_password_protection?).to eq(false)
+      end
+    end
   end
 end
diff --git a/spec/models/pageflow/site_spec.rb b/spec/models/pageflow/site_spec.rb
@@ -161,5 +161,31 @@ module Pageflow
         expect(result).to eq([matching_site])
       end
     end
+
+    describe 'custom_404_entry validation' do
+      it 'is valid when custom_404_entry belongs to same site' do
+        site = create(:site)
+        entry = create(:entry, site: site)
+        site.custom_404_entry = entry
+
+        expect(site).to be_valid
+      end
+
+      it 'is invalid when custom_404_entry belongs to different site' do
+        site1 = create(:site)
+        site2 = create(:site)
+        entry = create(:entry, site: site2)
+        site1.custom_404_entry = entry
+
+        site1.valid?
+        expect(site1.errors).to include(:custom_404_entry)
+      end
+
+      it 'is valid when custom_404_entry is nil' do
+        site = build(:site)
+
+        expect(site).to be_valid
+      end
+    end
   end
 end
diff --git a/spec/requests/pageflow/entries_show_request_spec.rb b/spec/requests/pageflow/entries_show_request_spec.rb
