add multisite support #102

afragen · 2024-11-02T16:20:46Z

Pull Request

What changed?

Added multisite support.

Why did it change?

Fixes #59

Did you fix any specific issues?

Multisite support.
Update to use site options.
Apparently there were also some spaces at the end of code lines that VSCode removed.

CERTIFICATION

By opening this pull request, I do agree to abide by
the CODE OF CONDUCT and be bound by the terms
of the Contribution Guidelines in effect on the date and time
of my contribution as proven by the
revision information in GitHub. I also agree that any previous contributions shall be deemed subject to the terms of the
version in effect on the date and time of this pull request, or any future revisions for pull requests I may submit.
Further, I certify that this work is my own, is original, does not violate the intellectual property of any other person
or entity, and I am not violating any license agreements or contracts I have with any person or entity. Finally, I agree
that this code may be licensed under any license deemed appropraite by AspirePress, including but not
limited to open source, closed source, proprietary or custom licenses, and that such license terms neither violate my
rights or my copyright to this code.

afragen · 2024-11-02T16:26:12Z

Network Admin

Single Site Admin on Multisite

namithj · 2024-11-02T16:28:12Z

Have you tested this in MU. This shouldn't be enough for MU support. We need an admin screen for Network Admin., take away the setting from normal admins as MU core and plugins needs to be updated as a whole and can't be updated individually. This needs a lot of thought before we embark on the path.

namithj · 2024-11-02T16:30:15Z

Also the settings should use network options instead of the regular options if we do this.

namithj · 2024-11-02T16:36:45Z

The URLs for reset button and the redirect url after the reset will need to be updated

namithj · 2024-11-02T16:39:41Z

While we are at it, let’s make that URL come from a new function now that there can be different values for it so that it can be unit tested.

afragen · 2024-11-02T17:28:39Z

Currently working on saving settings.

afragen · 2024-11-02T21:09:11Z

If I comment out the following saving settings works for both single and multisite.

add_action( 'admin_enqueue_scripts', array( $this, 'admin_enqueue_scripts' ) );

If not commented out there doesn't seem to be any action from the Save Changes button. I tested this in a new installation with a clean copy of the plugin and no other plugins.

See #103, #43

afragen · 2024-11-03T17:41:57Z

Aside from the issues #43 and #103, this PR should be ready for review.

FWIW, I have similar code running in many plugins for multisite compatibility, including the WordPress Beta Tester plugin.

settings page is same for both multisite/single site

costdev

Thanks Andy!

Haven't tested yet so just some minor things and a few clarification requests.

includes/class-admin-settings.php

costdev · 2024-11-03T18:08:20Z

includes/class-admin-settings.php

+		add_action( 'admin_init', array( $this, 'update_settings' ) );
+		add_action( 'network_admin_edit_aspireupdate-settings', array( $this, 'update_settings' ) );


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

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

costdev · 2024-11-03T18:15:00Z

includes/class-admin-settings.php

+					<?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>


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

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.

Works for me, thanks!

costdev · 2024-11-03T18:16:29Z

includes/class-admin-settings.php

+							<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'] ?? '' ); ?>"


🤔 What change am I not seeing?

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

Exactly 😂

costdev · 2024-11-03T18:16:37Z

includes/class-admin-settings.php

+						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' ] ?? '' ); ?>"


🤔 What change am I not seeing?

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

costdev · 2024-11-03T18:17:33Z

includes/class-admin-settings.php

@@ -539,18 +569,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;


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

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

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

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.

Let me test that.

Tested and working.

Further simplified, doesn't need ternary.

afragen · 2024-11-03T18:46:19Z

I have tested this in both single and multisite. It's working for me. Only caveat is the first setting to enable API rewrites must be set in order for any other settings to be set. This is unrelated to multisite.

namithj · 2024-11-04T12:44:11Z

@costdev Is this good to go?

costdev · 2024-11-04T12:47:27Z

I haven't had a chance to test this on multisite, but since @afragen has tested it, I'm pretty confident that it'll work as expected.

Nevertheless, I can follow up to test it, possibly later today.

namithj · 2024-11-04T12:49:48Z

Once you test, approve it in and let the testers know in slack so that they can test this one. Its a tricky one to test.

afragen · 2024-11-04T14:59:01Z

Be sure to test it in a single site too.

costdev · 2024-11-04T16:03:54Z

@afragen Be sure to test it in a single site too.

Make me!

Ok, I will 😂

costdev · 2024-11-04T16:37:29Z

Test Report

Environment

WordPress: 6.6.2
PHP: 7.4.33
Server: Apache/2.4.57 (Ubuntu)
Database: mysqli (Server: 8.0.35-0ubuntu0.20.04.1 / Client: mysqlnd 7.4.33)
Browser: Chrome 129.0.0.0
OS: Windows 10/11
Theme: Twenty Twenty-Four 1.2
MU Plugins: None activated
Plugins:
- AspireUpdate 0.5
- Test Reports 1.2.0

Actual Results

Single Site

✅ Settings appear under Dashboard > AspireUpdate.
✅ Settings can be saved and are retained upon page refresh.
✅ Settings are reset and are still reset upon page refresh.
✅ Plugins are installed from the dotorg API when rewriting is disabled.
✅ Plugins are installed from the AP API when rewriting is enabled.
✅ Themes are installed from the dotorg API when rewriting is disabled.
✅ Themes are installed from the AP API when rewriting is enabled.

Multisite

✅ Settings appear under Dashboard > AspireUpdate for Network Admin.
✅ Settings do not appear under Dashboard > AspireUpdate when viewing a single site's dashboard.
✅ Settings can be saved and are retained upon page refresh.
✅ Settings are reset and are still reset upon page refresh.
✅ Plugins are installed from the dotorg API when rewriting is disabled.
✅ Plugins are installed from the AP API when rewriting is enabled.
✅ Themes are installed from the dotorg API when rewriting is disabled.
✅ Themes are installed from the AP API when rewriting is enabled.

costdev

Thanks @afragen!

As expected, everything I tested works in Single Site and Multisite. ❤️

costdev · 2024-11-04T17:43:52Z

@asirota This one should be good to go in now.

afragen · 2024-11-04T18:39:43Z

includes/class-admin-settings.php

 			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>' . esc_html__( 'Settings have been reset to default.', 'AspireUpdate' ) . '</p></div>';


I don't know how the i18n and escaping got removed. Sorry.

I assume this made it into the PR?

It didn't but I made another PR for it.

Fixed typo in code deleted when addressing #102

* Prevent Recursion on certain Host name patterns (#69) --------- Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * Playground ready updating into main (#71) * Update blueprint.json Switching to default theme of 2022 to support minimum WP Playground support for WordPress version as 5.9.9 Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * Switching blueprint.json to 2022 theme Signed-off-by: Alex Sirota <alex@newpathconsulting.com> --------- Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * `Add New Plugin`: Remove 'Featured' and 'Favorites' tabs when API rewrite is enabled. (#80) * Add class for overriding plugins screens. * Remove unused tabs when API rewrite is enabled. * `Add New Theme`: Remove 'Favorites' filter tab when API rewrite is enabled. (#82) * Add class for overriding themes screens. * Hide unsupported filters. * Redirect unsupported filters to `theme-install.php`. --------- Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Co-authored-by: Alex Sirota <alex@newpathconsulting.com> * Add PHPUnit. (#87) * Add PHPUnit scaffolding. * Add Composer files for dependencies and test script. * Add `.gitignore`. * Add sample test for `Admin_Settings::get_setting()`. * Add PHP 8.3 to the test matrix. * Add `.cache/*` to `.gitignore`. * Limit testing to PHP 7.4 (minimum) and 8.3 (latest). * `Plugins Screens`: Initialize in `Controller::__construct()`. (#89) * `Workflows`: Fix PHP version and add more workflow triggers. (#92) * Fix PHP version. * Add `push` and `workflow_dispatch` workflow triggers. * `Tests`: Ensure options are not set when tests run. (#95) * Introduce a test constant. * Don't initialize when the tests are running. * Use a new object for testing `Admin_Settings::get_setting()`. * Warning when debug-aspire-update.log doesnt exist (#109) * `Tests`: Add default coverage settings. (#97) * Improve I18N Issues Based on version 0.5 (#104) * Fixes Settings not getting saved when rewrites were turned off (#108) * Add French Translation (#105) * I18n fr fr (#113) * Translations loading fix112 (#114) * Add French translations. * load any available translations with code in the plugin * #110 (#116) Fixes #110 * Add nonce verification and sanitize `$_REQUEST/$_GET` values. (#115) Signed-off-by: Colin Stewart <79332690+costdev@users.noreply.github.com> * Added German Translation (#111) * added German translation * added gitignore for .DS_Store Signed-off-by: Harikrishnan R <harikrishnanr994@gmail.com> * Voltron has landed. (#117) The Voltron has landed. See if you can find where. * `Admin Settings`: Delete all settings when the plugin is uninstalled. (#118) * `Admin Settings`: Add multisite-safe deletion of all settings. * Delete all settings when the plugin is uninstalled. * add multisite support (#102) * accidentally deleted (#124) Fixed typo in code deleted when addressing #102 * Add `.editorconfig`. (#123) * create zip with each new tagged release (#121) * create zip with each new tagged release * update .gitattributes --------- Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Signed-off-by: Colin Stewart <79332690+costdev@users.noreply.github.com> Signed-off-by: Harikrishnan R <harikrishnanr994@gmail.com> Co-authored-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com> Co-authored-by: Alex Lion <learnwithalex@gmail.com> Co-authored-by: Sébastien SERRE <sebastien@thivinfo.com> Co-authored-by: Harikrishnan R <harikrishnanr994@gmail.com> Co-authored-by: Andy Fragen <andy@thefragens.com>

* Prevent Recursion on certain Host name patterns (#69) --------- Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * Playground ready updating into main (#71) * Update blueprint.json Switching to default theme of 2022 to support minimum WP Playground support for WordPress version as 5.9.9 Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * Switching blueprint.json to 2022 theme Signed-off-by: Alex Sirota <alex@newpathconsulting.com> --------- Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> * `Add New Plugin`: Remove 'Featured' and 'Favorites' tabs when API rewrite is enabled. (#80) * Add class for overriding plugins screens. * Remove unused tabs when API rewrite is enabled. * `Add New Theme`: Remove 'Favorites' filter tab when API rewrite is enabled. (#82) * Add class for overriding themes screens. * Hide unsupported filters. * Redirect unsupported filters to `theme-install.php`. --------- Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Co-authored-by: Alex Sirota <alex@newpathconsulting.com> * Add PHPUnit. (#87) * Add PHPUnit scaffolding. * Add Composer files for dependencies and test script. * Add `.gitignore`. * Add sample test for `Admin_Settings::get_setting()`. * Add PHP 8.3 to the test matrix. * Add `.cache/*` to `.gitignore`. * Limit testing to PHP 7.4 (minimum) and 8.3 (latest). * `Plugins Screens`: Initialize in `Controller::__construct()`. (#89) * `Workflows`: Fix PHP version and add more workflow triggers. (#92) * Fix PHP version. * Add `push` and `workflow_dispatch` workflow triggers. * `Tests`: Ensure options are not set when tests run. (#95) * Introduce a test constant. * Don't initialize when the tests are running. * Use a new object for testing `Admin_Settings::get_setting()`. * Warning when debug-aspire-update.log doesnt exist (#109) * `Tests`: Add default coverage settings. (#97) * Improve I18N Issues Based on version 0.5 (#104) * Fixes Settings not getting saved when rewrites were turned off (#108) * Add French Translation (#105) * I18n fr fr (#113) * Translations loading fix112 (#114) * Add French translations. * load any available translations with code in the plugin * #110 (#116) Fixes #110 * Add nonce verification and sanitize `$_REQUEST/$_GET` values. (#115) Signed-off-by: Colin Stewart <79332690+costdev@users.noreply.github.com> * Added German Translation (#111) * added German translation * added gitignore for .DS_Store Signed-off-by: Harikrishnan R <harikrishnanr994@gmail.com> * Voltron has landed. (#117) The Voltron has landed. See if you can find where. * `Admin Settings`: Delete all settings when the plugin is uninstalled. (#118) * `Admin Settings`: Add multisite-safe deletion of all settings. * Delete all settings when the plugin is uninstalled. * add multisite support (#102) * accidentally deleted (#124) Fixed typo in code deleted when addressing #102 * Add `.editorconfig`. (#123) * create zip with each new tagged release (#121) * create zip with each new tagged release * update .gitattributes * Cleanup after `.editorconfig` was added to the repository. (#126) * Add coding standard. (#127) * Add Coding Standard. * Add PHPCS cache directory. * Coding Standards: Apply PHPCBF to codebase. * Coding Standards: Add GitHub workflow. --------- Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Co-authored-by: Alex Sirota <alex@newpathconsulting.com> --------- Signed-off-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> Signed-off-by: Alex Sirota <alex@newpathconsulting.com> Signed-off-by: Colin Stewart <79332690+costdev@users.noreply.github.com> Signed-off-by: Harikrishnan R <harikrishnanr994@gmail.com> Co-authored-by: Namith Jawahar <48271037+namithj@users.noreply.github.com> Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com> Co-authored-by: Alex Lion <learnwithalex@gmail.com> Co-authored-by: Sébastien SERRE <sebastien@thivinfo.com> Co-authored-by: Harikrishnan R <harikrishnanr994@gmail.com> Co-authored-by: Andy Fragen <andy@thefragens.com>

add multisite support

c6fe174

afragen marked this pull request as ready for review November 2, 2024 16:21

updated to use site options

13f43b6

afragen marked this pull request as draft November 2, 2024 16:34

afragen added 4 commits November 2, 2024 13:00

use network_admin_url

a584690

update for multisite saving settings

13e6731

need to check empty not isset for multisite

904596a

load scripts on multisite

f214c40

afragen marked this pull request as ready for review November 3, 2024 17:42

afragen added 3 commits November 3, 2024 10:05

add early return, simplify redirect

e55f7b6

action should redirect to settings page

47a8b21

settings page is same for both multisite/single site

remove remaining code from using variable

2342f72

costdev reviewed Nov 3, 2024

View reviewed changes

afragen added 3 commits November 3, 2024 10:19

no need to use variable for single instance

2f964da

add @return

ae88d99

simplified sanitization of some checkboxes, thanks @costdev

6248b74

afragen added 3 commits November 3, 2024 13:06

no need for ternary

f6a7019

Merge branch 'main' into multisite

8841bf9

new line before return

2851b3e

asirota requested a review from namithj November 4, 2024 08:26

costdev approved these changes Nov 4, 2024

View reviewed changes

namithj approved these changes Nov 4, 2024

View reviewed changes

afragen added 3 commits November 4, 2024 09:10

Merge branch 'main' into multisite

d154153

not necessary to add text here as default text is Save Changes

429ede3

add back i18n

69786dc

costdev approved these changes Nov 4, 2024

View reviewed changes

asirota assigned afragen Nov 4, 2024

asirota added the enhancement New feature or request label Nov 4, 2024

asirota added this to the Phase 1 milestone Nov 4, 2024

asirota merged commit 46cd8f1 into aspirepress:main Nov 4, 2024
2 checks passed

afragen commented Nov 4, 2024

View reviewed changes

afragen mentioned this pull request Nov 4, 2024

oops, accidentally deleted #124

Merged

asirota pushed a commit that referenced this pull request Nov 4, 2024

accidentally deleted (#124)

0951af0

Fixed typo in code deleted when addressing #102

afragen deleted the multisite branch November 6, 2024 19:45

		add_action( 'admin_init', array( $this, 'update_settings' ) );
		add_action( 'network_admin_edit_aspireupdate-settings', array( $this, 'update_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>

add multisite support #102

add multisite support #102

Conversation

afragen commented Nov 2, 2024 • edited Loading

Pull Request

What changed?

Why did it change?

Did you fix any specific issues?

CERTIFICATION

afragen commented Nov 2, 2024

namithj commented Nov 2, 2024

namithj commented Nov 2, 2024

namithj commented Nov 2, 2024

namithj commented Nov 2, 2024

afragen commented Nov 2, 2024

afragen commented Nov 2, 2024 • edited Loading

afragen commented Nov 3, 2024

costdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

costdev Nov 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afragen commented Nov 3, 2024

namithj commented Nov 4, 2024

costdev commented Nov 4, 2024

namithj commented Nov 4, 2024

afragen commented Nov 4, 2024

costdev commented Nov 4, 2024

costdev commented Nov 4, 2024

Test Report

Environment

Actual Results

costdev left a comment

Choose a reason for hiding this comment

costdev commented Nov 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afragen commented Nov 2, 2024 •

edited

Loading

afragen commented Nov 2, 2024 •

edited

Loading

costdev Nov 3, 2024 •

edited

Loading