Allow current_page? to match against specific HTTP method(s) with a method: option

[bensheldon]

Motivation / Background

The current_page? helper has historically only matched on GET and HEAD requests. This creates challenges when rendering content around validation errors on POST/PUT/PATCH actions.

This PR adds an optional method: option that allows passing an explicit http method (or methods) to the current_page? helper to match specific http verbs.

This makes it easier to render simple navigations like this:

<ul class="nav nav-tabs">
  <li class="nav-item"><%= link_to "All posts", posts_path, class: ["nav-link", { active: current_page?(posts_path) }] %></li>
  <li class="nav-item"><%= link_to "Create post", new_post_path, class: ["nav-link", { active: current_page?(new_post_path) || current_page?(posts_path, method: :post) }] %></li>
</ul>

This is necessary because both the PostsController#index and PostsController#create share the same URL path but are differentiated by their HTTP method.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[p8]

The extra nesting of this if statement could probably be removed?

Suggested change

	method_matches = if method == :get
	request.get? || request.head?
	else
	request_method = request.method.downcase.to_sym
	if method.is_a?(Array)
	method << :head if method.include?(:get)
	method.include?(request_method)
	else
	method == request_method
	end
	end
	method_matches = if method == :get
	request.get? || request.head?
	elsif method.is_a?(Array)
	request_method = request.method.downcase.to_sym
	method << :head if method.include?(:get)
	method.include?(request_method)
	else
	method == request_method
	end

Or maybe even:

Suggested change

	method_matches = if method == :get
	request.get? || request.head?
	else
	request_method = request.method.downcase.to_sym
	if method.is_a?(Array)
	method << :head if method.include?(:get)
	method.include?(request_method)
	else
	method == request_method
	end
	end
	method_matches = case method
	when :get
	request.get? || request.head?
	when Array
	request_method = request.method.downcase.to_sym
	method << :head if method.include?(:get)
	method.include?(request_method)
	else
	method == request_method
	end

[p8]

Argh, nevermind. request_method is used in the last case.

[bensheldon]

yeah, I couldn't find the prettiest logic myself 😞 I was wanting to preserve the existing/default as a fast-path and the else block is "new behavior".

[bensheldon]

... and I'm trying to avoid creating an array unnecessarily as I could coerce everything to Array(method) but I think I've been warned off that before 😁

[byroot]

Maybe I'm dense, and this change is probably fine, but I don't understand in which case you'd need pagination helpers on a POST.

[p8]

@byroot current_page? is unrelated to pagination. It returns true if "the current request URI was generated by the given options".
https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/UrlHelper.html#method-i-current_page-3F

[bensheldon]

@byroot The current_page? helper is almost exclusively used for adding an "active" state to HTML links in navigation elements within shared-partials and layouts.

That could be pagination, but my targeted need is when the navigation includes "New Post" or "Edit Post" links that render a form that could be submitted and have validation errors. In those cases, there can still be a requirement for an "active" state on the navigation when rendering validation errors. That's not possible without a change like this PR.

[byroot]

Yeah, sorry I had a bit of a brain fart.

I skimmed over the historical issues, seems like that behavior was almost immediately regretted and people wanted it reverted but nothing happened.

I need to find some clear headed time to see if there is an elegant solution to this.