Handle root favicon requests #4241
Conversation
This allows us to move the /favicon special route from static to frontend, removing the icon redirect controller from static in preparation for its retirement. - Remove favicon.ico in public directory as it's not copied to the S3 bucket and it short-circuits matching the route. - Route high up in the routes file so that we don't have to check for/load a content item to do this redirect.
KludgeKML
force-pushed
the
handle-favicon
branch
from
f44a5b6
to
1b3ac8e
Compare
| before_action { expires_in(1.day, public: true) } | ||
|
|
||
| def redirect_to_asset | ||
| redirect_to(view_context.asset_path("favicon.ico"), |
There was a problem hiding this comment.
There was a problem hiding this comment.
When the app compiles its assets, it includes the assets from the gem. So the app has a sprockets manifest that maps a logical name "favicon.ico" to the fingerprinted asset name ("/assets/frontend/favicon-121ab2(etc).ico"). When you ask for the logical name using .asset_path, it looks up the fingerprinted name in the sprockets manifest.
So here we're saying "if someone asks for /favicon.ico, permanently redirect them to /assets/frontend/favicon-be123a(etc).ico".
Here's the same code in Static https://github.com/alphagov/static/blob/main/app/controllers/icon_redirects_controller.rb (where it's doing exactly the same thing, but obviously to "/assets/static/favicon-(blahblah).ico").
There was a problem hiding this comment.
So will you merge this, and then update the Route in Router API, and then remove the controller from static?
There was a problem hiding this comment.
Exactly. This merge first, to let frontend do what static was doing, then an update to publishing-api (new home of special routes as of yesterday) to allow us to point favicon.ico at frontend in the router, then a final update to static to remove the controller/publishing_api tasks.
This allows us to move the /favicon special route from static to frontend, removing the icon redirect controller from static in preparation for its retirement.