get_gravatar_url: allow custom default

[nim65s]

Hi,

This allow users get a clear 404 or a variety of generated images based on the hash.

While here, fix default mm into mp,
ref. https://gravatar.com/site/implement/images/

Also add a test for this.

Please check the following:

• Do the tests still pass?¹
• Does the code comply with the style guide?
  • Run make lint from the Wagtail root.
• For Python changes: Have you added tests to cover the new/fixed behaviour?
• For front-end changes: Did you test on all of Wagtail’s supported environments?²
  • Please list the exact browser and operating system versions you tested:
  • Please list which assistive technologies ³ you tested:
• For new features: Has the documentation been updated accordingly?

Please describe additional details for testing this change.

Footnotes

1. Development Testing ↩

2. Browser and device support ↩

3. Accessibility Target ↩

[squash-labs]

Manage this branch in Squash

Test this branch here: https://nim65smain-dnil3.squash.io

[lb-]

I assume this is to use this for your own custom code?

May I suggest we take a different approach, what if we just make the support for WAGTAIL_GRAVATAR_PROVIDER_URL more robust.

e.g. if you provide a config like this

WAGTAIL_GRAVATAR_PROVIDER_URL = "//www.gravatar.com/avatar?d=robohash"

The util will parse that URL, pull out the query string params, and then merge them with the default params. (e.g. with import urllib.parse as urlparse).

These tests would be suitable to add in wagtail/admin/tests/test_templatetags.py as this is where the template tag is tested and/or the existing test file you have created.

This way we have a more flexible approach that supports qs params for other non-gravatar usage.

e.g. currently if you do something like this:
WAGTAIL_GRAVATAR_PROVIDER_URL = "//robohash.org/?set=4"

I am pretty sure it will not work, as the 'hash' part will be added after the qs and output something like //robohash.org/?set=4/123456hash?d=mm&size=100.

I still think the update to default=mm makes sense but this way we get a more flexible solution, no need to change kwargs on the util and we can even add documentation to suggest ways to change the hash to your favourite in docs/reference/settings.md.

[lb-]

Rough idea, not sure if this will work but might be a good starting point.

1. Rework where the default/size are declared to closer to where we need them.
2. Parse the URL provided, allowing us to pull out and merge the query string, then re-build the URL
3. Still strip the last slash to support existing compatibility
4. Update the default to be mp as per the docs, add docstring for reference to these docs

from urllib.parse import parse_qs, urlparse
# ...

def get_gravatar_url(email, size=50):
    """
    See https://gravatar.com/site/implement/images/ for Gravatar image options
    """

    gravatar_provider_url = getattr(
        settings, "WAGTAIL_GRAVATAR_PROVIDER_URL", "//www.gravatar.com/avatar"
    )

    if (not email) or (gravatar_provider_url is None):
        return None

    default = "mp"
    # requested at retina size by default and scaled down at point of use with css
    size = int(size) * 2
    url = urlparse(gravatar_provider_url)
    email_bytes = email.lower().encode("utf-8")
    hash = safe_md5(email_bytes, usedforsecurity=False).hexdigest()
    gravatar_url = "{base_url}/{hash}?{params}".format(
        gravatar_provider_url="".join(url[:3]).rstrip("/"),
        hash=hash,
        params={"z": size, "d": default, **parse_qs(url.query)},
    )

    return gravatar_url

[lb-]

wagtail/admin/tests/ui/test_sidebar.py will also need to be updated when this is done as it tests the full avatar URL for some reason.

"avatarUrl": "//www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=100&d=mm",

[nim65s]

Hi,

Thanks for the review !

Yes, this is for use in my custom code. But your approach wouldn't work in my use case, as I need some calls with default="mp", and others with default="404" (I want to detect users with a gravatar account).

But we could easily merge your urlparse ideas with the default="mp" addition in the argument list of this function which is already in this PR.

Would this work for you ?

[lb-]

Gravatar will ignore the d Param if you also pass in default, so the configured url could override the d.

However, I think adding the kwarg also is fine but calling this directly may not ever be officially supported.

Thank you for taking on the feedback.

[nandini584]

I have raised the PR #11800, Please have a look. Thanks.