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.

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 Autoptimize compatibility with Multisite #254

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@juanfran-granados
Copy link

commented May 17, 2019

Hello,

This is Juanfran Granados from Mirai, an hotelier agency. This month we were evaluating your terrific plugin to handle static resources optimization, and we found out that your plugin does not have a global configuration for WordPress Multisite:

https://wordpress.org/support/topic/globally-configure-for-multisite/

Yes, you can copy the configuration to each site, but that solution does not cover a change in configuration in the future.

I addressed this problem modifying your code to use a wrapper for get_option and update_option. Inside of that wrapper, I took care of the standalone and multisite cases. I tought it is a cleaner option than adding if/else clauses in each get_option and update_option of your code. Feel free to reformulate it.

Also, I've filtered the update_option logic using pre_update_option hook to check if it's a multisite instance. I was forced to do that because you are using the Settings API to save your admin form and I didn't find a way to create a clean separate logic for Multisite.

Thanks!

Juanfran Granados added some commits May 16, 2019

Juanfran Granados
Use a wrapper for getting and setting options.
In my opinion this is better than adding an if/else clause for all the 
get and update options.
Juanfran Granados
Add a filter to option to check if it's autoptimize.
Because the plugin uses register_setting for the form fields, I haven't 
found a another hook to check if it's a multisite instance and save the 
option in the network.

@juanfran-granados juanfran-granados marked this pull request as ready for review May 17, 2019

@futtta

This comment has been minimized.

Copy link
Owner

commented May 17, 2019

Wow, that would be a great addition @juanfran-granados , thanks for the work you did on this!

The mulitisite-tests on Travis-ci seem to fail with this error however;

Fatal error: Uncaught Error: Call to undefined function is_plugin_active_for_network() in /home/travis/build/futtta/autoptimize/classes/autoptimizeOption.php on line 32

Problem might be similar to what I encountered/ fixed earlier for is_plugin_active?

@juanfran-granados

This comment has been minimized.

Copy link
Author

commented May 17, 2019

Ooops! Ok, let's see if including the plugin.php file works...

@juanfran-granados

This comment has been minimized.

Copy link
Author

commented May 17, 2019

Done :) Thanks.

@futtta

This comment has been minimized.

Copy link
Owner

commented May 17, 2019

@juanfran-granados

This comment has been minimized.

Copy link
Author

commented May 20, 2019

Hello Frank,

Great news! Of course, test it yourself and ask me anything you need. Regarding your questions:

  • Based on my reading of the code, if AO is network activated then the AO
    settings won't be shown on subsites any more, only on the main one?

Yes, that's correct. Also the get_option fetches the configuration from the network settings.

  • What would happen if this got installed on an existing multisite install
    where subsites are already configured?

If you have subsites configured and the plugin is not active network wide, it will get the configuration from the subsite. But if you activate the plugin in the whole network, then the network settings will be the ones used.

@futtta

This comment has been minimized.

Copy link
Owner

commented May 20, 2019

@juanfran-granados

This comment has been minimized.

Copy link
Author

commented May 24, 2019

So how would you feel about adding an option on the network-wide page which tells AO to show and use the network-wide or the child-sites config?

Nice idea! I will try to get that done next week.

And while we're at it; adding a simple "ask your network admin"-page to child-site admins if network-wide config is uses? :-)

In my opinion, I would prefer to remove the admin menu in child-sites if network-wide configuration is in use. But if you are 100% sure of it, I could maintain the child-site admin menu, outputting "ask your network admin". It's better if you add the HTML+CSS layer on top of that, since I don't know what you have in mind.

@futtta

This comment has been minimized.

Copy link
Owner

commented May 27, 2019

great juanfran, looking forward to that! :-)

@futtta

This comment has been minimized.

Copy link
Owner

commented Jun 6, 2019

hey @juanfran-granados how are things going? :-)

@juanfran-granados

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

Hello @futtta, I'm pretty busy these days, but I have more or less 50% done. I'll upload the improvements as soon as I can, previously tested on multisite and standalone instances.

@futtta

This comment has been minimized.

Copy link
Owner

commented Jun 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.