Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look at if extending modules is still preferable to inheriting from classes #221

Open
jensljungblad opened this issue Feb 21, 2017 · 3 comments

Comments

@jensljungblad
Copy link
Collaborator

jensljungblad commented Feb 21, 2017

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.

@jensljungblad jensljungblad created this issue from a note in Godmin 2.0 (Backlog) Feb 21, 2017
@jensljungblad
Copy link
Collaborator Author

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.

@jensljungblad
Copy link
Collaborator Author

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.

@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Nov 22, 2017

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Godmin 2.0
Discussions
Development

No branches or pull requests

1 participant