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

resources macro _should_ accept a block for nesting unRESTful routes #1321

Open
crimson-knight opened this issue Jun 9, 2023 · 2 comments
Open

Comments

@crimson-knight
Copy link
Member

If you have a route that looks like this:

routes :web, "/admin" do
    resources "/users", UsersController, except: [:index, :new, :create, :edit, :destroy] do
        post "/login", UsersController, :login
        post "/logout", UsersController, :logout
        post "/sign-up", UsersController, :create
    end
end

I would expect the routes to be generated like this:

/admin/users/show
/admin/users/login
/admin/users/logout
/admin/users/sign-up

However, this does not happen. The routes are rendered like this:

/admin/users/show
/admin/login
/admin/logout
/admin/sign-up

We should fix this so that anytime you define a route within a resources block, it includes the named route root in the generated route

@crimson-knight crimson-knight changed the title resources routes macro does not nest additional routes in the name space properly resources macro does not nest additional routes in the name space properly Jun 9, 2023
@crimson-knight
Copy link
Member Author

crimson-knight commented Jun 10, 2023

I'm going to expand on this a bit, when using the resources macro, even though it accepts the block with the other request macros (get/post etc) it does not create the routes at all as far as I can tell. This is contrary to what the documentation says.

After doing some digging I found the namespace macro and it does exactly what you'd expect:

routes :web, "/admin" do
    namespace "/users" do
        post "/login", UsersController, :login
        post "/logout", UsersController, :logout
        post "/sign-up", UsersController, :create
    end

    resources "/users", UsersController, except: [:create] 
end

This produces the paths:

/admin/users/show
/admin/users/login
/admin/users/logout
/admin/users/sign-up

I found that when nesting namespaces a little deeper, the amber routes cli tool does not show the full path. It would show the routes from the original example as if they should work, but they aren't accessible at all when passed in as a block to the resources macro. They do work as expected when using the namespace macro.

When I say "deeper" nested, I meant the sign-up method url being /admin/users/sign-up, the routes CLI command will only show the sign-up end-point belonging to he admin scope and shows the URI pattern as /sign-up. So you'd never know if it was actually /admin/sign-up or /admin/users/sign-up from the CLI tool.

As best I can tell, this is an issue with the resources macro and the CLI command, not the amber router itself.

@crimson-knight crimson-knight changed the title resources macro does not nest additional routes in the name space properly resources macro _should_ accept a block for nesting unRESTful routes Jun 27, 2023
@crimson-knight
Copy link
Member Author

I updated my routing example after some further testing. A namespace that shares a route base with the resource must be defined before the resource. Otherwise the route will match to the show action and will cause an error because the string of the route is not a bigint

That exposes another concern: resource routes appear to be tightly coupled to integers for the actions that require an :id in the route.

I need to explore this further, because this should be something we can change. Adding a constraints option for the route, and/or a way to determine what the :id of the path is should also exist. It's common to change from standard sequential ID's to slugs or UUID's and I want to ensure that Amber supports that type of routing.

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