WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#26929 closed enhancement (maybelater)

add_site_option signature should match add_option signature

Reported by: ChrisWiegman Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.8
Component: Options, Meta APIs Keywords:
Focuses: multisite Cc:

Description

On plugins that are multisite compatible it is much safer to save options using add_site_option which falls back to add_option if not multisite. The catch is using this method the autoload cannot be set even for the non-multisite compatible.

Attached is a simple patch the passes the autoload through from add_site_option to add_option if present without affecting any multisite functionality.

Attachments (1)

patch.diff (1.2 KB) - added by ChrisWiegman 6 years ago.
patch to pass autoload parameter to add_option when add_site_option called

Download all attachments as: .zip

Change History (5)

@ChrisWiegman
6 years ago

patch to pass autoload parameter to add_option when add_site_option called

#1 @jeremyfelt
6 years ago

  • Focuses multisite added

Hi @ChrisWiegman, thanks for the patch.

add_site_option stores data in the $wpdb->sitemeta table if the installation is multisite, and stores data via add_option() in $wpdb->options if not.

A hard part of adding an autoload flag to add_site_option() would be its usefulness only for single site installations. The extra parameter may cause confusion for developers working on a primarily multisite function.

I wonder if $wpdb->options is the right place for an option requiring autoload anyway.

If we did add this parameter, I think the current patch's inclusion of add_option()'s deprecated parameter in add_site_option() is unnecessary.

Related #22846

#2 @ChrisWiegman
6 years ago

@jeremyfelt

You're right, this is useful in only a very small subset of uses and only really makes the is_multisite call non-consequential. As add_site_option is however listed as it will fallback to add_option however this is an inconsistency that just didn't quite make sense to me.

As for the deprecated parameter, my only reason for incorporating it was due solely to consistency. Now anyone trying to turn off autoload can maintain the capability and be multi-site compatible (if needed) by adding "site_" to the function call.

Long term, particularly after my current project a better fix would seem to be to add the ability to add_site_option for site_meta as well but, even in my own case, the use for such a change is so limited that I haven't tackled it yet. For now, as 90%+ of sites my own project goes in are not multi-site this would be quite a benefit without the cost of the extra multi-site call.

#3 @ChrisWiegman
6 years ago

  • Resolution set to maybelater
  • Severity changed from normal to minor
  • Status changed from new to closed

After discussing this with Nacin I'm going to work on adding the option to override autoload in Multi-site (currently the sitemeta table is not autoloaded and doesn't support the option).

#4 @ocean90
6 years ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.