Replace DashboardManifest with explicit routes

[c-lliope]

Problem:

When users run the administrate:install generator multiple times, the generated routes from the first install prevent following installs from working correctly.

In order to get subsequent installs to run correctly, users must delete the already-generated routes and run the installer again from scratch.

This has thrown many people off, and there is discussion of the issue in:

• uninitialized constant Administrate::Generators::InstallGenerator::DashboardManifest #431
• 0.1.3 - install generator fails uninitialized constant Administrate::Generators::InstallGenerator::DashboardManifest #448
• InstallGenerator::DashboardManifest (NameError) #453

Solution:

We should change the generated routes so they don't rely on a DashboardManifest being defined.

I'm not sure what the best way to do that would be, but here are a couple options

Defining routes directly

This option would look something like:

namespace :admin do
  resources :customers
  resources :orders
  resources :products
  resources :line_items

  root to: "customers#index"
end

That would fix the error that people are getting when they run rails generate administrate:install multiple times, but it would be more work to update the routes if they change over time.

If we went this route, I'd want to look into getting rid of the DashboardManifest completely. It would be cumbersome to update the list of dashboards in multiple locations, and we could potentially look at the routes that the user's defined to determine which sidebar links to display.

Wrap routes in a conditional

Alternatively we could keep it how it is, but wrap the block in a conditional - that might look like:

if defined?(DashboardManifest)
  namespace :admin do
    DashboardManifest::DASHBOARDS.each do |dashboard_resource|
      resources dashboard_resource
    end

    root controller: DashboardManifest::ROOT_DASHBOARD, action: :index
  end
end

Define routes through the engine

This is the approach that RailsAdmin and Active Admin take. It would look like:

Rails.application.routes.draw do
  mount :administrate
end

This approach is rather opaque, and it makes it harder for the developer to see what's going on. Behind the scenes we could use any approach we wanted to make sure the DashboardManifest is defined when we need it.

Thoughts?

What's the best approach to take here?

• Are we comfortable mounting an engine and abstracting away the complexity?
• Is it worth it to introduce a conditional check into peoples' routes file?
• Would we prefer to get rid of the DashboardManifest altogether, and use the user's routes to fill that role instead?

[coreyward]

To be clear, this happened to me on the very first run of the generator.

[sgonyea]

Likewise, I just tried installing and it blew up with this error on the first run.

Given how painful the magic ends up being in Active Admin, I agree that it's better to just define each admin route like you define your app's routes. The problem with most magic in the routes file is that it basically eager loads the world every time you run a basic command.

Rather than using the resources method, perhaps just define a dashboard_resources helper? This would let you internalize definitions for use on the sidebar, while keeping a minimal footprint.

Rails.application.routes.draw do
  namespace :admin do
    dashboard_resources :orders
  end
end

[c-lliope]

@coreyward reading back through your comments, that makes sense. I was able to reproduce it locally as well on my first run.

I believe any of the proposed solutions will resolve the first-run issue as well as the issues on subsequent runs.

I'm going to take a crack at implementing this in the routes file, and see where that gets us. Stay tuned!

[houndci-bot]

Line is too long. [82/80]

[c-lliope]

Alright, I restructured Administrate to get rid of the DashboardManifest and use the routes in its place. Need to experiment a little bit, but it seems like this approach will be simpler and more robust.

[danbee]

I definitely like this solution. It keeps the route files cleaner and means one less configuration file.

[c-lliope]

Sounds good, merging.