Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • A administrate
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 96
    • Issues 96
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 32
    • Merge requests 32
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • thoughtbot, inc.
  • administrate
  • Merge requests
  • !464

Replace DashboardManifest with explicit routes

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Administrator requested to merge gw-routes-generator into master Feb 13, 2016
  • Overview 6
  • Commits 1
  • Pipelines 0
  • Changes 16

Created by: gracewashere

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:

  • #431 (closed)
  • #448 (closed)
  • #453 (closed)

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?
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: gw-routes-generator