Fix issue with loading dynamic routes

[davidlormor]

Description

Per the [Rails guides](https://guides.rubyonrails.org/routing.html#breaking-up-very-large-route-file-into-multiple-small-ones for details), you should not call Avo::Engine.routes.draw again when loading routes from another file.

This is related to #3368. Doesn't actually fix the issue, but this change is required in order to ensure that an :id constraint can be applied to all resource-related routes.

Based on the Rails guides, I would suggest the following change, but didn't want to do anything too drastic:

# config/routes.rb
Avo::Engine.routes.draw.do
  # ...other routes
  scope "resources", as: "resources" do
    draw :resources
  end
end

# config/routes/resources.rb
Avo::Resources::ResourceManager.fetch_resources.map do |resource|
  resources resource.route_key do
    member do
      get :preview
    end
  end
end

If you want to take it even a step further, the Rails guides suggests only breaking up your routes file when it goes extremely large and complex, so an even simpler version would be:

# config/routes.rb
Avo::Engine.routes.draw.do
  # ...other routes
  scope "resources", as: "resources" do
    Avo::Resources::ResourceManager.fetch_resources.map do |resource|
      resources resource.route_key do
        member do
          get :preview
        end
      end
    end
  end
end

Either method would ensure that any potential fix to #3368 would be able to properly apply a routing constraint to all resource routes.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

1. Referenced this change in an existing Avo implementation and confirmed that all resource routes are still working as expected.

[codeclimate]

Code Climate has analyzed commit 41ee230 and detected 0 issues on this pull request.

View more on Code Climate.

[Paul-Bob]

This approach isn't generating the routes as expected.

In my opinion, Avo::DynamicRouter no longer needs this level of abstraction. Previously, it had more code, making the extraction useful, but now it’s compact enough to apply directly in config/routes.rb instead of a separate file.

Let's remove the lib/avo/dynamic_router.rb and directly apply the code where Avo::DynamicRouter.routes was called.

Replace

    # Generate resource routes as below:
    # resources :posts
    Avo::DynamicRouter.routes

With

    # Generate resource routes as below:
    # resources :posts
    Avo::Resources::ResourceManager.fetch_resources.map do |resource|
      resources resource.route_key do
        member do
          get :preview
        end
      end
    end

P.S. I just noticed you suggested this in the description as well. At this point, the change makes sense. It didn’t when Avo::DynamicRouter was more complex.

[Paul-Bob]

Thank you @davidlormor for this contribution!

[davidlormor]

🙌 no problem @Paul-Bob - happy to help get that cleaned up! Especially now that I've realized my challenges with ID constraints (#3368) are a bit more broad than I'd originally realized. Turns out the proprietary data set that I'm working with has several tables with composite keys in addition to some of the other ID formats already mentioned. I also realized that all of the workarounds (to_param, friendly_id, etc.) all seem to be strictly on or off, which isn't great because we'll likely need to use some of these IDs in their original format for SEO purposes.

I'm taking some time to work through the whole data set so I can have a better grasp of the scope of the issue, and which parts are Avo-related vs. Rails-related.

[adrianthedev]

Thanks Paul for pushing this over the finish line and thanks David for helping us deal with our dirty laundry 🙌