Show defaults in controller annotation

[zverok]

Route may be declared with defaults: {some: param}, which is necessary information to understand how it is called in this case. Some examples (from real codebase, with slightly changed route names for anonymization):

case 1: "by id" vs "all"

get '/users/:id/events', controller: :users, action: :events
get '/users/all/events', controller: :users, action: :events, defaults: {all: true}

Now, controller method events has this annotation:

# @route GET /users/:id/events
# @route GET /users/all/events
def events

...which requires digging deep in code to understand how in "all" case it would understand it is all (or looking into routes.rb, which Chusaku helps to avoid). With proposed addon:

# @route GET /users/:id/events
# @route GET /users/all/events{all: true}
def events

case 2: this or that

get '/dashboard', controller: :dashboard, action: :index, {context: 'general'}
get '/dashboard/mine', controller: :dashboard, action: :index, {context: 'current_user'}

Annotations:

# before:
# @route GET /dashboard
# @route GET /dashboard/mine

# after:
# @route GET /dashboard{context: "generic"}
# @route GET /dashboard/mine{context: "current_user"}

case 2: many of them!

REPORTS.each do |name, definition|
  get '/report/monthly', defaults: {name: name, **definition}
end

Annotations:

# before:
# @route GET /reports/montly
# @route GET /reports/montly
# @route GET /reports/montly
# ...repeat 20 times

# after:
# @route GET /reports/montly{name: "performance"}
# @route GET /reports/montly{name: "money", private: true, group: "weeks"}
# @route GET /reports/montly{name: "messages", group: "days"}
# ...

WDYT?..

One concern with this idea is defaults: frequently used to pass defaults: {format: "json"} or something like that, which current solution renders as any other default variable, but maybe it could be done smarter (by attaching .json to path?..)

[nshki]

🙇🏻‍♂️ Thanks so much for the PR! This is definitely something that hasn't been considered before, so this is a very welcome addition to the gem. Some initial thoughts:

1. With the repeated example above, I believe Rails will only register one of the routes if no custom paths are defined. e.g. If I have 10 /reports/monthly routes with no paths (like monthly_performance_report_path, as an example), hitting /reports/monthly would only use one of the sets of defaults. We should annotate just a single line with the set of defaults that Rails will use if there are multiple "overrides," so to speak.
2. I think leaving the format in the defaults is okay, unless there are explicit situations where something like .json is absolutely necessary for it to work.
3. What do you think about having a space between the annotated defaults and the path to have a clearer visual separation?

[zverok]

1. You are right, I've oversimplified the example, just checked, they have different paths in reality (which still doesn't give enough info to read the controller code comfortably)
2. Yes, I was thinking the same -- it will do for now, if there would be people with good use-cases for more advanced processing, they'll come and tell :)
3. OK, tried it both ways, it is better with space. Pushed.

[zverok]

@nshki I fixed CC's offenses (though I must say that line length 80 seems a bit too limiting for Ruby, especially for inline blocks), plese take a look. PS: Probably it might make sense to set up rubocop the same way the CC set (I checked code with Rubocop, saw it had some offenses previously...)

[zverok]

@nshki Thanks! I applied the suggestions.

My Twitter is just the same as GitHub: @zverok, and of course, I'd be happy to be tagged :)