WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 10 months ago

Last modified 10 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:
PR Number:

Description

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 17 months ago.
44033.2.diff (4.1 KB) - added by birgire 17 months ago.
44033.3.diff (5.6 KB) - added by birgire 17 months ago.

Download all attachments as: .zip

Change History (23)

#1 @lucas05
17 months ago

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

Luca

#2 @dshanske
17 months ago

That would be great.

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

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


17 months ago

@dshanske
17 months ago

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

@birgire
17 months ago

@birgire
17 months ago

#8 @birgire
17 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
17 months ago

Looks good to me.

#10 @ocean90
16 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

#11 @pbiron
16 months ago

  • Keywords needs-refresh needs-testing added

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


16 months ago

#13 @pento
15 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
13 months ago

  • Milestone changed from 4.9.9 to 5.0.1

#15 @pento
10 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#16 @pento
10 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#17 @audrasjb
10 months ago

  • Milestone changed from 5.0.3 to 5.1

Hello,

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.

Cheers,

Jb

#18 @pento
10 months ago

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

#19 @pento
10 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
10 months ago

In 44502:

Comments: Add new tests missed in [44499].

Props dshanske, birgire.
Fixes #44033.

Note: See TracTickets for help on using tickets.