Nested RESTful resource(s) #51

AlfonsoUceda · 2015-04-13T16:12:16Z

This PR adds the feature of nested routes to router:

require 'lotus/router'

router = Lotus::Router.new do
  namespace :admin do
    resources :users
  end

  resources :users do
    resources :posts
    resource :avatar
  end

  resource :identity do
    resource :api_keys
  end

  resource :user do
    resources :comments
  end

  resources :users do
    resources :posts do
      resources :comments do
        collection { get 'search' }
        member     { get 'screenshot' }
      end
      collection { get 'search' }
      member     { get 'screenshot' }
    end
  end
end

puts router.inspector

#          admin_users GET, HEAD  /admin/users                   Users::Index                  
#       new_admin_user GET, HEAD  /admin/users/new               Users::New                    
#          admin_users POST       /admin/users                   Users::Create                 
#           admin_user GET, HEAD  /admin/users/:id               Users::Show                   
#      edit_admin_user GET, HEAD  /admin/users/:id/edit          Users::Edit                   
#           admin_user PATCH      /admin/users/:id               Users::Update                 
#           admin_user DELETE     /admin/users/:id               Users::Destroy                
#           user_posts GET, HEAD  /users/:user_id/posts          Users::Posts::Index           
#        new_user_post GET, HEAD  /users/:user_id/posts/new      Users::Posts::New             
#           user_posts POST       /users/:user_id/posts          Users::Posts::Create          
#            user_post GET, HEAD  /users/:user_id/posts/:id      Users::Posts::Show            
#       edit_user_post GET, HEAD  /users/:user_id/posts/:id/edit Users::Posts::Edit            
#            user_post PATCH      /users/:user_id/posts/:id      Users::Posts::Update          
#            user_post DELETE     /users/:user_id/posts/:id      Users::Posts::Destroy         
#      new_user_avatar GET, HEAD  /users/:user_id/avatar/new     Users::Avatar::New            
#          user_avatar POST       /users/:user_id/avatar         Users::Avatar::Create         
#          user_avatar GET, HEAD  /users/:user_id/avatar         Users::Avatar::Show           
#     edit_user_avatar GET, HEAD  /users/:user_id/avatar/edit    Users::Avatar::Edit           
#          user_avatar PATCH      /users/:user_id/avatar         Users::Avatar::Update         
#          user_avatar DELETE     /users/:user_id/avatar         Users::Avatar::Destroy        
#                users GET, HEAD  /users                         Users::Index                  
#             new_user GET, HEAD  /users/new                     Users::New                    
#                users POST       /users                         Users::Create                 
#                 user GET, HEAD  /users/:id                     Users::Show                   
#            edit_user GET, HEAD  /users/:id/edit                Users::Edit                   
#                 user PATCH      /users/:id                     Users::Update                 
#                 user DELETE     /users/:id                     Users::Destroy                
# new_identity_api_key GET, HEAD  /identity/api_keys/new         Identity::ApiKeys::New        
#     identity_api_key POST       /identity/api_keys             Identity::ApiKeys::Create     
#     identity_api_key GET, HEAD  /identity/api_keys             Identity::ApiKeys::Show       
# edit_identity_api_key GET, HEAD  /identity/api_keys/edit        Identity::ApiKeys::Edit       
#     identity_api_key PATCH      /identity/api_keys             Identity::ApiKeys::Update     
#     identity_api_key DELETE     /identity/api_keys             Identity::ApiKeys::Destroy    
#         new_identity GET, HEAD  /identity/new                  Identity::New                 
#             identity POST       /identity                      Identity::Create              
#             identity GET, HEAD  /identity                      Identity::Show                
#        edit_identity GET, HEAD  /identity/edit                 Identity::Edit                
#             identity PATCH      /identity                      Identity::Update              
#             identity DELETE     /identity                      Identity::Destroy             
#        user_comments GET, HEAD  /user/comments                 User::Comments::Index         
#     new_user_comment GET, HEAD  /user/comments/new             User::Comments::New           
#        user_comments POST       /user/comments                 User::Comments::Create        
#         user_comment GET, HEAD  /user/comments/:id             User::Comments::Show          
#    edit_user_comment GET, HEAD  /user/comments/:id/edit        User::Comments::Edit          
#         user_comment PATCH      /user/comments/:id             User::Comments::Update        
#         user_comment DELETE     /user/comments/:id             User::Comments::Destroy       
#             new_user GET, HEAD  /user/new                      User::New                     
#                 user POST       /user                          User::Create                  
#                 user GET, HEAD  /user                          User::Show                    
#            edit_user GET, HEAD  /user/edit                     User::Edit                    
#                 user PATCH      /user                          User::Update                  
#                 user DELETE     /user                          User::Destroy                 
# search_user_post_comments GET, HEAD  /users/:user_id/posts/:post_id/comments/search Users::Posts::Comments::Search
# screenshot_user_post_comment GET, HEAD  /users/:user_id/posts/:post_id/comments/:id/screenshot Users::Posts::Comments::Screenshot
#   user_post_comments GET, HEAD  /users/:user_id/posts/:post_id/comments Users::Posts::Comments::Index 
# new_user_post_comment GET, HEAD  /users/:user_id/posts/:post_id/comments/new Users::Posts::Comments::New   
#   user_post_comments POST       /users/:user_id/posts/:post_id/comments Users::Posts::Comments::Create
#    user_post_comment GET, HEAD  /users/:user_id/posts/:post_id/comments/:id Users::Posts::Comments::Show  
# edit_user_post_comment GET, HEAD  /users/:user_id/posts/:post_id/comments/:id/edit Users::Posts::Comments::Edit  
#    user_post_comment PATCH      /users/:user_id/posts/:post_id/comments/:id Users::Posts::Comments::Update
#    user_post_comment DELETE     /users/:user_id/posts/:post_id/comments/:id Users::Posts::Comments::Destroy
#    search_user_posts GET, HEAD  /users/:user_id/posts/search   Users::Posts::Search          
# screenshot_user_post GET, HEAD  /users/:user_id/posts/:id/screenshot Users::Posts::Screenshot      
#           user_posts GET, HEAD  /users/:user_id/posts          Users::Posts::Index           
#        new_user_post GET, HEAD  /users/:user_id/posts/new      Users::Posts::New             
#           user_posts POST       /users/:user_id/posts          Users::Posts::Create          
#            user_post GET, HEAD  /users/:user_id/posts/:id      Users::Posts::Show            
#       edit_user_post GET, HEAD  /users/:user_id/posts/:id/edit Users::Posts::Edit            
#            user_post PATCH      /users/:user_id/posts/:id      Users::Posts::Update          
#            user_post DELETE     /users/:user_id/posts/:id      Users::Posts::Destroy         
#                users GET, HEAD  /users                         Users::Index                  
#             new_user GET, HEAD  /users/new                     Users::New                    
#                users POST       /users                         Users::Create                 
#                 user GET, HEAD  /users/:id                     Users::Show                   
#            edit_user GET, HEAD  /users/:id/edit                Users::Edit                   
#                 user PATCH      /users/:id                     Users::Update                 
#                 user DELETE     /users/:id                     Users::Destroy

AlfonsoUceda · 2015-04-13T16:14:07Z

lib/lotus/routing/resource.rb

@@ -1,6 +1,7 @@
 require 'lotus/utils/class_attribute'
 require 'lotus/routing/resource/options'
 require 'lotus/routing/resource/action'
+# require 'lotus/routing/resources'


[reminder] remove

AlfonsoUceda · 2015-04-14T13:53:40Z

close #47

nicocharlery · 2015-04-14T14:43:58Z

Thank you for this @AlfonsoUceda !

AlfonsoUceda · 2015-04-15T15:17:14Z

@jodosha @joneslee85 when you have time, can you review this PR? thanks ;)

Erol · 2015-04-17T06:42:43Z

This is exactly what I needed. Thanks @AlfonsoUceda!

Erol · 2015-04-17T06:53:49Z

Hey @AlfonsoUceda, it looks like nested routes lack the ability to accept options:

router.define do
  resources 'products' do
    resources 'variants', only: [:index, :show]
  end
end

#=> ArgumentError: wrong number of arguments (2 for 1)

AlfonsoUceda · 2015-04-17T06:59:40Z

@Erol Good catch, I am going to fix that

Erol · 2015-04-17T07:29:19Z

👍

AlfonsoUceda · 2015-04-17T07:32:57Z

@Erol fixed

Erol · 2015-04-17T08:14:34Z

Great, thanks!

Erol · 2015-04-17T08:29:09Z

Hi @AlfonsoUceda, found another one. Namespaces are being dropped inside nested routes:

router.define do
  namespace 'api' do
    resources 'products' do
      resources 'variants'
    end
  end
end

Here are the routes generated:

   products_variants GET, HEAD  /products/:products_id/variants Products::Variants::Index
new_products_variants GET, HEAD  /products/:products_id/variants/new Products::Variants::New
   products_variants POST       /products/:products_id/variants Products::Variants::Create
   products_variants GET, HEAD  /products/:products_id/variants/:id Products::Variants::Show
edit_products_variants GET, HEAD  /products/:products_id/variants/:id/edit Products::Variants::Edit
   products_variants PATCH      /products/:products_id/variants/:id Products::Variants::Update
   products_variants DELETE     /products/:products_id/variants/:id Products::Variants::Destroy
        api_products GET, HEAD  /api/products                  Products::Index
    new_api_products GET, HEAD  /api/products/new              Products::New
        api_products POST       /api/products                  Products::Create
        api_products GET, HEAD  /api/products/:id              Products::Show
   edit_api_products GET, HEAD  /api/products/:id/edit         Products::Edit
        api_products PATCH      /api/products/:id              Products::Update
        api_products DELETE     /api/products/:id              Products::Destroy

AlfonsoUceda · 2015-04-17T08:32:03Z

thanks @Erol I'm goint to fix that

AlfonsoUceda · 2015-04-17T08:54:12Z

Ok @Erol the problem was I wasn't merging parents options

Erol · 2015-04-17T08:59:47Z

Thanks again for this @AlfonsoUceda!

AlfonsoUceda · 2015-04-17T09:15:54Z

I added more tests

jodosha · 2015-04-17T10:53:15Z

@AlfonsoUceda Thank you for this PR 💯 ✨
Can you please add support for member and collection routes? Thank you very much!

AlfonsoUceda · 2015-04-17T10:57:01Z

@jodosha IMO add nested routes to member and collection block it doesn't make sense.

could you add an example?

thanks ;)

jodosha · 2015-04-17T11:15:11Z

@AlfonsoUceda /admin/users/search for collection and /organizations/1/projects/123/configure for member.

AlfonsoUceda · 2015-04-17T11:16:49Z

@jodosha how would it be the code (routes definition)?

AlfonsoUceda · 2015-04-18T16:59:26Z

@jodosha I created another router in test, because with namespaced router routes is different, you can see it at the end of the file

Erol · 2015-04-19T02:39:48Z

This is awesome work! 👍

jodosha · 2015-04-20T15:10:02Z

test/integration/nested_routes_test.rb

+
+      resource :user do
+        resources :comments
+        resource :api_keys


@AlfonsoUceda Shouldn't be this api_key instead of api_keys?

jodosha · 2015-04-22T08:02:47Z

test/integration/nested_routes_test.rb

+      end
+
+      it 'for resource -> resource' do
+        @inspector.must_match 'new_user_api_key GET, HEAD  /user/:user_id/api_key/new     Nested::Controllers::User::ApiKey::New'


@AlfonsoUceda This is wrong, it should be: /user/api_key/new. With resource you never have :user_id around. That is something that resources has (notice the plural).

you are rigth @jodosha I'll fix that ;)

jodosha · 2015-04-22T08:27:16Z

@AlfonsoUceda Here's another issue. Given the following routes:

resources :users do
  resources :posts
end

(byebug) puts @router.inspector
    new_users_avatar GET, HEAD  /users/:users_id/avatar/new    Nested::Controllers::Users::Avatar::New
        users_avatar POST       /users/:users_id/avatar        Nested::Controllers::Users::Avatar::Create
        users_avatar GET, HEAD  /users/:users_id/avatar        Nested::Controllers::Users::Avatar::Show
   edit_users_avatar GET, HEAD  /users/:users_id/avatar/edit   Nested::Controllers::Users::Avatar::Edit
        users_avatar PATCH      /users/:users_id/avatar        Nested::Controllers::Users::Avatar::Update
        users_avatar DELETE     /users/:users_id/avatar        Nested::Controllers::Users::Avatar::Destroy
         users_posts GET, HEAD  /users/:users_id/posts         Nested::Controllers::Users::Posts::Index
     new_users_posts GET, HEAD  /users/:users_id/posts/new     Nested::Controllers::Users::Posts::New
         users_posts POST       /users/:users_id/posts         Nested::Controllers::Users::Posts::Create
         users_posts GET, HEAD  /users/:users_id/posts/:id     Nested::Controllers::Users::Posts::Show
    edit_users_posts GET, HEAD  /users/:users_id/posts/:id/edit Nested::Controllers::Users::Posts::Edit
         users_posts PATCH      /users/:users_id/posts/:id     Nested::Controllers::Users::Posts::Update
         users_posts DELETE     /users/:users_id/posts/:id     Nested::Controllers::Users::Posts::Destroy
               users GET, HEAD  /users                         Nested::Controllers::Users::Index
           new_users GET, HEAD  /users/new                     Nested::Controllers::Users::New
               users POST       /users                         Nested::Controllers::Users::Create
               users GET, HEAD  /users/:id                     Nested::Controllers::Users::Show
          edit_users GET, HEAD  /users/:id/edit                Nested::Controllers::Users::Edit
               users PATCH      /users/:id                     Nested::Controllers::Users::Update
               users DELETE     /users/:id                     Nested::Controllers::Users::Destroy
nil
(byebug) puts @router.path(:users_posts, users_id: 1, id: 23)
/users/1/posts/23
nil
(byebug) puts @router.path(:users_posts, users_id: 1)
*** HttpRouter::InvalidRouteException Exception: No route (path) could be generated for :users_posts # THIS SHOULD BE GENERATED

nil
(byebug) puts @router.path(:users)
/users
nil
(byebug) puts @router.path(:users, id: 1)
/users/1
nil

There is a conflict between :users_posts as way to show /users/1/posts/23 and the same name is used for /users/1/posts.

Right now in master, we have this conflict as well, but it works fine. See the last two lines above: :users is used both for /users and /users/23.

At this point, instead of fixing this I'm wondering if it's the time to introduce the right pluralization rules. In this way it won't create conflicts, and at the same time is improves the dev experience: users_id for a single user is ugly.

The would become:

(byebug) puts @router.inspector
     new_user_avatar GET, HEAD  /users/:user_id/avatar/new     Nested::Controllers::Users::Avatar::New
         user_avatar POST       /users/:user_id/avatar         Nested::Controllers::Users::Avatar::Create
         user_avatar GET, HEAD  /users/:user_id/avatar         Nested::Controllers::Users::Avatar::Show
    edit_user_avatar GET, HEAD  /users/:user_id/avatar/edit    Nested::Controllers::Users::Avatar::Edit
         user_avatar PATCH      /users/:user_id/avatar         Nested::Controllers::Users::Avatar::Update
         user_avatar DELETE     /users/:user_id/avatar         Nested::Controllers::Users::Avatar::Destroy
          user_posts GET, HEAD  /users/:user_id/posts          Nested::Controllers::Users::Posts::Index
       new_user_post GET, HEAD  /users/:user_id/posts/new      Nested::Controllers::Users::Posts::New
          user_posts POST       /users/:user_id/posts          Nested::Controllers::Users::Posts::Create
           user_post GET, HEAD  /users/:user_id/posts/:id      Nested::Controllers::Users::Posts::Show
      edit_user_post GET, HEAD  /users/:user_id/posts/:id/edit Nested::Controllers::Users::Posts::Edit
           user_post PATCH      /users/:user_id/posts/:id      Nested::Controllers::Users::Posts::Update
           user_post DELETE     /users/:user_id/posts/:id      Nested::Controllers::Users::Posts::Destroy
               users GET, HEAD  /users                         Nested::Controllers::Users::Index
            new_user GET, HEAD  /users/new                     Nested::Controllers::Users::New
               users POST       /users                         Nested::Controllers::Users::Create
                user GET, HEAD  /users/:id                     Nested::Controllers::Users::Show
           edit_user GET, HEAD  /users/:id/edit                Nested::Controllers::Users::Edit
                user PATCH      /users/:id                     Nested::Controllers::Users::Update
                user DELETE     /users/:id                     Nested::Controllers::Users::Destroy
nil

This would fix the problem above: :user_posts will generate /users/1/posts, while :user_post will have /users/1/posts/23 as output.

/cc @joneslee85

runlevel5 · 2015-04-22T09:19:59Z

After talking with @jodosha, we agreed on introducing Util::String#pluralize, and inflection rules can be parsed to the constructor like:

initialize(string, inflections = nil)

where inflections can be a hash { "knife" => "knives" }

Waiting for @jodosha to cook up a patch in lotus/utils

runlevel5 · 2015-05-07T13:10:01Z

lib/lotus/routing/nested.rb

+
+      def _calculate(param_wildcard, resource = nil)
+        return if resource.nil?
+          @path << resource.wildcard_param(param_wildcard.pop)


what do you think about aligning to the same level as return?

can you more detailed? thanks @joneslee85 ;)

I think @joneslee85 meant lines 36-38 should have the same indentation. :)

return if resource.nil? @path << resource.wildcard_param(param_wildcard.pop) _calculate(param_wildcard, resource.parent_resource)

Thanks guys I didn't see it sorry ;P

runlevel5 · 2015-05-08T09:04:16Z

lib/lotus/routing/resource/action.rb

@@ -179,7 +188,8 @@ def rest_path
        # @api private
        # @since 0.1.0
        def as
-          namespace.relative_join(resource_name, self.class.named_route_separator).to_sym
+          singularized_as = resource_name.to_s.split(NESTED_ROUTES_SEPARATOR).map { |name| Lotus::Utils::String.new(name).singularize }.join(self.class.named_route_separator)


This long is a bit long, can you break into 2 lines?

runlevel5 · 2015-05-08T09:22:44Z

It is probably the longest PR I've ever reviewed, overall, it looks good to me

AlfonsoUceda reviewed Apr 13, 2015
View reviewed changes

AlfonsoUceda force-pushed the nested_routes branch 5 times, most recently from 8debb02 to 7109574 Compare April 14, 2015 13:26

AlfonsoUceda changed the title ~~Added nested routes [WIP]~~ Added nested routes Apr 14, 2015

AlfonsoUceda force-pushed the nested_routes branch from 7109574 to 4b08e61 Compare April 17, 2015 07:32

AlfonsoUceda force-pushed the nested_routes branch from 4b08e61 to 8b979f8 Compare April 17, 2015 08:53

AlfonsoUceda force-pushed the nested_routes branch from 8b979f8 to 8d58f18 Compare April 17, 2015 09:15

jodosha self-assigned this Apr 17, 2015

AlfonsoUceda force-pushed the nested_routes branch from fcdb7ed to 97d069f Compare April 18, 2015 16:18

AlfonsoUceda force-pushed the nested_routes branch from 97d069f to 52f1b95 Compare April 18, 2015 18:47

jodosha reviewed Apr 20, 2015
View reviewed changes

Added nested routes

e85f7e1

AlfonsoUceda force-pushed the nested_routes branch from 52f1b95 to e85f7e1 Compare April 21, 2015 08:58

jodosha reviewed Apr 22, 2015
View reviewed changes

jodosha mentioned this pull request Apr 23, 2015

String pluralization hanami/utils#72

Merged

jodosha mentioned this pull request May 4, 2015

String Singularization hanami/utils#73

Merged

Added singularization and pluralization

7f8c47c

AlfonsoUceda force-pushed the nested_routes branch from ca1fc0b to 2b78dae Compare May 7, 2015 12:03

runlevel5 reviewed May 7, 2015
View reviewed changes

AlfonsoUceda force-pushed the nested_routes branch 3 times, most recently from 9044eea to 9205df4 Compare May 8, 2015 06:46

runlevel5 reviewed May 8, 2015
View reviewed changes

Fixed nested routes

57b462c

AlfonsoUceda force-pushed the nested_routes branch from 9205df4 to 57b462c Compare May 8, 2015 13:12

jodosha changed the title ~~Added nested routes~~ Nested RESTful resource(s) May 8, 2015

jodosha merged commit 57b462c into hanami:master May 11, 2015

jodosha added a commit that referenced this pull request May 11, 2015

Cleanup per #51

f2b99ae

jodosha mentioned this pull request May 11, 2015

Nested RESTful resource(s) #47

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nested RESTful resource(s) #51

Nested RESTful resource(s) #51

AlfonsoUceda commented Apr 13, 2015

AlfonsoUceda Apr 13, 2015

AlfonsoUceda commented Apr 14, 2015

nicocharlery commented Apr 14, 2015

AlfonsoUceda commented Apr 15, 2015

Erol commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

jodosha commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

jodosha commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

AlfonsoUceda commented Apr 18, 2015

Erol commented Apr 19, 2015

jodosha Apr 20, 2015

jodosha Apr 22, 2015

AlfonsoUceda Apr 22, 2015

jodosha commented Apr 22, 2015

runlevel5 commented Apr 22, 2015

runlevel5 May 7, 2015

AlfonsoUceda May 7, 2015

Erol May 8, 2015

AlfonsoUceda May 8, 2015

runlevel5 May 8, 2015

runlevel5 commented May 8, 2015

Nested RESTful resource(s) #51

Nested RESTful resource(s) #51

Conversation

AlfonsoUceda commented Apr 13, 2015

Choose a reason for hiding this comment

AlfonsoUceda commented Apr 14, 2015

nicocharlery commented Apr 14, 2015

AlfonsoUceda commented Apr 15, 2015

Erol commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

Erol commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

jodosha commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

jodosha commented Apr 17, 2015

AlfonsoUceda commented Apr 17, 2015

AlfonsoUceda commented Apr 18, 2015

Erol commented Apr 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodosha commented Apr 22, 2015

runlevel5 commented Apr 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runlevel5 commented May 8, 2015