WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#38990 closed defect (bug) (fixed)

REST API: admin_email setting description is inaccurate

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

Description

The description field for the admin_email setting has been copied from the Multisite description (1). The description and the behaviour of this field in the admin area actually differs between single site and Multisite (2).

Not strictly related to the REST API, but the setting was introduced for it.

(1) https://core.trac.wordpress.org/browser/trunk/src/wp-includes/option.php?rev=39335&marks=1751-1760#L1750
(2) https://core.trac.wordpress.org/browser/trunk/src/wp-admin/options-general.php?rev=39335&marks=81-83,100-102#L80

Attachments (2)

38990.diff (838 bytes) - added by ocean90 5 months ago.
38990.2.diff (1.1 KB) - added by nacin 5 months ago.

Download all attachments as: .zip

Change History (24)

This ticket was mentioned in Slack in #core by helen. View the logs.


5 months ago

@ocean90
5 months ago

#2 @ocean90
5 months ago

  • Keywords has-patch added; needs-patch removed

38990.diff uses is_multisite() to change the message based on context.

#3 @ocean90
5 months ago

  • Keywords commit dev-feedback added

#4 @rachelbaker
5 months ago

  • Keywords dev-reviewed added; dev-feedback removed

#5 @rachelbaker
5 months ago

  • Owner set to rachelbaker
  • Status changed from new to assigned

#6 @rachelbaker
5 months ago

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

In 39406:

REST API: Correct the admin_email setting description for single site installs.

Display different descriptions for multisite or single site installations.

Props johnbillion, ocean90.
Fixes #38990.

#7 @rachelbaker
5 months ago

In 39407:

REST API: Correct the admin_email setting description for single site installs.
Display different descriptions for multisite or single site installations.

Props johnbillion, ocean90.

Merges [39406] to the 4.7 branch.
Fixes #38990 for 4.7

#8 @johnbillion
5 months ago

  • Keywords needs-patch added; has-patch commit dev-reviewed removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Turns out the issue is a little more complex.

On Multisite, the admin_email input field is actually for an option named new_admin_email which gets set immediately when the form is saved. Only after the user clicks the link in their confirmation email, does the actual admin_email option get updated.

The REST API should use this same model to prevent users from accidentally changing their email address to something invalid.

#9 @johnbillion
5 months ago

  • Focuses multisite added

#10 @netweb
5 months ago

  • Keywords i18n-change added

Noting r39406 resulted in a string change, Slack discussion here

This ticket was mentioned in Slack in #core-restapi by rachelbaker. View the logs.


5 months ago

#12 @rachelbaker
5 months ago

On Multisite, the admin_email input field is actually for an option named new_admin_email which gets set immediately when the form is saved. Only after the user clicks the link in their confirmation email, does the actual admin_email option get updated.
The REST API should use this same model to prevent users from accidentally changing their email address to something invalid.

Hmm.. @joehoyle any thoughts on how we should approach this?

#13 @rachelbaker
5 months ago

  • Owner changed from rachelbaker to joehoyle
  • Status changed from reopened to assigned

#14 @joehoyle
5 months ago

On Multisite, the admin_email input field is actually for an option named new_admin_email which gets set immediately when the form is saved. Only after the user clicks the link in their confirmation email, does the actual admin_email option get updated.

I'm not totally sure why this exists in multisite and not in single site?

On the assumption that we want to continue with that, we probably need to run a custom update hook by using rest_pre_update_setting which I guess if you try to update it on multisite it will noop the update, and instead trigger the confirmation email. How does that sound? I can put together a patch.

#15 @nacin
5 months ago

  • Keywords i18n-change removed

For the record - I don't see a string change here. [39406] appears to use the existing multisite and single site strings.

I'm not totally sure why this exists in multisite and not in single site?

This only existed in MU. This is probably one of those features that should *not* have been merged into core at all, but that ship sailed nearly seven (!?) years ago.

As for fixing this - I would suggest just pulling this setting for 4.7 and fixing it properly in 4.7.1 or 4.8.

#16 @nacin
5 months ago

FWIW, admin_email isn't in XML-RPC.

#17 follow-up: @helen
5 months ago

Leave for single site and remove for multisite?

#18 in reply to: ↑ 17 @nacin
5 months ago

Replying to helen:

Leave for single site and remove for multisite?

Sure.

@nacin
5 months ago

#19 @nacin
5 months ago

  • Keywords has-patch commit added; needs-patch removed

#20 @nacin
5 months ago

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

In 39470:

REST API: Register the admin_email setting in single site only.

fixes #38990.

#21 @nacin
5 months ago

In 39471:

REST API: Register the admin_email setting in single site only.

See [39470]. This time including unit test changes.

fixes #38990.

#22 @nacin
5 months ago

In 39472:

REST API: Register the admin_email setting in single site only.

Merges [39470] and [39471] to the 4.7 branch.

fixes #38990.

Note: See TracTickets for help on using tickets.