WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 20 months ago

#34093 new enhancement

New filter: `get_calendar_post_type` in get_calendar()

Reported by: sebastian.pisula Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

Filter for post type in calendar. If I want show posts from others post type

Attachments (7)

34093.patch (2.5 KB) - added by sebastian.pisula 3 years ago.
34093-2.patch (3.2 KB) - added by dwainm 2 years ago.
requested changes added plus a refreshed patch after revision 36405
34093-3.patch (3.2 KB) - added by dwainm 2 years ago.
Patch with updated spacing.
34093-4.diff (5.4 KB) - added by dwainm 20 months ago.
Adding array args parameter with backwards compatibly for older argument types. Changing the filter to be applied to all args and not just the post type. Allow post type to be submitted as an argument. Also laying the foundation allowing for other arguments to be added in future.
34093-5.diff (5.4 KB) - added by dwainm 20 months ago.
Fixed bug where args are being used before filter. Also fix bug where args filter was assigning to $post_type variable and not $args.
34093-6.diff (6.5 KB) - added by dwainm 20 months ago.
Adding args to all filters in the function also improve comments.
34093-7.diff (8.3 KB) - added by dwainm 20 months ago.
Adding basic tests

Download all attachments as: .zip

Change History (13)

#1 @swissspidy
3 years ago

  • Keywords has-patch needs-refresh added

I can see that this enhancement would be helpful.

Just please use $wpdb->prepare to sanitize the post type and check if $post_type is indeed a valid (public) post type.

@dwainm
2 years ago

requested changes added plus a refreshed patch after revision 36405

#2 @dwainm
2 years ago

  • Keywords needs-refresh removed

@swissspidy I've updated the patch with your requested changes. Please confirm if this is in order.

@dwainm
2 years ago

Patch with updated spacing.

#3 @dwainm
2 years ago

Hi @swissspidy, is the refreshed patch sufficient?

#4 follow-up: @swissspidy
2 years ago

Thanks for the ping, and sorry for missing it before!

Implementation-wise it looks OK, just needs a bit of a cleanup eventually (e.g. the spacing is wrong).

I wonder if we should go even one step further and allow passing an $args param to get_calendar(). That way you could configure the post type, whether it should echo, etc. all when calling the function. Of course it should be backwards compatible.

Then, we could have 1 single get_calendar_args filter for these arguments instead of one only for the post type. That means we could use 34093-3.patch to build upon.

@dwainm
20 months ago

Adding array args parameter with backwards compatibly for older argument types. Changing the filter to be applied to all args and not just the post type. Allow post type to be submitted as an argument. Also laying the foundation allowing for other arguments to be added in future.

@dwainm
20 months ago

Fixed bug where args are being used before filter. Also fix bug where args filter was assigning to $post_type variable and not $args.

#5 in reply to: ↑ 4 @dwainm
20 months ago

  • Version set to trunk

Replying to swissspidy:

Thanks for the ping, and sorry for missing it before!

Implementation-wise it looks OK, just needs a bit of a cleanup eventually (e.g. the spacing is wrong).

I wonder if we should go even one step further and allow passing an $args param to get_calendar(). That way you could configure the post type, whether it should echo, etc. all when calling the function. Of course it should be backwards compatible.

Then, we could have 1 single get_calendar_args filter for these arguments instead of one only for the post type. That means we could use 34093-3.patch to build upon.

@swissspidy Please review the changes in 34093-5.diff. The code styling is updated along with the changes your requested.

#6 @swissspidy
20 months ago

  • Component changed from General to Widgets
  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release
  • Version trunk deleted

Version is for indicating when a bug was first introduced.

The patch is coming along nicely, thanks @dwainm!

I'm changing milestone to Future Release for now. Also changing the component as get_calendar() is only used in the calendar widget right now. The patch might benefit from some unit tests.

At first glance, the function's docblock needs to be updated. $args should probably be passed to the get_calendar filter.

See https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/ for our PHP Documentation Standards.

@dwainm
20 months ago

Adding args to all filters in the function also improve comments.

@dwainm
20 months ago

Adding basic tests

Note: See TracTickets for help on using tickets.