Make WordPress Core

Opened 15 months ago

Closed 6 months ago

Last modified 6 months ago

#44033 closed enhancement (fixed)

Get Avatar Comment Types should be a Function Not Just a Filter

Reported by: dshanske Owned by: pento
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests needs-refresh needs-testing
Focuses: Cc:


Pretty much what it says.

I envision, is_avatar_comment_type, that takes WP_Comment, or a comment_ID and returns true or false.

Right now, if I want to use this filter in the code I use to bypass Gravatar, I have call the filter myself. I don't think it is a good idea to call filters multiple times if you can help it.

Attachments (3)

44033.diff (1.8 KB) - added by dshanske 14 months ago.
44033.2.diff (4.1 KB) - added by birgire 14 months ago.
44033.3.diff (5.6 KB) - added by birgire 14 months ago.

Download all attachments as: .zip

Change History (23)

#1 @lucas05
14 months ago

Can I work on this? It will be my first bug :)
Thank you!


#2 @dshanske
14 months ago

That would be great.

#3 @lucas05
14 months ago

Can you give me some instructions on how to proceed? :)
I was looking at https://codex.wordpress.org/User:HEngel/How_To_Become_A_WordPress_Developer but i don't know if it's a good direction.
Also where can i find the documentation for the Comments component?

Last edited 14 months ago by lucas05 (previous) (diff)

#4 @dshanske
14 months ago

That broader question may do better to ask in the Slack chat as you get a broader opinion. There is no specific documentation by component, though I like the developer resources on WordPress.org

For the ticket, I would create a function that calls the filter then replace all usage of the filter in core with the function.

This ticket was mentioned in Slack in #core by lucas05. View the logs.

14 months ago

14 months ago

#6 @dshanske
14 months ago

  • Keywords has-patch added; good-first-bug removed
  • Milestone changed from Awaiting Review to 4.9.7

I went to the simpler checking of comment type and just moved the filter to a function.

#7 @birgire
14 months ago

  • Keywords has-unit-tests added

The patch in 44033.2.diff:

  • Adds a strict in array check.
  • Adds unit tests for is_avatar_comment_type() under tests/phpunit/tests/comment/isAvatarCommentType.php.

I could not find any explicit tests for the get_avatar_data() function.

But I will try to add a test under tests/phpunit/tests/avatar.php.

Last edited 14 months ago by birgire (previous) (diff)

14 months ago

14 months ago

#8 @birgire
14 months ago

The patch 44033.3.diff additionally:

  • Adjusts the @return description for the is_avatar_comment_type() function.
  • Adds the following tests cases for the get_avatar_data() function:
    • Tests_Avatar::test_get_avatar_data_should_return_gravatar_url_when_input_avatar_comment_type()
    • Tests_Avatar::test_get_avatar_data_should_return_invalid_url_when_input_not_avatar_comment_type()

#9 @dshanske
13 months ago

Looks good to me.

#10 @ocean90
13 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

#11 @pbiron
12 months ago

  • Keywords needs-refresh needs-testing added

This ticket was mentioned in Slack in #core by joshuawold. View the logs.

12 months ago

#13 @pento
12 months ago

  • Milestone changed from 4.9.8 to 4.9.9

4.9.8 is in RC, moving to 4.9.9.

#14 @pento
10 months ago

  • Milestone changed from 4.9.9 to 5.0.1

#15 @pento
7 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#16 @pento
7 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#17 @audrasjb
7 months ago

  • Milestone changed from 5.0.3 to 5.1


5.0.3 is going to be released in a couple of weeks.

It doesn't appear this ticket can be handled in the next couple of weeks (still needs some tests). Let's address it in 5.1 which is coming in February. Feel free to ask for changing the milestone if you think this issue can be quickly resolved.



#18 @pento
6 months ago

  • Owner set to pento
  • Status changed from new to assigned

#19 @pento
6 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 44499:

Comments: Add a new is_avatar_comment_type() function.

This function splits the get_avatar_comment_types filter out of get_avatar_data().

Props dshanske, birgire.
Fixes #44033.

#20 @pento
6 months ago

In 44502:

Comments: Add new tests missed in [44499].

Props dshanske, birgire.
Fixes #44033.

Note: See TracTickets for help on using tickets.