Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add multisite support #102

Merged
merged 18 commits into from
Nov 4, 2024
Merged

add multisite support #102

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c6fe174
add multisite support
afragen Nov 2, 2024
13f43b6
updated to use site options
afragen Nov 2, 2024
a584690
use network_admin_url
afragen Nov 2, 2024
13e6731
update for multisite saving settings
afragen Nov 2, 2024
904596a
need to check empty not isset for multisite
afragen Nov 2, 2024
f214c40
load scripts on multisite
afragen Nov 2, 2024
e55f7b6
add early return, simplify redirect
afragen Nov 3, 2024
47a8b21
action should redirect to settings page
afragen Nov 3, 2024
2342f72
remove remaining code from using variable
afragen Nov 3, 2024
2f964da
no need to use variable for single instance
afragen Nov 3, 2024
ae88d99
add @return
afragen Nov 3, 2024
6248b74
simplified sanitization of some checkboxes, thanks @costdev
afragen Nov 3, 2024
f6a7019
no need for ternary
afragen Nov 3, 2024
8841bf9
Merge branch 'main' into multisite
afragen Nov 4, 2024
2851b3e
new line before return
afragen Nov 4, 2024
d154153
Merge branch 'main' into multisite
afragen Nov 4, 2024
429ede3
not necessary to add text here as default text is `Save Changes`
afragen Nov 4, 2024
69786dc
add back i18n
afragen Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions aspire-update.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* License URI: https://www.gnu.org/licenses/gpl-3.0.txt
* Text Domain: AspireUpdate
* Domain Path: /languages
* Network: true
*/

if ( ! defined( 'ABSPATH' ) ) {
Expand Down
85 changes: 58 additions & 27 deletions includes/class-admin-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,14 @@ class Admin_Settings {
* The Constructor.
*/
public function __construct() {
add_action( 'admin_menu', array( $this, 'register_admin_menu' ) );
add_action( is_multisite() ? 'network_admin_menu' : 'admin_menu', array( $this, 'register_admin_menu' ) );
add_action( 'admin_init', array( $this, 'reset_settings' ) );
add_action( 'admin_init', array( $this, 'register_settings' ) );
add_action( 'admin_notices', array( $this, 'reset_admin_notice' ) );
add_action( 'admin_enqueue_scripts', array( $this, 'admin_enqueue_scripts' ) );

add_action( 'admin_init', array( $this, 'update_settings' ) );
add_action( 'network_admin_edit_aspireupdate-settings', array( $this, 'update_settings' ) );
Comment on lines +53 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-checking if this is necessary for this issue or if this fixes a different issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary for saving from multisite and provides a consistent manner of saving for both single and multisite.

}

/**
Expand Down Expand Up @@ -87,16 +90,16 @@ public function reset_settings() {
wp_verify_nonce( sanitize_key( $_GET['reset-nonce'] ), 'aspireupdate-reset-nonce' )
) {
$options = $this->get_default_settings();
update_option( $this->option_name, $options );
update_option( 'aspireupdate-reset', 'true' );
update_site_option( $this->option_name, $options );
update_site_option( 'aspireupdate-reset', 'true' );

wp_safe_redirect(
add_query_arg(
array(
'reset-success' => 'success',
'reset-success-nonce' => wp_create_nonce( 'aspireupdate-reset-success-nonce' ),
),
admin_url( 'index.php?page=aspireupdate-settings' )
network_admin_url( 'index.php?page=aspireupdate-settings' )
)
);
exit;
Expand All @@ -110,14 +113,14 @@ public function reset_settings() {
*/
public function reset_admin_notice() {
if (
( 'true' === get_option( 'aspireupdate-reset' ) ) &&
( 'true' === get_site_option( 'aspireupdate-reset' ) ) &&
isset( $_GET['reset-success'] ) &&
( 'success' === $_GET['reset-success'] ) &&
isset( $_GET['reset-success-nonce'] ) &&
wp_verify_nonce( sanitize_key( $_GET['reset-success-nonce'] ), 'aspireupdate-reset-success-nonce' )
) {
echo '<div class="notice notice-success is-dismissible"><p>Settings have been reset to default.</p></div>';
delete_option( 'aspireupdate-reset' );
delete_site_option( 'aspireupdate-reset' );
}
}

Expand All @@ -131,13 +134,13 @@ public function reset_admin_notice() {
*/
public function get_setting( $setting_name, $default_value = false ) {
if ( null === $this->options ) {
$options = get_option( $this->option_name, false );
$options = get_site_option( $this->option_name, false );
/**
* If the options are not set load defaults.
*/
if ( false === $options ) {
$options = $this->get_default_settings();
update_option( $this->option_name, $options );
update_site_option( $this->option_name, $options );
}
$config_file_options = $this->get_settings_from_config_file();
if ( is_array( $options ) ) {
Expand Down Expand Up @@ -210,12 +213,37 @@ private function get_settings_from_config_file() {
return $options;
}

/**
* Update settings for single site or network activated.
*
* @link http://wordpress.stackexchange.com/questions/64968/settings-api-in-multisite-missing-update-message
* @link http://benohead.com/wordpress-network-wide-plugin-settings/
afragen marked this conversation as resolved.
Show resolved Hide resolved
*/
public function update_settings() {
// Exit if improper privileges.
if ( ! isset( $_POST['_wpnonce'] ) || ! wp_verify_nonce( sanitize_key( wp_unslash( $_POST['_wpnonce'] ) ), 'aspireupdate-settings' ) ) {
return;
}

// Save settings and redirect.
if ( ( isset( $_POST['option_page'] ) && 'aspireupdate_settings' === $_POST['option_page'] ) ) {
update_site_option( $this->option_name, $this->sanitize_settings( wp_unslash( $_POST['aspireupdate_settings'] ) ) );

wp_safe_redirect(
add_query_arg( array( network_admin_url( 'index.php?page=aspireupdate-settings' ) ) )
);
exit;
}
}

/**
* Register the Admin Menu.
*
* @return void
*/
public function register_admin_menu() {
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';

if ( ! defined( 'AP_REMOVE_UI' ) ) {
define( 'AP_REMOVE_UI', false );
}
Expand All @@ -224,7 +252,7 @@ public function register_admin_menu() {
'index.php',
'AspireUpdate',
'AspireUpdate',
'manage_options',
$capability,
'aspireupdate-settings',
array( $this, 'the_settings_page' )
);
Expand All @@ -238,7 +266,7 @@ public function register_admin_menu() {
* @return void
*/
public function admin_enqueue_scripts( $hook ) {
if ( 'dashboard_page_aspireupdate-settings' !== $hook ) {
if ( ! in_array( $hook, array( 'dashboard_page_aspireupdate-settings','index_page_aspireupdate-settings' ) ) ) {
return;
}
wp_enqueue_style( 'aspire_update_settings_css', plugin_dir_url( __DIR__ ) . 'assets/css/aspire-update.css', array(), AP_VERSION );
Expand All @@ -247,7 +275,7 @@ public function admin_enqueue_scripts( $hook ) {
'aspire_update_settings_js',
'aspireupdate',
array(
'ajax_url' => admin_url( 'admin-ajax.php' ),
'ajax_url' => network_admin_url( 'admin-ajax.php' ),
'nonce' => wp_create_nonce( 'aspireupdate-ajax' ),
'domain' => Utilities::get_top_level_domain(),
)
Expand All @@ -260,23 +288,26 @@ public function admin_enqueue_scripts( $hook ) {
* @return void
*/
public function the_settings_page() {
$action = is_multisite() ? 'index.php?page=aspireupdate-settings' : 'options.php';
$reset_url = add_query_arg(
array(
'reset' => 'reset',
'reset-nonce' => wp_create_nonce( 'aspireupdate-reset-nonce' ),
),
admin_url( 'index.php?page=aspireupdate-settings' )
network_admin_url( 'index.php?page=aspireupdate-settings' )
);
?>
<div class="wrap">
<h1><?php esc_html_e( 'AspireUpdate Settings', 'AspireUpdate' ); ?></h1>
<form id="aspireupdate-settings-form" method="post" action="options.php">
<form id="aspireupdate-settings-form" method="post" action="<?php echo esc_attr( $action ); ?>">
<?php
settings_fields( $this->option_group );
do_settings_sections( 'aspireupdate-settings' );
?>
<p class="submit">
<input type="submit" name="submit" id="submit" class="button button-primary" value="Save Changes">
<?php wp_nonce_field( 'aspireupdate-settings' ); ?>
<?php submit_button( esc_html__( 'Save Changes', 'aspireupdate-settings' ), 'primary', 'submit', false ); ?>

<a href="<?php echo esc_url( $reset_url ); ?>" class="button button-secondary" >Reset</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same clarification request about if this is necessary for this issue or if it's aprt of another?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary for sending a nonce with the submit button. I did change the link to use the submit_button() though it may not be technically necessary to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, thanks!

</p>
</form>
Expand All @@ -291,13 +322,13 @@ public function the_settings_page() {
*/
public function register_settings() {
$nonce = wp_create_nonce( 'aspireupdate-settings' );
$options = get_option( $this->option_name, false );
$options = get_site_option( $this->option_name, false );
/**
* If the options are not set load defaults.
*/
if ( false === $options ) {
$options = $this->get_default_settings();
update_option( $this->option_name, $options );
update_site_option( $this->option_name, $options );
}

register_setting(
Expand Down Expand Up @@ -502,10 +533,10 @@ public function add_settings_field_callback( $args = array() ) {
<?php
foreach ( $group_options as $group_option ) {
?>
<option
data-api-key-url="<?php echo esc_html( $group_option['api-key-url'] ?? '' ); ?>"
data-require-api-key="<?php echo esc_html( $group_option['require-api-key'] ?? 'false' ); ?>"
value="<?php echo esc_attr( $group_option['value'] ?? '' ); ?>"
<option
data-api-key-url="<?php echo esc_html( $group_option['api-key-url'] ?? '' ); ?>"
data-require-api-key="<?php echo esc_html( $group_option['require-api-key'] ?? 'false' ); ?>"
value="<?php echo esc_attr( $group_option['value'] ?? '' ); ?>"
Comment on lines +535 to +538
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 What change am I not seeing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VSCode removed the extra space at the end of some of the lines automatically.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly 😂

<?php selected( esc_attr( $group_option['value'] ?? '' ), esc_attr( $options[ $id ] ?? '' ) ); ?>
>
<?php echo esc_html( $group_option['label'] ?? '' ); ?>
Expand All @@ -516,10 +547,10 @@ public function add_settings_field_callback( $args = array() ) {
</select>
<p>
<input
type="text"
id="aspireupdate-settings-field-<?php echo esc_attr( $id ); ?>_other"
name="<?php echo esc_attr( $this->option_name ); ?>[<?php echo esc_attr( $id ); ?>_other]"
value="<?php echo esc_attr( $options[ $id . '_other' ] ?? '' ); ?>"
type="text"
id="aspireupdate-settings-field-<?php echo esc_attr( $id ); ?>_other"
name="<?php echo esc_attr( $this->option_name ); ?>[<?php echo esc_attr( $id ); ?>_other]"
value="<?php echo esc_attr( $options[ $id . '_other' ] ?? '' ); ?>"
Comment on lines +549 to +552
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 What change am I not seeing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, my VSCode settings removed the extra space at the end of the line automatically.

class="regular-text"
/>
</p>
Expand All @@ -539,18 +570,18 @@ class="regular-text"
public function sanitize_settings( $input ) {
$sanitized_input = array();

$sanitized_input['enable'] = ( isset( $input['enable'] ) && $input['enable'] ) ? 1 : 0;
$sanitized_input['enable'] = ( ! empty( $input['enable'] ) && $input['enable'] ) ? 1 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change here for enable and later for enable_debug, and is this needed for multisite support?

Copy link
Contributor

@costdev costdev Nov 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the second condition shouldn't be necessary here if using ! empty()

What about just (int) ! empty( $input['enable'] )?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values are unset by default. With multisite the sanitize function runs twice, I assume once from the register_setting() callback. The second time this runs the input is 0 and therefore it is set, but it is also empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me test that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and working.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further simplified, doesn't need ternary.

$sanitized_input['api_key'] = sanitize_text_field( $input['api_key'] ?? '' );
$sanitized_input['api_host'] = sanitize_text_field( $input['api_host'] ?? '' );
$sanitized_input['api_host_other'] = sanitize_text_field( $input['api_host_other'] ?? '' );

$sanitized_input['enable_debug'] = isset( $input['enable_debug'] ) ? 1 : 0;
$sanitized_input['enable_debug'] = ! empty( $input['enable_debug'] ) ? 1 : 0;
if ( isset( $input['enable_debug_type'] ) && is_array( $input['enable_debug_type'] ) ) {
$sanitized_input['enable_debug_type'] = array_map( 'sanitize_text_field', $input['enable_debug_type'] );
} else {
$sanitized_input['enable_debug_type'] = array();
}
$sanitized_input['disable_ssl_verification'] = ( isset( $input['disable_ssl_verification'] ) && $input['disable_ssl_verification'] ) ? 1 : 0;
$sanitized_input['disable_ssl_verification'] = ( ! empty( $input['disable_ssl_verification'] ) && $input['disable_ssl_verification'] ) ? 1 : 0;
return $sanitized_input;
}
}