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

Improve support for custom controller #288

Open
adzap opened this issue Aug 7, 2019 · 0 comments
Open

Improve support for custom controller #288

adzap opened this issue Aug 7, 2019 · 0 comments

Comments

@adzap
Copy link

adzap commented Aug 7, 2019

At the moment if you want to use a custom controller but otherwise use the HighVoltage internal mechanics, you have to turn off routes and unset the home_page due to the way the routes files is coded:

HighVoltage.parent_engine.routes.draw do
  if HighVoltage.home_page
    get "/#{HighVoltage.home_page}", to: redirect('/')
    root to: 'high_voltage/pages#show', id: HighVoltage.home_page
  end

  if HighVoltage.routes
    get HighVoltage.route_drawer.match_attributes
  end
end

We could nest the home page routes under the if HighVoltage.routes logic, which is more what I would expect if I disabled the gem routes. I think that is a standalone improvement.

But I would also like to suggest that the home page routes and could be modified to support just using a custom routes drawer to enable full custom controller support. One way is to add a new config option controller_name or rack_endpoint with a default of "high_voltage/pages" which could be inserted in the routes i.e.

root to: "#{HighVoltage.controller_name}#show", id: HighVoltage.home_page

and a similar change in the route drawers.

Alternatively we could modify the current route drawers to use a different get options method signature
i.e. in routes drawer

def self.match_attributes
  [
    "/#{HighVoltage.content_path}*id",
    {
      :to => 'high_voltage/pages#show',
      :as => :page,
      :format => false
    }
  ]
end

paired with

HighVoltage.parent_engine.routes.draw do
  if HighVoltage.home_page
    get "/#{HighVoltage.home_page}", to: redirect('/')
    root to: HighVoltage.route_drawer.match_attributes[:to], id: HighVoltage.home_page
  end

  if HighVoltage.routes
    get(*HighVoltage.route_drawer.match_attributes)
  end
end

This is neatly internally consistent, but has the problem of backward compatibility. Which could be overcome with a little ugly check on the return value type of match_attributes being an array or hash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant