Remove unnecessary route #2005
Remove unnecessary route #2005
Conversation
This was already defined by `lib/blacklight/routes/searchable.rb`
2 similar comments
|
@jcoyne I think this change looks good, but it's raising the question of where the Searchable routes are tested? I might be missing them, so I'll spend a minute more looking. |
|
Some testing in blacklight/spec/controllers/catalog_controller_spec.rb |
|
@barmintor I suspect so, but I also suspect it was broken before this too as that route was explicitly to |
|
Is it possible that this route is not actually redundant/unnecessary? When the |
|
@jcoyne that is, it might be wrong or deceptive (especially when you don't name your BL controller Catalog, so it's just a free standing broken route). |
| @@ -72,7 +72,7 @@ def session_tracking_path document, params = {} | |||
| if respond_to?(controller_tracking_method) | |||
| send(controller_tracking_method, params.merge(id: document)) | |||
There was a problem hiding this comment.
Should this message be sent to main_app as well? cf respond_to? guard.
| @@ -72,7 +72,7 @@ def session_tracking_path document, params = {} | |||
| if respond_to?(controller_tracking_method) | |||
| send(controller_tracking_method, params.merge(id: document)) | |||
| else | |||
| blacklight.track_search_context_path(params.merge(id: document)) | |||
| main_app.track_catalog_path(params.merge(id: document)) | |||
There was a problem hiding this comment.
Should there be a log message here warning that falling back to an unsafe default?
There was a problem hiding this comment.
Yeah, it would be nice if these track path worked with more than just catalog... we usually have to add these after the fact in order to make our build not fail (i.e. track for articles vs catalog etc.)
There was a problem hiding this comment.
I think that logging a message about the controller_tracking_method not being defined and falling back on the unsafe default, perhaps with a pointer to some docs, would be a good addition here.
|
Closed via #2031 |
This was already defined by
lib/blacklight/routes/searchable.rb