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

Allow multiple posts with different paths in a resource #13

Closed
wants to merge 2 commits into from

Conversation

njd5475
Copy link

@njd5475 njd5475 commented May 10, 2016

The path prefix, suffix doesn't seem to be part of the options hash in grape routes only the namespace which does not allow for multiple post routes.

@@ -10,7 +10,13 @@ def decorated_routes

def all_routes
routes = subclasses.flat_map { |s| s.send(:prepare_routes) }
routes.uniq(&:options)
routes.uniq do |r|
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of implementing a method on the route that returns the attributes that make it unique? In this case it's the route options + the path. Something like this:

class DecoratedRoute
  attr_reader :unique_key

  def initialize(route)
    # setting other instance variables
   @unique_key = route_options.merge(path: route.path)
  end
end

Then we can shorten lines 13-19 to routes.uniq(&:unique_key). What do you think?

@reprah
Copy link
Owner

reprah commented Dec 18, 2016

@njd5475 Hi, this bug is fixed in the current master branch. There were some other things I wanted to change so I couldn't merge your PR, but I mentioned you in the commit and I'm really thankful for your help!

@reprah reprah closed this Dec 18, 2016
@njd5475
Copy link
Author

njd5475 commented Dec 18, 2016

@reprah Thanks for the mention, all good stuff, keep it up! 😁

P.S. Sorry about not responding sooner been busy lately!

@reprah
Copy link
Owner

reprah commented Dec 18, 2016

@njd5475 No worries, thank you again :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants