Pass User and Comment Objects to rest_get_avatar_urls

[dshanske]

This function retrieves avatars based on email address. It is used by the comments and user controllers.

However, this means that we are locking ourselves into Gravatar, which is a whole other conversation.

The dependent Comment and User 'prepare_item_for_response' functions pass the email address from a User Object or Comment Object to the rest_get_avatar_urls function.

Instead, they should pass the entire Comment or User object. Since get_avatar_url supports this, it will result in no backward compatibility issues, and this will allow the objects to be filtered using existing filters, assuming that you wanted to return alternative avatar data, such as a local avatar, alternative remote service, what have you.

Since this is internal to Core, it won't result in any data leakage to the API.

[joehoyle]

This makes sense to me, we'll need to change the signature of rest_get_avatar_urls to accept an object too - Another idea would be to define a new function to be used instead; if we don't want to make more magical functions!

[dshanske]

@joehoyle All of the avatar functions accept multiple inputs. Is there an issue with changing the passed variables and the signature? I wish avatars had a different design, but we're stuck with it.

Or, if I prepare the patch, for milestoning this for a point release?

[joehoyle]

I wish avatars had a different design, but we're stuck with it.

That's kinda my point, we are _not_ stuck with it in this case, we have the oppurtinty to make something like rest_get_avatar_urls_for_user if we wanted to; I think.

[dshanske]

Well, that does add a level of complexity to it. I am perfectly happy to write functions like that, but is that the best approach?

[pputzer]

Can we do something about this? I just stumbled upon the issue in one of my plugins and there don't even seem to be enough hooks in the controllers to implement a workaround :(

[pputzer]

Replying to joehoyle:

I wish avatars had a different design, but we're stuck with it.

That's kinda my point, we are _not_ stuck with it in this case, we have the oppurtinty to make something like rest_get_avatar_urls_for_user if we wanted to; I think.

Having implemented a ​workaround in Avatar Privacy, you will then also need a rest_get_avatar_urls_for_comment to handle a similar issue in WP_REST_Comments_Controller::prepare_item_for_response. As is, these functions would each have to reimplement the looping logic. I'm not sure this is worth the duplication.

[TimothyBlynJacobs]

I think passing the full objects to rest_get_avatar_urls() is workable. Considering there aren’t any filters here and the function is really only a wrapper around get_avatar_url, I don’t think a separate function is necessary.

@dshanske, @pputzer do either of you want to work on a patch for this?

[pputzer]

@donmhico's patch looks good to me, except that I think the parameter object should not be referred to as "the Gravatar". I know that's the wording from get_avatar_url(), but I think "The object to retrieve an avatar URL for." would be better to make it clear what the parameter actually is. (The suggested wording would also not reinforce the erroneous conception that "avatar" and "Gravatar" are interchangeable.)

[SergeyBiryukov]

Replying to pputzer:

@donmhico's patch looks good to me, except that I think the parameter object should not be referred to as "the Gravatar". I know that's the wording from get_avatar_url(), but I think "The object to retrieve an avatar URL for." would be better to make it clear what the parameter actually is.

Given that the $id_or_email parameter is described as "The Gravatar to retrieve a URL for" in
get_avatar_url(), get_avatar_data(), get_avatar(), and their respective filters, I'd rather do the same for consistency here.

We could open a new ticket to change that description globally if necessary.

[SergeyBiryukov]

In 45632:

REST API: Allow rest_get_avatar_urls() to accept full user, post, or comment objects, rather than just an email address, to provide better flexibility for alternative avatar data.

Since the function uses get_avatar_url() internally, which already supports it, this should not have any backward compatibility concerns.

Props donmhico, dshanske, pputzer, joehoyle, TimothyBlynJacobs.
Fixes #40030.