Add favicon to Django API

[madewithkode]

Fixes

Fixes #4341 by @sarayourfriend

Description

Added Favicon to Django API

Testing Instructions

Trying to serve favicons for the API index managed by drf-spectacular proved inconsistent and problematic, hence the recommendation to manage serving of favicons at the proxy level. Thus, to see this change in development you need to access via the proxy on port 50270 as against the Django server(port 50280) directly.

• Start the API server -> just a

• Navigate to localhost:50270

You should be able to observe an Openverse Favicon present on the tab.

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• I ran the DAG documentation generator (just catalog/generate-docs for catalog
  PRs) or the media properties generator (just catalog/generate-docs media-props
  for the catalog or just api/generate-docs for the API) where applicable.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[krysal]

Thank you for tacking on this, @madewithkode! This looks like a good solution, though after searching in the Spetacular documentation, there seems to be a simpler approach. It should be enough specifying the favicon URL in the SWAGGER_UI_FAVICON_HREF configuration (see the previous link).

I'll get back to you with what URL to use, I think we'll avoid using static files via Django.

[madewithkode]

Thanks for pointing this out @krysal. Certainly easier and more straightforward lol. Would be standing by for the link!
Also, does Spectacular related config changes require any special file regeneration commands or steps in order for them to take effect or they're picked up in real time?

[krysal]

@madewithkode Forget what I said about the file location/URL. It should be good there in the static folder!

Also, does Spectacular related config changes require any special file regeneration commands or steps in order for them to take effect or they're picked up in real time?

I don't think running a command is necessary, but @dhruvkb can confirm with us since he knows more about Spectacular.

[dhruvkb]

@krysal is right that SWAGGER_UI_FAVICON_HREF should be the approach here. I made the following change to the api/conf/settings/spectacular.py file and the favicon showed up immediately (this diff also includes a fix for the broken logo in the API docs).

With this change^, you should drop the redoc.html template and undo the changes to openapi.py.

[dhruvkb]

Suggested change

<g clip-path="url(#clip0_4508_11559)">

[dhruvkb]

Suggested change

	</g>
	<defs>
	<clipPath id="clip0_4508_11559">
	<rect width="8.8" height="8" fill="white" transform="translate(3.59998 4)"/>
	</clipPath>
	</defs>

[sarayourfriend]

This is also missing (and I believe not addressed by the feedback from Krystle and Dhruv) the favicon on the actual API routes themselves. https://api.openverse.org/v1/images/ exists outside the DRF spectacular view, so won't be covered by the swagger changes, I think.

For that, we'll need to override the rest_framework/api.html template: https://github.com/encode/django-rest-framework/blob/master/rest_framework/templates/rest_framework/api.html

Documentation about that here: https://www.django-rest-framework.org/topics/browsable-api/#customizing

After extending the base template, load static, open the head block, and add <link rel="shortcut icon" href="{% static 'favicon.ico' %}"> (and one for SVG for browsers that support it, more on that below) to the head block along with calling super().

One last thing: SVG favicons are not supported by all browsers, notably Safari on iOS, even on the latest version: https://caniuse.com/link-icon-svg

I discussed this in the PR that updated the frontend favicon, but we need to supply both: #3724

You can generate a multi-size ico from an svg using svg-to-ico, but I found I needed to do some manual editing of the ico file in GIMP to remove sizes we didn't have before, and to ensure the size of the file was not unnecessarily huge. This was probably the most complex and definitely the most tedious part of the issue (and hence why I probably wouldn't necessarily recommend this work as a good first issue, by the way, @dhruvkb, in reference to your suggestion on the issue). At the very least, it's a requirement that wasn't spelled out in the issue, and only evident through context of the other recent favicons in our codebase.

[madewithkode]

Hi @dhruvkb /@krysal I followed your suggestions of using spectacular-drfs SWAGGER_UI_FAVICON_HREF and sadly I couldn't quite get it to work. Tried dropping the image and the volumes, repeatedly rebuilt, all with no luck. However the modifications to the logo url worked and the image was displayed. A look at the api web container logs kept showing that as against the specified favicon.svg the request was indeed looking for favicon.ico which was weird.

Also @sarayourfriend, for individual API pages outside of drf-spectacular context, I tried following your suggested approach by overriding rest_framework/api.html and {% block head %} with the links to the favicon as described in the docs and also tried calling {% block.super %} but without any luck. I don't know if I am missing something. Regarding generating a multi-size ico from an svg and doing some other modifications with gimp, like you mentioned, I'm not so sure how to go about those, is this something that can be done and the final blobs shared with me somehow?

In the meantime I have updated the PR with the latest changes I tried and also converted it to draft as it is obviously not ready and might require a few more iterations. Looking forward to your insights.

[dhruvkb]

I have used https://realfavicongenerator.net/ in the past to generate a whole bunch of favicons (and related images). Maybe consider trying that to process the SVG into ICO (and other useful formats as needed for different browsers and platforms).

SWAGGER_UI_FAVICON_HREF should've worked (even without rebuilding any of the Docker images). I can see it working just fine locally. Did you drop your custom view and template before setting SWAGGER_UI_FAVICON_HREF? Let me pull your PR locally and verify.

[madewithkode]

Hi @dhruvkb, Yes, I indeed dropped the custom view and templates but no luck. I updated the branch with my latest local state so you should be able to test locally, if it works for you then maybe something is wrong with my local environment.
Also, thanks for recommending https://realfavicongenerator.net/, I'd have a look.

[sarayourfriend]

Small issue, but you need to call block.super here:

Suggested change

	{% block head %}
	<link rel="icon" type="image/x-icon" href="{% static 'favicon.ico' %}">
	{% endblock %}
	{% block head %}
	{{ block.super }}
	<link rel="icon" type="image/x-icon" href="{% static 'favicon.ico' %}">
	{% endblock %}

The bigger thing preventing this from working is that there is no favicon.ico file in api/api/static. It should work with svg, as that's already there?

[sarayourfriend]

favicon_package_v0.16.zip

Here's the generated favicon package from the website Dhruv recommended. You just need the favicon.ico file (and the original SVG), everything else is unnecessary for the API.

[madewithkode]

Thanks @sarayourfriend, your suggestion worked and the favicon now displays for individual API pages outside of DRF spectacular's scope. Sadly, still no luck getting the favicon to show up on the API index managed by spectacular using the SWAGGER_UI_FAVICON_HREF declaration. I have repeatedly cleared my browse cache, tried multiple browsers, played around with the url path to favicon.ico and none seemed to work. I have once again updated the MR with my current local state. Hoping this issue can be replicated by someone else.

[sarayourfriend]

Worst case scenario, we can add a rule in the Nginx configuration for the API to proxy a root /favicon.ico request to the static path. In fact, if we just do that with the ico and svg, then we don't need to override the template or anything like that, just put the static files there with Nginx proxying the root-level requests and the browsers will automatically pick up the right file and show it.

Here's the Nginx configuration (template) for the API:

openverse/api/nginx.conf.template

Line 1 in 520d4d9

# The following variables are substituted into the environment using envsubst.

You can test it by visiting the nginx service locally after running just api/up, on port 50270 of localhost. That routes requests through the Nginx container, which is how we serve production. You will probably need to run just dc build nginx to get changes to the template to show up correctly.

Overall, that's probably the simplest solution, even if it's a bit indirect, because it doesn't require fiddling with templates or environment variables. Unfortunately those are proving to be more tempramental than we'd like 😦

[madewithkode]

This makes sense. I'd try it out and revert, hoping it works!

[krysal]

The "funny" thing is that the solution should be similar, modifying the redoc template, that @dhruvkb asked to support templating last year! tfranzel/drf-spectacular#978

It's not working for me tho, so the Nginx solution seems the most straightforward for now. We'll get back to this next week if you need more help.

[sarayourfriend]

@dhruvkb this PR does not close #4373. Did you intend to link them? That issue is for the logo at the top of the sidebar, not the favicon.

[dhruvkb]

@sarayourfriend I had linked them because the change to EXTENSIONS_INFO.x-logo.url in api/conf/settings/spectacular.py should fix the logo in the sidebar.

[sarayourfriend]

The issue's requirements are to host the image ourselves, rather than use GitHub, FWIW, but I suppose it would strictly solve the issue of the logo missing.

[krysal]

Setting the URL from the root makes it work for swagger: http://localhost:50280/v1/swagger/#/

Suggested change

	"SWAGGER_UI_FAVICON_HREF": f"{STATIC_URL}favicon.ico",
	"SWAGGER_UI_FAVICON_HREF": f"/{STATIC_URL}favicon.ico",

Although this does not solve the problem for the index.

[dhruvkb]

Ah yes, I think its worth adding the leading slash to STATIC_URL itself in the conf, but that can be addressed separately.

openverse/api/conf/settings/static.py

Line 10 in 18a6606

STATIC_URL = "static/"

[madewithkode]

@dhruvkb @krysal It appears that doing this at the nginx proxy level as suggested by @sarayourfriend is so far the most consistent solution. Therefore, it's worth noting that accessing the api as served by django would likely not have favicons show up but accessing via the nginx proxy works fine. I have now updated this PR with that change.

[dhruvkb]

The SVG seems broken. I previewed it in https://www.svgviewer.dev/ and it only shows up as a white square with black border.

[madewithkode]

@dhruvkb thanks for spotting the svg issue. Fixed that and updated the PR.

[sarayourfriend]

This is great, much simpler in the end!

[dhruvkb]

It doesn't work directly for the web service, which means I won't get to experience it locally 😢 but it will work on the actual website!