WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#40458 closed defect (bug) (fixed)

Cannot retrieve custom site details values

Reported by: ocean90 Owned by: ocean90
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.6
Component: Networks and Sites Keywords: has-patch has-unit-tests commit
Focuses: multisite Cc:
PR Number:

Description

With the blog_details filter and get_blog_details() function it's possible to set and retrieve custom site details. That's not possible with the site_details filter because WP_Site::get_details() is private and WP_Site::__get() uses a whitelist for blogname, siteurl, post_count and home.

I think we should adjust WP_Site::__get() to always fall back to details, see attached patch. This would be one more step to make get_site() a direct replacement for get_blog_details().

Related: [38936], #36935

Attachments (2)

40458.patch (1.7 KB) - added by ocean90 3 years ago.
40458.2.patch (2.6 KB) - added by ocean90 3 years ago.

Download all attachments as: .zip

Change History (9)

@ocean90
3 years ago

#1 @ocean90
3 years ago

Also noticed that the magic setter allows to set any property but there is no way to retrieve it by the current getter and isset-er.

#2 follow-up: @flixos90
3 years ago

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

@ocean90 Good point, that should definitely be possible. It actually feels more like an enhancement to me, but since it was possible prior, it's probably a bug.

40458.patch looks good, I think we should split the unit test into two parts, one for the __isset() and one for __get() to have one assertion per test.

Also noticed that the magic setter allows to set any property but there is no way to retrieve it by the current getter and isset-er.

Let's deal with that in a separate ticket, okay?

@ocean90
3 years ago

#3 in reply to: ↑ 2 @ocean90
3 years ago

Replying to flixos90:

It actually feels more like an enhancement to me, but since it was possible prior, it's probably a bug.

Yeah, it's kinda both. It's an enhancement to the new site details handling and a bug if we want to promote get_site() as a direct replacement for get_blog_details().

I think we should split the unit test into two parts, one for the __isset() and one for __get() to have one assertion per test.

Done in 40458.2.patch, also includes a test for a default property.

Let's deal with that in a separate ticket, okay?

Sure, created #40459.

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


3 years ago

#5 follow-up: @johnjamesjacoby
3 years ago

Is the end result to be able to add keys & values to the object that's in site-details?

It's not currently possible (with this patch or without) to save a custom variable in the site-details cache object, because the site_details filter is ran after the site-details cache is getted/setted.

The attached tests filter site_details directly, but those custom values will not be part of the cached details. The magic getters and setters will report back correctly, but any custom variables would need their own separate cache group.

Version 0, edited 3 years ago by johnjamesjacoby (next)

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

  • Keywords has-unit-tests commit added

Replying to johnjamesjacoby:

Is the end result to be able to add keys & values to the object that's in site-details?

Not really. That might be another enhancement which I don't think we have to support in core.

#7 @ocean90
3 years ago

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

In 40478:

Multisite: After [37918] add support for retrieving custom site properties set by the site_details filter.

The behaviour was previously possible with the blog_details filter and get_blog_details() function. The former is deprecated since [38936].
This change adjusts the magic methods of WP_Site to also check if $key exists in WP_Site::get_details().

Fixes #40458.

Note: See TracTickets for help on using tickets.