WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 6 months ago

#39005 closed defect (bug) (fixed)

REST API: Site URL setting should not be present on multisite installations

Reported by: johnbillion Owned by: pento
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Options, Meta APIs Keywords: has-patch commit dev-reviewed
Focuses: multisite, rest-api Cc:

Description

In register_initial_settings(), the siteurl setting is registered and made available to the REST API. On a multisite installation, this setting is not configurable from the General Settings screen, but due to the above it is configurable from the REST API.

This setting should not be present (and not be available to the REST API) on a multisite installation.

Attachments (2)

39005.diff (2.4 KB) - added by peterwilsoncc 9 months ago.
39005.2.diff (2.1 KB) - added by peterwilsoncc 9 months ago.

Download all attachments as: .zip

Change History (13)

#1 @joehoyle
9 months ago

Makes sense to me, as all Settings are configurable, we should remove this from the endpoint in the case of multisite. If clients want to get information about the site, (such as url) that should be retrieved form the API index, or other places. For posterity, the settings endpoint is to change configurable things, not just to provide information about the site.

@peterwilsoncc
9 months ago

#2 @peterwilsoncc
9 months ago

  • Keywords has-patch added; needs-patch removed

Inelegant with is_multisite() checks when registering the setting.

#3 @pento
9 months ago

In the tests in 39005.diff, I'd just unset the 'url' item, rather than repeating the array.

I'm fine with the conditional to fix the bug.

#4 @rachelbaker
9 months ago

@peterwilsoncc @pento I don't think we should register the setting at all in multisite. I am sitting next to @johnbillion and he confirmed this.

#5 @rachelbaker
9 months ago

  • Keywords needs-refresh added

#6 @peterwilsoncc
9 months ago

  • Keywords needs-refresh removed

In 39005.2.diff:

  • unit tests use an array_diff to remove url
  • option not registered in multisite

#7 @nacin
9 months ago

  • Keywords commit added

Looks good.

#8 @pento
9 months ago

  • Keywords dev-reviewed added
  • Owner set to pento
  • Status changed from new to assigned

#9 @pento
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39468:

REST API: Site URL setting should not be present on multisite installations.

The siteurl setting is registered and made available to the REST API. On a multisite installation, this setting is not configurable from the General Settings screen, but due to the above it is configurable from the REST API.

Props peterwilsoncc.
Fixes #39005.

#10 @pento
9 months ago

In 39469:

REST API: Site URL setting should not be present on multisite installations.

The siteurl setting is registered and made available to the REST API. On a multisite installation, this setting is not configurable from the General Settings screen, but due to the above it is configurable from the REST API.

Merge of [39468] to the 4.7 branch.

Props peterwilsoncc.
Fixes #39005.

#11 @jnylen0
6 months ago

In 40077:

REST API: Skip generating the client test fixtures in multisite mode.

There are a couple of changes to the generated API schemas between single-site and multisite mode - for example, the url and email settings are not present in the settings endpoint (see #39005).

To avoid unexpected changes to the wp-api-generated.js fixture file, skip generating the client test fixtures when running the test suite in multisite mode.

See #39264.

Note: See TracTickets for help on using tickets.