Merge pull request #3093 from mamhoff/verify-module-controllers-at-ru…

…ntime Verify module controllers at runtime
AlchemyCMS · Nov 21, 2024 · c755127 · c755127
2 parents c8c85f9 + 6f3007f
commit c755127
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 63 deletions.
diff --git a/app/helpers/alchemy/admin/navigation_helper.rb b/app/helpers/alchemy/admin/navigation_helper.rb
@@ -11,6 +11,7 @@ module NavigationHelper
       #   The Hash representing a Alchemy module
       #
       def alchemy_main_navigation_entry(alchemy_module)
+        validate_controller_existence!(alchemy_module)
         render(
           "alchemy/admin/partials/main_navigation_entry",
           alchemy_module: alchemy_module,
@@ -141,6 +142,26 @@ def url_options_for_module(alchemy_module)
         url_options_for_navigation_entry(alchemy_module["navigation"] || {})
       end
 
+      # Validates the existence of a given controller configuration.
+      #
+      # @param String
+      #   The controller name
+      def validate_controller_existence!(definition_hash)
+        controllers = [definition_hash["navigation"]["controller"]]
+
+        if definition_hash["navigation"]["sub_navigation"].is_a?(Array)
+          controllers.concat(definition_hash["navigation"]["sub_navigation"].map { |x| x["controller"] })
+        end
+
+        controllers.each do |controller|
+          controller_const_name = "#{controller.camelize}Controller"
+          controller_const_name.constantize
+        rescue NameError
+          raise "Error in AlchemyCMS module definition: '#{definition_hash["name"]}'. Could not find the " \
+            "matching controller class #{controller_const_name} for the specified controller: '#{controller}'"
+        end
+      end
+
       # Returns a url options hash for given navigation entry.
       #
       # @param [Hash]

diff --git a/lib/alchemy/modules.rb b/lib/alchemy/modules.rb
@@ -27,35 +27,8 @@ def included(base)
       def register_module(module_definition)
         definition_hash = module_definition.deep_stringify_keys
 
-        ### Validate controller(s) existence
-        if definition_hash["navigation"].is_a?(Hash)
-          defined_controllers = [definition_hash["navigation"]["controller"]]
-
-          if definition_hash["navigation"]["sub_navigation"].is_a?(Array)
-            defined_controllers.concat(definition_hash["navigation"]["sub_navigation"].map { |x| x["controller"] })
-          end
-
-          validate_controllers_existence(defined_controllers, definition_hash)
-        end
-
         @@alchemy_modules |= [definition_hash]
       end
-
-      private
-
-      def validate_controllers_existence(controllers, definition_hash)
-        controllers.each do |controller_val|
-          next if controller_val.blank?
-
-          controller_name = "#{controller_val.camelize}Controller"
-
-          begin
-            controller_name.constantize
-          rescue NameError
-            raise "Error in AlchemyCMS module definition: '#{definition_hash["name"]}'. Could not find the matching controller class #{controller_name.sub(/^::/, "")} for the specified controller: '#{controller_val}'"
-          end
-        end
-      end
     end
 
     # Get the module definition for given module name

diff --git a/spec/helpers/alchemy/admin/navigation_helper_spec.rb b/spec/helpers/alchemy/admin/navigation_helper_spec.rb
@@ -104,6 +104,46 @@
           expect(helper.alchemy_main_navigation_entry(alchemy_module))
             .to have_selector ".main_navi_entry .sub_navigation"
         end
+
+        context "with a bad controller name" do
+          let(:alchemy_module) do
+            {
+              "name" => "bad_module_name",
+              "navigation" => {
+                "controller" => "admin/events",
+                "action" => "index",
+                "sub_navigation" => [{
+                  "controller" => "bad",
+                  "action" => "index"
+                }]
+              }
+            }
+          end
+
+          it "raises an understandable error" do
+            expect { helper.alchemy_main_navigation_entry(alchemy_module) }.to raise_error(
+              "Error in AlchemyCMS module definition: 'bad_module_name'. Could not find the matching controller class BadController for the specified controller: 'bad'"
+            )
+          end
+        end
+      end
+
+      context "with a bad controller name" do
+        let(:alchemy_module) do
+          {
+            "name" => "bad_module_name",
+            "navigation" => {
+              "controller" => "bad",
+              "action" => "index"
+            }
+          }
+        end
+
+        it "raises an understandable error" do
+          expect { helper.alchemy_main_navigation_entry(alchemy_module) }.to raise_error(
+            "Error in AlchemyCMS module definition: 'bad_module_name'. Could not find the matching controller class BadController for the specified controller: 'bad'"
+          )
+        end
       end
     end
 

diff --git a/spec/libraries/modules_spec.rb b/spec/libraries/modules_spec.rb
@@ -132,47 +132,11 @@ class ModulesTestController < ApplicationController
         }
       end
 
-      let(:bad_alchemy_module_a) do
-        {
-          "name" => "bad_module_a",
-          "navigation" => {
-            "controller" => "bad_module",
-            "action" => "index"
-          }
-        }
-      end
-
-      let(:bad_alchemy_module_b) do
-        {
-          "name" => "bad_module_b",
-          "navigation" => {
-            "controller" => "register_module_dummy",
-            "action" => "index",
-            "sub_navigation" => [{
-              "controller" => "bad_module",
-              "action" => "index"
-            }]
-          }
-        }
-      end
-
       it "registers a module definition into global list of modules" do
         Modules.register_module(alchemy_module)
         expect(Modules.alchemy_modules).to include(alchemy_module)
       end
 
-      it "fails to register a module when a matching navigation controller cannot be found" do
-        expect { Modules.register_module(bad_alchemy_module_a) }.to raise_error(
-          "Error in AlchemyCMS module definition: 'bad_module_a'. Could not find the matching controller class BadModuleController for the specified controller: 'bad_module'"
-        )
-      end
-
-      it "fails to register a module when a matching sub_navigation controller cannot be found" do
-        expect { Modules.register_module(bad_alchemy_module_b) }.to raise_error(
-          "Error in AlchemyCMS module definition: 'bad_module_b'. Could not find the matching controller class BadModuleController for the specified controller: 'bad_module'"
-        )
-      end
-
       it "registers a module definition only once" do
         2.times do
           Modules.register_module(alchemy_module)