From 69c499360989327f65c4473894a20bd1e9ea4a0c Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 7 May 2024 16:48:04 +0200 Subject: [PATCH 1/3] Helper #render_site_layout: Raise in dev envs If we miss a partial, our specs should tell us. --- app/helpers/alchemy/pages_helper.rb | 10 +++++++--- spec/helpers/alchemy/pages_helper_spec.rb | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/app/helpers/alchemy/pages_helper.rb b/app/helpers/alchemy/pages_helper.rb index 73f9c26aea..ce0c7256b8 100644 --- a/app/helpers/alchemy/pages_helper.rb +++ b/app/helpers/alchemy/pages_helper.rb @@ -64,9 +64,13 @@ def render_page_layout # def render_site_layout(&block) render current_alchemy_site, &block - rescue ActionView::MissingTemplate - warning("Site layout for #{current_alchemy_site.try(:name)} not found. Please run `rails g alchemy:site_layouts`") - "" + rescue ActionView::MissingTemplate => error + if Rails.application.config.consider_all_requests_local? + raise error + else + warning("Site layout for #{current_alchemy_site.try(:name)} not found. Please run `rails g alchemy:site_layouts`") + "" + end end # Renders a menu partial diff --git a/spec/helpers/alchemy/pages_helper_spec.rb b/spec/helpers/alchemy/pages_helper_spec.rb index 82d0613ea1..84450090e8 100644 --- a/spec/helpers/alchemy/pages_helper_spec.rb +++ b/spec/helpers/alchemy/pages_helper_spec.rb @@ -41,9 +41,22 @@ module Alchemy end context "with missing partial" do - it "returns empty string and logges warning" do - expect(helper).to receive(:current_alchemy_site).twice.and_return(default_site) - expect(helper.render_site_layout).to eq("") + context "in production environment" do + before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(false) } + + it "returns empty string and logges warning" do + expect(helper).to receive(:current_alchemy_site).twice.and_return(default_site) + expect(helper.render_site_layout).to eq("") + end + end + + context "in dev or test environment" do + before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(true) } + + it "raises missing template error" do + expect(helper).to receive(:current_alchemy_site).and_return(default_site) + expect { helper.render_site_layout }.to raise_error(ActionView::MissingTemplate) + end end end end From 6e874f9e667d8d138d7ab791556bf66d86815899 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 7 May 2024 16:48:46 +0200 Subject: [PATCH 2/3] Helper #render_menu: Raise on missing partial in dev When we miss a partial, we want to know (at least in the development and test environments). --- app/helpers/alchemy/pages_helper.rb | 14 +++++++++----- spec/helpers/alchemy/pages_helper_spec.rb | 12 +++++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/helpers/alchemy/pages_helper.rb b/app/helpers/alchemy/pages_helper.rb index ce0c7256b8..8888330668 100644 --- a/app/helpers/alchemy/pages_helper.rb +++ b/app/helpers/alchemy/pages_helper.rb @@ -91,11 +91,15 @@ def render_menu(menu_type, options = {}) end render("alchemy/menus/#{menu_type}/wrapper", menu: root_node, options: options) - rescue ActionView::MissingTemplate => e - warning <<~WARN - Menu partial not found for #{menu_type}. - #{e} - WARN + rescue ActionView::MissingTemplate => error + if Rails.application.config.consider_all_requests_local? + raise error + else + warning <<~WARN + Menu partial not found for #{menu_type}. + #{error} + WARN + end end # Returns page links in a breadcrumb beginning from root to current page. diff --git a/spec/helpers/alchemy/pages_helper_spec.rb b/spec/helpers/alchemy/pages_helper_spec.rb index 84450090e8..ece3f0e106 100644 --- a/spec/helpers/alchemy/pages_helper_spec.rb +++ b/spec/helpers/alchemy/pages_helper_spec.rb @@ -79,7 +79,17 @@ module Alchemy context "but the template does not exist" do let(:menu_type) { "unknown" } - it { is_expected.to be_nil } + context "in production environment" do + before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(false) } + + it { is_expected.to be_nil } + end + + context "in dev or test environment" do + before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(true) } + + it { expect { subject }.to raise_error(ActionView::MissingTemplate) } + end end end From 48be9420701419f48a0be35b1ac6584dce03db07 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 8 May 2024 09:52:52 +0200 Subject: [PATCH 3/3] Refactor missing template error handling This extracts the error handling for missing templates into a helper method. --- app/helpers/alchemy/pages_helper.rb | 27 +++++++++++------------ spec/helpers/alchemy/pages_helper_spec.rb | 4 ++-- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/app/helpers/alchemy/pages_helper.rb b/app/helpers/alchemy/pages_helper.rb index 8888330668..733a637e85 100644 --- a/app/helpers/alchemy/pages_helper.rb +++ b/app/helpers/alchemy/pages_helper.rb @@ -65,12 +65,7 @@ def render_page_layout def render_site_layout(&block) render current_alchemy_site, &block rescue ActionView::MissingTemplate => error - if Rails.application.config.consider_all_requests_local? - raise error - else - warning("Site layout for #{current_alchemy_site.try(:name)} not found. Please run `rails g alchemy:site_layouts`") - "" - end + error_or_warning(error, "Site layout for #{current_alchemy_site.try(:name)} not found. Please run `rails g alchemy:site_layouts`") end # Renders a menu partial @@ -92,14 +87,7 @@ def render_menu(menu_type, options = {}) render("alchemy/menus/#{menu_type}/wrapper", menu: root_node, options: options) rescue ActionView::MissingTemplate => error - if Rails.application.config.consider_all_requests_local? - raise error - else - warning <<~WARN - Menu partial not found for #{menu_type}. - #{error} - WARN - end + error_or_warning(error, "Menu partial for #{menu_type} not found. Please run `rails g alchemy:menus`") end # Returns page links in a breadcrumb beginning from root to current page. @@ -177,5 +165,16 @@ def meta_keywords def meta_robots "#{@page.robot_index? ? "" : "no"}index, #{@page.robot_follow? ? "" : "no"}follow" end + + private + + def error_or_warning(error, message) + if Rails.application.config.consider_all_requests_local? + raise error, message + else + Rails.logger.error message + "" + end + end end end diff --git a/spec/helpers/alchemy/pages_helper_spec.rb b/spec/helpers/alchemy/pages_helper_spec.rb index ece3f0e106..2ad20e332c 100644 --- a/spec/helpers/alchemy/pages_helper_spec.rb +++ b/spec/helpers/alchemy/pages_helper_spec.rb @@ -54,7 +54,7 @@ module Alchemy before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(true) } it "raises missing template error" do - expect(helper).to receive(:current_alchemy_site).and_return(default_site) + expect(helper).to receive(:current_alchemy_site).twice.and_return(default_site) expect { helper.render_site_layout }.to raise_error(ActionView::MissingTemplate) end end @@ -82,7 +82,7 @@ module Alchemy context "in production environment" do before { allow(Rails.application.config).to receive(:consider_all_requests_local?).and_return(false) } - it { is_expected.to be_nil } + it { is_expected.to eq("") } end context "in dev or test environment" do