WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 6 months ago

#15691 new feature request

Network admin should have its own settings API

Reported by: joostdevalk Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch dev-feedback settings-api needs-unit-tests
Focuses: administration, multisite Cc:

Description

preferably using options.php and the same API as normal admin, this way making a plugin multisite compatible (ie. adding a Network admin screen to it) would be much easier.

Attachments (2)

15691.patch (7.7 KB) - added by boonebgorges 7 years ago.
15691.2.diff (22.8 KB) - added by flixos90 2 years ago.
new take on the issue based on original patch

Download all attachments as: .zip

Change History (44)

#1 @nacin
7 years ago

  • Milestone changed from Awaiting Review to Future Release

#2 @boonebgorges
7 years ago

  • Cc boonebgorges@… added

15691.patch takes the following tack:

  • Don't use options.php but instead have a wrapper settings_form_action() that returns options.php on Site Admin and edit.php?action=pluginsettings on Network Admin. Seems more consistent with the way that Network Admin works, which strikes me as a good thing. Downside is that plugins would have to be updated to be Network-compatible. But this is not a regression, so that seems OK to me.
  • When settings come from Network Admin, save them to site options automatically. This strikes me as appropriate for the majority of plugins. It does mean that plugin authors will need to be smart when they are calling up their options in order to prepopulate the settings fields - if they want plugins to work both networkwide and on single sites, they'll have to use $options = is_network_admin() ? get_site_option( 'my_plugin_options' ) : get_option( 'my_plugin_options' ); or something like that.

This second point seems problematic to me. Ideally, it would be nice if settings were registered from the very beginning (register_setting()) as site_options or not. But this would require a pretty significant rewrite of how register_settings(), and the option whitelists, currently work.

Feedback welcome.

@boonebgorges
7 years ago

#3 @mercime
6 years ago

  • Cc mercijavier@… added

#4 @scribu
6 years ago

  • Keywords has-patch added

if they want plugins to work both networkwide and on single sites, they'll have to use $options = is_network_admin() ? get_site_option( 'my_plugin_options' ) : get_option( 'my_plugin_options' );

If plugin authors want to have per-site options that overwrite network-wide options, they'll have to handle it themselves. It would be nice to have it in Core, but it's not a sticking point.

#5 @convissor
6 years ago

A separate API for network settings is unnecessary. My Login Security Solution plugin, http://wordpress.org/extend/plugins/login-security-solution/, successfully uses the same API for both regular and multisite network installs.

The main tweaks to make it fly are a few conditional statements for changing action and file names. Take a look at the plugin's source code (it's only two files) and search the text for the handful of is_multisite() calls.

In addition, this plugin is structured for easy reuse. Just rename the directory and plugin file, then change a few class constants and properties and it'll run as a whole new plugin.

#6 @boonebgorges
6 years ago

convissor - It looks like your solution works on Multisite by displaying the settings panel in Network Admin, but then saving blog options to blog #1. This won't work for plugins that need to have separate settings for each site on a network (those plugins can use the regular settings API and avoid the Network Admin altogether). It also won't work for plugins that want to store their settings in Network Admin - options.php doesn't know how to handle site_options. This latter point is the crux of the issue described in this ticket.

#7 @convissor
5 years ago

boonebgorges:

The patch moves a large chunk of code from wp-admin/options.php to a new function called process_settings() in wp-admin/includes/misc.php. The patch also modifies the code that's been moved. While both things are good ideas, doing them in one patch obscures what changes are being made inside that block of moved code.

I have pretty much no say in what happens in core, but may I suggest making two patches? One that moves the existing code into the function. Then another that makes the rest of the changes.

Question: this new Network Settings API will use the same API (function calls, HTML, etc) as the existing Settings API, right? So plugins will be able to create one settings screen that'll work under both regular and multisite installs. If so, a hearty +1 from me.

#8 @unknowndomain
5 years ago

  • Keywords settings-3.6 added

#9 @unknowndomain
5 years ago

  • Cc me@… added

#10 @SergeyBiryukov
5 years ago

  • Keywords settings-api added; settings-3.6 removed

#11 @unknowndomain
5 years ago

I would like to suggest that this be marked as related to #18285 so that when it gets revised a Network option is added with a couple options to define scope of the setting, blog or network.

#12 @toscho
5 years ago

  • Cc info@… added

#13 @sbrajesh
5 years ago

  • Cc brajesh@… added

#14 @DrewAPicture
5 years ago

  • Cc xoodrew@… added

#15 @Offereins
4 years ago

  • Cc lmoffereins@… added

#16 @nacin
4 years ago

  • Component changed from Network Admin to Plugins
  • Focuses administration added

#17 @nacin
4 years ago

  • Component changed from Plugins to Admin APIs

#18 @nacin
4 years ago

  • Component changed from Admin APIs to Plugins

Sorry for the noise.

#19 @tubiz
4 years ago

nacin: Please is there any update on this.
Is there a workaround that can be used temporarily till when this is added to core.

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


3 years ago

#21 @johnbillion
3 years ago

Retroactive dupe of #31274?

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


2 years ago

@flixos90
2 years ago

new take on the issue based on original patch

#23 @flixos90
2 years ago

  • Keywords dev-feedback added

As discussed on Slack, 15691.2.diff is a new take on this ticket, based on @boonebgorges original patch. Some notes on the implementation:

  • Settings are processed in a new process_settings() function which contains the code that generally overlaps between regular site vs network options. The function has a $context parameter which is set to site by default, but accepts network as well. This implementation is flexible to support possible settings-related features in the future, for example global options (that go for all networks) or user options.
  • The process_settings() function is called from wp-admin/options.php and wp-admin/network/settings.php respectively. I left the code parts in there which I considered to be too specific to go into process_settings(), but maybe we could even merge more of that into the function.
  • Lines 150-188 of wp-admin/options.php and lines 47-65 of wp-admin/network/settings.php contain actions that are specific to their respective type of options (see above bullet point) - these should probably placed elsewhere, but I left them in there for now for a better overview.
  • In wp-admin/network/settings.php, there is a global $whitelist_options array now, similar to wp-admin/options.php. I gave the one existing default settings page the slug general, and by default this is the only settings page in that array. A filter whitelist_network_options allows for new custom network settings and is used by register_setting() when called in network mode.
  • I also adjusted the related Settings API functions with the new $context parameter (for example register_setting() and settings_errors(), providing an easy way to distinguish between the type of setting to manage. Settings errors are handled on a network-wide level when needed.
  • I also allowed settings_fields() to be network-compatible and adjusted the related code of the network settings page to output the nonce and action hidden fields, so that the network settings check for these variables instead of simply checking whether $_POST contains something, aligning it with the regular API.
  • What I didn't include in this implementation are functions like add_settings_section() and do_settings_sections() which I think should be enhanced in the same manner as well, but they are not crucial to this first approach, so we could do that later.

The following code shows an example of how to use this implementation.

<?php
function nst_menu() {
        add_submenu_page( 'settings.php', __( 'Network Settings Test', 'nst' ), __( 'Network Settings Test', 'nst' ), 'manage_network_options', 'network-settings-test', 'nst_render_page' );
}
add_action( 'network_admin_menu', 'nst_menu' );

function nst_register_setting() {
        register_setting( 'network_settings_test', 'network_settings_test', 'nst_sanitize_settings', 'network' );
}
add_action( 'admin_init', 'nst_register_setting' ); // consider creating a new action 'network_admin_init' for things like that

function nst_sanitize_settings( $options ) {
        if ( false !== strpos( $options['test_value2'], '@example.com' ) ) {
                add_settings_error( 'network_settings_test', 'some_email_error', __( 'You must not provide emails @example.com.', 'nst' ), 'error' );
                $options['test_value2'] = '';
        }
        return $options;
}

function nst_render_page() {
        $options = get_site_option( 'network_settings_test', array() );
        ?>
        <div class="wrap">
                <h1><?php _e( 'Network Settings Test', 'nst' ); ?></h1>
                <form method="post" action="settings.php" novalidate="novalidate">
                        <?php settings_fields( 'network_settings_test', 'network' ); ?>
                        <table class="form-table">
                                <tr>
                                        <th scope="row"><label for="test_value1">Name</label></th>
                                        <td><input type="text" id="test_value1" name="network_settings_test[test_value1]" value="<?php echo isset( $options['test_value1'] ) ? $options['test_value1'] : ''; ?>" /></td>
                                </tr>
                                <tr>
                                        <th scope="row"><label for="test_value2">Email</label></th>
                                        <td><input type="email" id="test_value2" name="network_settings_test[test_value2]" value="<?php echo isset( $options['test_value2'] ) ? $options['test_value2'] : ''; ?>" /></td>
                                </tr>
                        </table>
                        <?php submit_button(); ?>
                </form>
        </div>
        <?php
}

I'm curious for feedback, if this is something we could build upon - making the regular Settings API network-compatible. And regarding this implementation, especially about possible issues with backwards compatibility. One point that I'm not sure about is whether in line 46 of wp-admin/network/settings.php we might need to continue support for just checking on $_POST.

#24 @sc0ttkclark
2 years ago

This proposed patch will work well with our Fields API implementation for Settings API, it should even make it easier to add support for network settings on our end.

#25 @flixos90
22 months ago

#31274 was marked as a duplicate.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


22 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


22 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


22 months ago

#29 @jeremyfelt
22 months ago

  • Milestone changed from Future Release to 4.6

Getting this on the radar for 4.6 to solicit feedback, etc...

#30 @flixos90
22 months ago

Related: #18088 and #35379 should probably be committed before continuing work on this ticket. The network settings API could use some functionality of what they would introduce.

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


21 months ago

#32 @chriscct7
21 months ago

  • Milestone changed from 4.6 to Future Release

This ticket was mentioned in Slack in #core-multisite by boone. View the logs.


19 months ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


17 months ago

#35 @afercia
16 months ago

  • Keywords changed from has-patch settings-api dev-feedback to has-patch settings-api dev-feedback settings-api

#36 @Mista-Flo
16 months ago

  • Keywords settings-api settings-api removed

#37 @Mista-Flo
16 months ago

  • Keywords settings-api added

This ticket was mentioned in Slack in #buddypress by jjj. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 months ago

#40 @flixos90
8 months ago

#40377 was marked as a duplicate.

#41 @spacedmonkey
6 months ago

  • Component changed from Plugins to Networks and Sites
  • Keywords has-unit-tests added
  • Version set to 3.0

#42 @spacedmonkey
6 months ago

  • Keywords needs-unit-tests added; has-unit-tests removed
Note: See TracTickets for help on using tickets.