Make WordPress Core

Opened 6 weeks ago

Last modified 9 days ago

#44033 new enhancement

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

Reported by: dshanske Owned by:
Milestone: 4.9.7 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests
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 2 weeks ago.
44033.2.diff (4.1 KB) - added by birgire 2 weeks ago.
44033.3.diff (5.6 KB) - added by birgire 2 weeks ago.

Download all attachments as: .zip

Change History (12)

#1 @lucas05
4 weeks ago

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


#2 @dshanske
4 weeks ago

That would be great.

#3 @lucas05
4 weeks 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 4 weeks ago by lucas05 (previous) (diff)

#4 @dshanske
4 weeks 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.

4 weeks ago

2 weeks ago

#6 @dshanske
2 weeks 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
2 weeks 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 2 weeks ago by birgire (previous) (diff)

2 weeks ago

2 weeks ago

#8 @birgire
2 weeks 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
9 days ago

Looks good to me.

Note: See TracTickets for help on using tickets.