godmin
godmin copied to clipboard
Look at if extending modules is still preferable to inheriting from classes
Take a good hard look at if extending modules is still preferable to inheriting from classes. The file resolver has been the cause of much pain.
The reason we have it is because we want to support template and partial overrides. A template placed in views/articles/index.html.erb
works as expected. A partial placed in views/articles/_table.html.erb
overrides the table for articles, while views/resources/_table.html.erb
overrides the table for all resources. This would work out of the box if we inherited instead of included modules, like so:
class ArticlesController < Godmin::ResourcesController; end
The reason we do use modules instead is because of problems with what the above base controller would inherit from. Consider this, which is what gems like Devise and Clearance do:
class Godmin::ResourcesController < ::ApplicationController;
This means all resource controllers would inherit from the main app's application controller, which does not work for engine installs. We basically have the requirement that our gem needs to work in isolation within engines. Consider the following scenario:
admin_one
controllers
admin_one
application_controller.rb
articles_controller.rb
admin_two
controllers
admin_two
application_controller.rb
articles_controller.rb
app
controllers
application_controller.rb
articles_controller.rb
AdminOne::ArticlesController
needs to inherit from AdminOne::ApplicationController
, AdminTwo::ArticlesController
needs to inherit from AdminTwo::ApplicationController
and
::ArticlesController
needs to inherit from ::ApplicationController
.
And for this reason, we went with modules, and that way, we do not get template overriding out of the box, which meant we had to implement our own resolver.
The question is, is there a better way to solve this? Do we need the ability to override by placing files in /resources
at all? Administrate solves this differently by using a generator: https://administrate-prototype.herokuapp.com/customizing_page_views. Also take into consideration that we're thinking about different ways to do column overrides.
If there would always be a ResourceController
, generated by the installer, would that solve basic overriding? We tend to always create such a class anyway because we want shared behaviour for all resource controllers.
I experimented a bit with this and tried the following:
module Godmin
module Controllers
def self.resources_controller(parent)
resources_controller = Class.new(parent) do
def index; end
end
Godmin.const_set("ResourcesController", resources_controller)
end
end
end
class ResourcesController < Godmin::Controllers.resources_controller(ApplicationController)
end
require_dependency "admin/application_controller"
module Admin
class ResourcesController < Godmin::Controllers.resources_controller(ApplicationController)
end
end
This works for template overriding without the need of a custom resolver. Godmin templates are found by apps and engines, and app templates override the Godmin templates.
As for partial overriding, it's trickier. If we render partials from the same folder, e.g. godmin/resources/index.html.erb
wants to render godmin/resources/_table.html.erb
, it works:
<%= render "table" %>
However, if we need to render something from another folder, it does not, because Rails goes into absolute path mode:
<%= render "foo/table" %>
This is part of what we solve with the custom resolver today, and it might be simpler to solve with the inheritance solution above. However, perhaps we could just place partials in the same folder and try and come up with a different solution for column and filter overrides, which are the real problems here.
Another alternative would be to skip the resources_controller
entirely and move all of that functionality to the application_controller
. That would cater more towards the 80% use case and not be as much "just a rails app" as what we have now. It would solve the inheritance problem, however:
ApplicationController < Godmin::ApplicationController < ActionController::Base
We do have an additional sessions_controller
now though, which would be a little tricky...