WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#30254 closed enhancement (fixed)

add bypass filter to remove months dropdown on post table

Reported by: norcross Owned by: helen
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.1
Component: Posts, Post Types Keywords: has-patch 4.2-early commit
Focuses: administration Cc:

Description

currently the months_dropdown function has a filter months_dropdown_results that can be used to remove the results (and thus hide the dropdown) but the SQL query is still run. this patch adds a filter to simply bypass the entire process (including post type to target specific types but allow others) and avoid running the query

Attachments (6)

30254.patch (747 bytes) - added by norcross 3 years ago.
the filter (including docblocks) to bypass the dropdown query and return
30254.2.patch (746 bytes) - added by norcross 3 years ago.
updated patch to change the filter name
30254.3.patch (807 bytes) - added by norcross 3 years ago.
updated patch to change the filter name (again) and doc block title
30254.4.patch (921 bytes) - added by norcross 3 years ago.
updated docblock formatting per WP standards
30254.5.patch (875 bytes) - added by norcross 3 years ago.
even BETTER DocBlocks. who doesn't like a good DocBlock, am I right?
30254.6.patch (855 bytes) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (29)

@norcross
3 years ago

the filter (including docblocks) to bypass the dropdown query and return

#1 @norcross
3 years ago

  • Keywords has-patch 2nd-opinion dev-feedback added

#5 follow-up: @markjaquith
3 years ago

Works for me. Do we use the include_* naming convention elsewhere, or do we have a convention for filters that enable/disable things?

#6 in reply to: ↑ 5 @DrewAPicture
3 years ago

Replying to markjaquith:

Works for me. Do we use the include_* naming convention elsewhere, or do we have a convention for filters that enable/disable things?

We have a few what I call "boolean" filters with an "enable_" prefix on them. It helps to use a naming convention that is "positive" so that when we check if it's enabled/disabled it doesn't end up as a double negative, e.g. false === disable_*.

@norcross
3 years ago

updated patch to change the filter name

@norcross
3 years ago

updated patch to change the filter name (again) and doc block title

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


3 years ago

#8 @GaryJ
3 years ago

@norcross
3 years ago

updated docblock formatting per WP standards

#9 @DrewAPicture
3 years ago

Some notes on improving 30254.4.patch:

  • Add a version to the @since, probably 4.2.0
  • Looks like the indentation is off on the line following @since
  • Hook docs don't use @return as the first parameter is the return. So in this case, you could actually just use the @return description for the first parameter.
  • Hook docs can't have "Optional" parameters. You can mention in the $post_type parameter description that it is optionally passed to the parent function, but otherwise remove the "Optional." sentence at the beginning

#10 @norcross
3 years ago

@GaryJ and @DrewAPicture - can you two decide which one it is for the optional context?

regarding the @return portion, it's a filter, not a hook (not sure if that matters).

also, I didn't add a @since because I don't know when it will get added. I would say 4.1 would be a good fit....but that's just me.

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


3 years ago

@norcross
3 years ago

even BETTER DocBlocks. who doesn't like a good DocBlock, am I right?

#12 @DrewAPicture
3 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release

Sorry this got confusing.

  • All filters are hooks just the same as all actions are hooks.
  • Parameters in hook docs can't be marked optional because that's a misnomer -- the value is always passed to the primary instance of the hook, optional or not.
  • Adding a version now is no problem, most committers will happily fix it on commit.

#13 follow-up: @GaryJ
3 years ago

  • Keywords 2nd-opinion added

On all things inline-docs, I defer to Drew for the official and correct advice. In this case, all data args for action and filter hooks are optional in that the customising function uses them or not, but they are always passed, so there's no need to document as Optional.. My point was on how to document optional args for functions, and I'd overlooked this was a hook, so my suggestion was somewhat irrelevant here.

This is going to be too late for 4.1, as we're in string freeze, past a couple of betas and approaching release sometime this week I believe.

#14 in reply to: ↑ 13 @rmccue
3 years ago

  • Keywords 4.2-early added; 2nd-opinion removed

Replying to GaryJ:

This is going to be too late for 4.1, as we're in string freeze, past a couple of betas and approaching release sometime this week I believe.

I don't think string freeze matters too much in this instance, but enhancements in general (including new filters) are too late.

#15 @johnbillion
3 years ago

  • Version changed from trunk to 3.1

#16 @iseulde
3 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

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


3 years ago

#18 follow-up: @helen
3 years ago

Couple nitpicks - we tend to use truthy/falsey checks on these (so ! apply_filters() or vice versa), and I think using enable/disable terminology would be more descriptive, as one of the goals is to avoid the query. No opinion at the moment which of those it should be.

#19 in reply to: ↑ 18 @norcross
3 years ago

Replying to helen:

Couple nitpicks - we tend to use truthy/falsey checks on these (so ! apply_filters() or vice versa), and I think using enable/disable terminology would be more descriptive, as one of the goals is to avoid the query. No opinion at the moment which of those it should be.

I'm totally fine with changing the verbiage and going truthy / falsey with it. just lemme know what y'all prefer and I'll update the patch.

#20 follow-up: @SergeyBiryukov
3 years ago

  • Component changed from Administration to Posts, Post Types
  • Keywords commit added; dev-feedback removed

30254.6.patch switches to 'disable' terminology and makes some minor adjustments.

#21 in reply to: ↑ 20 @norcross
3 years ago

Replying to SergeyBiryukov:

30254.6.patch switches to 'disable' terminology and makes some minor adjustments.

so is this good to go, or is anything additional needed from me?

#22 @SergeyBiryukov
3 years ago

Should be good to go, just wanted a second look from @DrewAPicture and @helen.

#23 @helen
3 years ago

  • Owner set to helen
  • Resolution set to fixed
  • Status changed from new to closed

In 31438:

Posts list table: Add a filter to disable the months dropdown.

It was previously possible to prevent it from displaying by filtering everything out from the results, but if one really doesn't want it, they should be able to short-circuit before the query even runs.

props norcross, SergeyBiryukov.
fixes #30254.

Note: See TracTickets for help on using tickets.