refactor(media): Refactor month retrieval and add new function for easier caching by apermo · Pull Request #8688 · WordPress/wordpress-develop

apermo · 2025-04-14T16:35:24Z

This introduces get_media_library_months_with_files() just as described in 63279, this allows an easy and consistent way to create transients in media_library_months_with_files without the need to copy and paste WordPress core code.

Trac ticket: https://core.trac.wordpress.org/ticket/63279#ticket

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2025-04-14T16:35:42Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props apermo, dmsnell, jasonsa19, westonruter.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2025-04-14T16:49:09Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

JasonSa19

LGTM

Extract the months query from wp_enqueue_media() into get_media_library_months_with_files() and cache the result in a transient. The transient is invalidated in clean_post_cache() when an attachment is modified.

src/wp-includes/media.php

@westonruter

Apply @westonruter's suggestion to use object shape notation for wp_get_media_library_months_with_files() return type, with string types matching the actual wpdb query output.

src/wp-includes/media.php

Rename wp_get_media_library_months_with_files() to wp_get_media_library_attachment_months() for clarity and improve the docblock with a usage example and a return description focused on purpose rather than shape.

src/wp-includes/media.php

src/wp-includes/post.php

src/wp-includes/media.php

Rename transient from media_library_months_with_files to wp_media_library_attachment_months to align with the function name. Update the docblock example to use the assertion style matching other core examples.

Ensures wp_get_media_library_attachment_months() returns integer values instead of strings from the database query, producing cleaner typed output.

…nt_months() Move the media_library_months_with_files filter and transient cache logic from wp_media_view_settings() into the function itself, so both call sites (grid view and legacy list view) benefit from caching and filter support. Replace the raw SQL query in media_upload_library_form() with a function call.

dmsnell · 2026-02-25T18:24:29Z

src/wp-includes/media.php

+	 * @since 4.7.4
+	 * @since tbd Moved to {@see wp_get_media_library_attachment_months()}.
+	 *
+	 * @link https://core.trac.wordpress.org/ticket/31071


I don’t think that we are linking Trac tickets like this but I could be wrong. we could easily find an overwhelming list of tickets for every function though if we were consistent in adding every one.

That was there before, I just moved it and didn't want to touch it.

src/wp-includes/media.php

…hment_months() Move the media_library_months_with_files filter back to the two call sites (wp_enqueue_media and media_upload_library_form) where it originally lived. Simplify the function to just the raw query without transient caching or int casting. Remove the now-unnecessary transient invalidation from clean_post_cache(). Update tests to match.

Consolidate the media_library_months_with_files filter inside the function so both call sites just use the getter. No transient caching. Add test for filter override behavior.

…nths()" This reverts commit 018ca41.

apermo · 2026-02-27T20:55:42Z

After your ( @dmsnell and @westonruter ) comments I rolled back to a mix of the original draft and last state, adding the filter in the second file, and using the new function in both places. And updated the unit test.

I also rolled back the type cast to int. As pointed out there is the filter and we cannot ensure that the filter would provide the right type. The code using the result will always need to accept string and int. So the type cast would add no really value.
With this approach, we ensure that the behavior is unchanged for any instance without custom code (or a plugin). But both places will use the new function and the filter, plus the function is unit-tested now.

This approach fixes inconsistency in core, will add a unit test, and will improve extensibility without any possible side effects.

The previous approach is still in the commit history up to ad1cec4

Note:
The reverted commit 018ca41 was a dead end, I thought I'd add the filter to the function, but that would just create an infinite recursion if doing

add_filter(
   'media_library_months_with_files',
   'custom_caching_function_that_uses_wp_get_media_library_attachment_months' 
);

…ttachment_months() docblock

apermo · 2026-02-27T21:04:48Z

src/wp-includes/media.php

 	 *
 	 * @link https://core.trac.wordpress.org/ticket/31071
 	 *
 	 * @param stdClass[]|null $months An array of objects with `month` and `year`


@westonruter what do you think, should we update the @param to be in sync with the return type of wp_get_media_library_attachment_months()

apermo · 2026-02-27T21:10:53Z

src/wp-includes/media.php

 	 *
 	 * @since 4.7.4
 	 *
 	 * @link https://core.trac.wordpress.org/ticket/31071


@dmsnell as i said, it was here before, I have no preference.

I checked core, there are a total of 29 matches in 24 files for @link https://core.trac.wordpress.org/ticket/ inside /src/.

If you deem it should be removed, feel free to push a fix.
It was added in this commit: 74e7dab

it was here before

sorry I missed that. if it was there then it’s fine to leave in.

dmsnell

This looks like an incremental improvement that opens up the ability for plugins to reuse the same expensive DB query and then cache it. It looks like a worthwhile change to me.

I want to give @westonruter or anyone else a final chance to speak up about the caching, if they feel strongly like we should eagerly cache the results and try to invalidate them appropriately.

Otherwise, on Thursday I will merge this and set the appropriate @since version.

JasonSa19 approved these changes Apr 14, 2025

View reviewed changes

refactor(media): add transient caching for media library months query

e7d9b93

Extract the months query from wp_enqueue_media() into get_media_library_months_with_files() and cache the result in a transient. The transient is invalidated in clean_post_cache() when an attachment is modified.

apermo force-pushed the patch-63279 branch from 790402d to e7d9b93 Compare February 23, 2026 14:27

Hyperlink the docblock to get_media_library_months_with_files().

409acfe

westonruter reviewed Feb 23, 2026

View reviewed changes

src/wp-includes/media.php Outdated Show resolved Hide resolved

src/wp-includes/media.php Outdated Show resolved Hide resolved

src/wp-includes/media.php Outdated Show resolved Hide resolved

apermo added 2 commits February 23, 2026 20:19

Prefix get_media_library_months_with_files() with wp_

52a87d6

Docs: Use object shape notation for return type

ce3c55a

Apply @westonruter's suggestion to use object shape notation for wp_get_media_library_months_with_files() return type, with string types matching the actual wpdb query output.

dmsnell reviewed Feb 23, 2026

View reviewed changes

src/wp-includes/media.php Outdated Show resolved Hide resolved

dmsnell reviewed Feb 23, 2026

View reviewed changes

src/wp-includes/media.php Outdated Show resolved Hide resolved

Rename function and improve docblock

6f8c4e3

Rename wp_get_media_library_months_with_files() to wp_get_media_library_attachment_months() for clarity and improve the docblock with a usage example and a return description focused on purpose rather than shape.

apermo requested review from dmsnell and westonruter February 24, 2026 18:39

westonruter reviewed Feb 24, 2026

View reviewed changes

apermo added 5 commits February 24, 2026 23:31

Rename transient and fix docblock example style

959d814

Rename transient from media_library_months_with_files to wp_media_library_attachment_months to align with the function name. Update the docblock example to use the assertion style matching other core examples.

Cast year and month to int before returning

52b5ed3

Ensures wp_get_media_library_attachment_months() returns integer values instead of strings from the database query, producing cleaner typed output.

Docs: Reorder param properties to match return type

a7042ee

test(media): Add tests for wp_get_media_library_attachment_months()

ad1cec4

dmsnell reviewed Feb 25, 2026

View reviewed changes

apermo force-pushed the patch-63279 branch from 46e27f3 to b7f1f68 Compare February 27, 2026 20:30

apermo added 2 commits February 27, 2026 21:50

refactor: Move filter into wp_get_media_library_attachment_months()

018ca41

Consolidate the media_library_months_with_files filter inside the function so both call sites just use the getter. No transient caching. Add test for filter override behavior.

Revert "refactor: Move filter into wp_get_media_library_attachment_mo…

95b9d62

…nths()" This reverts commit 018ca41.

apermo requested review from dmsnell and westonruter February 27, 2026 21:01

docs: Remove outdated caching information from wp_get_media_library_a…

4e736b1

…ttachment_months() docblock

apermo commented Feb 27, 2026

View reviewed changes

dmsnell approved these changes Mar 3, 2026

View reviewed changes

Merge branch 'trunk' into patch-63279

f7bc910

Conversation

apermo commented Apr 14, 2025

Uh oh!

github-actions bot commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 14, 2025

Test using WordPress Playground

Some things to be aware of

Uh oh!

JasonSa19 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmsnell Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

apermo Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

apermo commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apermo Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

apermo Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

dmsnell Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

dmsnell left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Apr 14, 2025 •

edited

Loading

apermo commented Feb 27, 2026 •

edited

Loading