Skip to content

Conversation

@afragen
Copy link
Contributor

@afragen afragen commented Aug 16, 2025

The function_exists check needs to be moved as function not yet defined and so the entire inc\dashboard-widgets/namespace.php::bootstrap() is skipped.

This also adds caching to the wp_remote_get() call.

Fixes #222

function_exists check needs to be moved as function not yet defined.
Signed-off-by: Andy Fragen <[email protected]>
@afragen afragen self-assigned this Aug 16, 2025
Copy link
Member

@costdev costdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one change request, otherwise looks good

$response = wp_remote_get( EVENTS_API );
if ( is_wp_error( $response ) ) {
return $response;
$response = wp_cache_get( EVENTS_API );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the Transient API here instead. Sites without a persistent cache won't see a benefit when this isn't likely to get hit many times in one page load.

Copy link
Contributor Author

@afragen afragen Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, even without a persistent cache WP_Cache_Object lasts for a page load. It's subsequent page loads that are affected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I mean - the non-persistent cache only lasts for a page load, so using wp_cache_* wouldn't give any benefit for those sites, so the Transient API is better here

@afragen afragen merged commit 44e2351 into fairpm:main Aug 17, 2025
46 checks passed
@afragen afragen deleted the dashboard-events branch August 17, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard News widget doesn't seem to be loading properly

2 participants