WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 8 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 11 months ago.
39005.2.diff (2.1 KB) - added by peterwilsoncc 11 months ago.

Download all attachments as: .zip

Change History (13)

#1 @joehoyle
11 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.

#2 @peterwilsoncc
11 months ago

  • Keywords has-patch added; needs-patch removed

Inelegant with is_multisite() checks when registering the setting.

#3 @pento
11 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
11 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
11 months ago

  • Keywords needs-refresh added

#6 @peterwilsoncc
11 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
11 months ago

  • Keywords commit added

Looks good.

#8 @pento
11 months ago

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

#9 @pento
11 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
11 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
8 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.