Skip to content

Change trailing slash from constraints to default for insights routes

Catalin Irimie requested to merge cat-fix-trailing-slash-defaults into master

What does this MR do and why?

Simply specifying trailing_slash: true as a parameter to the route matcher adds it as both a constraint and option.

However this means url_for will not be able to generate this dynamically, since trailing_slash is a reserved keyword for url_for and cannot be used as an option.

This makes the default, not constraint to have a trailing slash, so that path and URL helpers generate it with the trailing slash, but not require it.

Longer description in !72864 (comment 721630438) with a rabbit-hole dive into ActionDispatch.

Related to !72864 (merged) and #343551 (closed).

How to set up and validate locally

Before:

Gitlab::Routing.url_helpers.namespace_project_insights_url("namespace1", "project2")
=> "http://gdk.test:3000/namespace1/project2/insights/"

Gitlab::Application.routes.generate_url_helpers(true).url_for({:controller=>"projects/insights", :action=>"show", :namespace_id=>"namespace1", :project_id=>"project2"})
ActionController::UrlGenerationError: No route matches {:action=>"show", :controller=>"projects/insights", :namespace_id=>"namespace1", :project_id=>"project2"}

After:

Gitlab::Routing.url_helpers.namespace_project_insights_url("namespace1", "project2")
=> "http://gdk.test:3000/namespace1/project2/insights/"

Gitlab::Application.routes.url_for({:controller=>"projects/insights", :action=>"show", :namespace_id=>"namespace1", :project_id=>"project2"})
=> "http://gdk.test:3000/namespace1/project2/insights"

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports