Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#38990 closed defect (bug) (fixed)

REST API: admin_email setting description is inaccurate

Reported by: johnbillion's profile johnbillion Owned by: joehoyle's profile 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 8 years ago.
38990.2.diff (1.1 KB) - added by nacin 8 years ago.

Download all attachments as: .zip

Change History (24)

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


8 years ago

@ocean90
8 years ago

#2 @ocean90
8 years ago

  • Keywords has-patch added; needs-patch removed

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

#3 @ocean90
8 years ago

  • Keywords commit dev-feedback added

#4 @rachelbaker
8 years ago

  • Keywords dev-reviewed added; dev-feedback removed

#5 @rachelbaker
8 years ago

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

#6 @rachelbaker
8 years 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
8 years 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
8 years 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
8 years ago

  • Focuses multisite added

#10 @netweb
8 years 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.


8 years ago

#12 @rachelbaker
8 years 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
8 years ago

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

#14 @joehoyle
8 years 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
8 years 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
8 years ago

FWIW, admin_email isn't in XML-RPC.

#17 follow-up: @helen
8 years ago

Leave for single site and remove for multisite?

#18 in reply to: ↑ 17 @nacin
8 years ago

Replying to helen:

Leave for single site and remove for multisite?

Sure.

@nacin
8 years ago

#19 @nacin
8 years ago

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

#20 @nacin
8 years 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
8 years 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
8 years 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.