Add new UI Config panel for Host Exclusion Patterns; See: websharks/comet-cache#754 #250

kristineds · 2016-05-15T17:19:07Z

Add new UI Config panel for Host Exclusion Patterns;

kristineds · 2016-05-15T17:35:00Z

@jaswsinc @raamdev Need some help with the wording and the conditional for the is_multisite(). 266cdf5#diff-7303c30aabc49958b1963a5f09c57332R700

jaswrks · 2016-05-17T12:30:57Z

Lookin' great! :-)

I suggest these opening paragraphs in this panel:

If there are specific domains that should not be cached, you can enter them here so they are excluded automatically. The easiest way to exclude a host is to enter the full domain name on a line of it's own in the field below.

This field also supports Watered-Down Regex syntax, which means that you can also exclude a pattern like: *.example.com or *.example.*. So for instance, if you wanted to exclude all child sites and only cache pages on the Main Site of a Network installation, you could exclude all sub-domains using: *.mynetwork.com. That excludes anything.mynetwork.com, but not mynetwork.com by itself.

I would ditch the Tip: underneath the panel, but leave the Note: as-is.

jaswrks · 2016-05-17T12:33:24Z

src/includes/classes/MenuPageOptions.php

@@ -697,6 +697,28 @@ public function __construct()

        echo '<div class="plugin-menu-page-panel">'."\n";

+        if (is_multisite() && $this->plugin->applyWpFilters(GLOBAL_NS.'_exclude_hosts_option_enable', is_multisite())) {


This line should be:

if ($this->plugin->applyWpFilters(GLOBAL_NS.'_exclude_hosts_option_enable', is_multisite())) {

Meaning, the filter is true on a network by default, and the panel is shown. On a standard install, is_multisite() is false and the filter will default to being off. In either case, a developer can override by attaching to and altering the filtered value used in the conditional.

kristineds · 2016-05-21T09:35:40Z

@jaswsinc @raamdev Updated the description and conditional (0d45adf
) per Jason's feedback. However, when I tried testing, it doesn't seem to work. When I added *.websharks-inc.net on Host Exclusion Patterns on my multisite install, that setting doesn't get saved. Any ideas why? :)

jaswrks · 2016-05-21T21:40:49Z

@kristineds I suggest adding a logging routine while you're debugging this so that you can see more about what's happening internally. For instance, in the updateOptions() routine you could log what occurs there and take a closer look.

file_put_contents(WP_CONTENT_DIR.'/debug.log', print_r(get_defined_vars(), true)."\n\n", FILE_APPEND);

Instead of anything.mynetwork.com I would try:

anything.mynetwork.com ; i.e., anything. in <code> tags instead of the full host name.

just to help improve clarity with respect to anything. being the important difference.

The echo '<div class="plugin-menu-page-panel">'."\n"; line should go inside of the conditional that checks if that markup should be created or not. If not, that line should not be printed. The same with the </div> that closes that tag.

kristineds · 2016-05-23T10:34:22Z

For instance, in the updateOptions() routine you could log what occurs there and take a closer look.

@jaswsinc @raamdev Added the debugging routine here. bac2103

Tested this on a multisite, but the domain still doesn't get save on the field when I save the options. Looking at my debug.log file, I'm seeing this PHP Notice:

[23-May-2016 10:13:09 UTC] PHP Notice:  Undefined index: exclude_hosts in /src/includes/classes/MenuPageOptions.php on line 716

Could you point me in the right direction on what I'm doing wrong? :)

Instead of anything.mynetwork.com I would try:
anything.mynetwork.com ; i.e., anything. in <code> tags instead of the full host name.

Updated here. 8af5937

jaswrks · 2016-05-23T21:10:41Z

src/includes/classes/VsUpgrades.php

@@ -94,6 +94,9 @@ protected function fromLte151107()
                if (!empty($existing_options['cache_clear_xml_sitemap_patterns']) && strpos($existing_options['cache_clear_xml_sitemap_patterns'], '**') === false) {
                    $this->plugin->options['cache_clear_xml_sitemap_patterns'] = str_replace('*', '**', $existing_options['cache_clear_xml_sitemap_patterns']);
                }
+                if (!empty($existing_options['exclude_hosts']) && strpos($existing_options['exclude_hosts'], '**') === false) {


You won't need these lines, because this option did not exist prior; i.e., nothing to upgrade.

kristineds · 2016-05-25T07:28:36Z

You won't need these lines, because this option did not exist prior; i.e., nothing to upgrade.

@jaswsinc That has been updated. 1f3a9aa cc @raamdev

raamdev · 2016-05-26T19:53:19Z

@kristineds Awesome work! 😀 I just put this through a couple of tests and it's working GREAT.

@jaswsinc I noticed that the Host Exclusion Patterns panel shows up even when the Multisite Network is configured to use sub-directories (define('SUBDOMAIN_INSTALL', false);). Is the Host Exclusions Panel of any use when that's the case, or should it only appear when using sub-domains (define('SUBDOMAIN_INSTALL', true);)?

Also, just to confirm: When using multisite with sub-directories, the existing URI Exclusion Patterns feature should be used, correct? (I confirmed that it works with sub-directories; I just want to confirm that's expected.)

If so, I think we should do an additional things there:

Have a special Multisite Network note appear in URI Exclusion Patterns when define('SUBDOMAIN_INSTALL', false); that explains to Network Admins that they can also use URI Exclusion Patterns to exclude specific sites from being cached.

jaswrks · 2016-05-31T11:26:19Z

Is the Host Exclusions Panel of any use when that's the case, or should it only appear when using sub-domains (define('SUBDOMAIN_INSTALL', true);)?

Not really, no. An exception might be for sites running a domain-mapping plugin though.

I would show that panel if ...

if ((defined('SUBDOMAIN_INSTALL') && SUBDOMAIN_INSTALL) || $this->plugin->canConsiderDomainMapping()) {

}

Also, just to confirm: When using multisite with sub-directories, the existing URI Exclusion Patterns feature should be used, correct? (I confirmed that it works with sub-directories; I just want to confirm that's expected.)

Correct, yes.

Have a special Multisite Network note appear in URI Exclusion Patterns when define('SUBDOMAIN_INSTALL', false); that explains to Network Admins that they can also use URI Exclusion Patterns to exclude specific sites from being cached.

Agree.

jaswrks · 2016-05-31T11:27:07Z

Referencing: https://github.com/websharks/comet-cache-pro/blob/160521/src/includes/traits/Shared/DomainMappingUtils.php#L17

jaswrks · 2016-05-31T11:32:35Z

@kristineds So, in the context of these changes where we have the filter. The filter could look like this.

$exclude_hosts_option_enable = is_multisite() &&
    ((defined('SUBDOMAIN_INSTALL') && SUBDOMAIN_INSTALL) || $this->plugin->canConsiderDomainMapping());

if ($this->plugin->applyWpFilters(GLOBAL_NS.'_exclude_hosts_option_enable', $exclude_hosts_option_enable)) {
    ...
}

kristineds · 2016-05-31T17:23:49Z

@jaswsinc Like this? 7516eb9

jaswrks · 2016-06-01T00:28:08Z

Perfect, yes.

raamdev · 2016-06-01T13:12:24Z

@kristineds Is this ready for another review, or are you still working on this?

kristineds · 2016-06-01T18:53:48Z

@raamdev This is ready for review. :)

jaswrks · 2016-06-07T01:04:05Z

src/includes/traits/Plugin/OptionUtils.php

@@ -62,6 +62,8 @@ public function updateOptions(array $options)
        $this->options = array_intersect_key($this->options, $this->default_options);
        update_site_option(GLOBAL_NS.'_options', $this->options);

+        file_put_contents(WP_CONTENT_DIR.'/debug.log', print_r(get_defined_vars(), true)."\n\n", FILE_APPEND);


@kristineds Please remove this line before submitting a PR. This is for local debugging only and should never be included in any merge into the dev branch where it could end up on a live site and create a huge log file over time.

kristineds · 2016-06-07T10:48:53Z

@jaswsinc Yikes. I have removed that line: 7bce007. Thanks for checking! cc @raamdev

raamdev · 2016-06-07T21:10:44Z

@kristineds I just ran this through a few more tests. The automatic hiding of the new Host Exclusion Patterns works great.

There's only one thing left to do:

Have a special Multisite Network note appear at the bottom of URI Exclusion Patterns when is_multisite() && define('SUBDOMAIN_INSTALL', false); that explains to Network Admins that they can also use URI Exclusion Patterns to exclude specific sites from being cached. It should appear at the bottom of that panel and start with "Multisite Networks w/ Sub-Directories:" and explain that URI Exclusion Patterns can also be used to exclude specific sites, e.g., /site1/*.

kristineds · 2016-06-08T17:41:25Z

Have a special Multisite Network note appear at the bottom of URI Exclusion Patterns when is_multisite() && define('SUBDOMAIN_INSTALL', false);

@raamdev Like this? 93e48bf

raamdev · 2016-06-09T16:54:12Z

@kristineds I tested this (did you test after committing your change?) and I'm getting a notice at the bottom:

It looks like there was a bug in my code example. I suggest searching elsewhere in the codebase for a similar check to see how it should be done and then running a test to make sure the code works as expected.

kristineds · 2016-06-10T18:05:18Z

@jaswsinc @raamdev It seems to work but only when define('SUBDOMAIN_INSTALL'). 2a3bf73

raamdev · 2016-06-10T21:07:13Z

@kristineds define() is the wrong function to use here.

raamdev · 2016-06-10T21:10:07Z

@kristineds I see in 2a3bf73 that you're not using define() as your last comment mentions, but rather defined(), which is the correct function.

However, if(defined('SUBDOMAIN_INSTALL') && SUBDOMAIN_INSTALL) checks if SUBDOMAIN_INSTALL is defined and if SUBDOMAIN_INSTALL is true. Do we want SUBDOMAIN_INSTALL to be true when showing the note about sub-directories?

raamdev · 2016-06-10T21:11:37Z

@kristineds So the questions that need to be asked here are:

What are we trying to do?
What condition should this conditional be checking for?

kristineds · 2016-06-14T20:43:50Z

Do we want SUBDOMAIN_INSTALL to be true when showing the note about sub-directories?

@raamdev Should it be if(defined('SUBDOMAIN_INSTALL') && !SUBDOMAIN_INSTALL) instead since what we want is to have SUBDOMAIN_INSTALL to be false to show the note about sub-directories?

raamdev · 2016-06-14T20:47:26Z

@kristineds writes...

Should it be if(defined('SUBDOMAIN_INSTALL') && !SUBDOMAIN_INSTALL)

Yes, that looks correct to me. If you could test that on a multisite network with sub-directories (see https://codex.wordpress.org/Multisite_Network_Administration#Switching_network_types), that would be great. The note should NOT show up when a Multisite Network is using Sub-Domains, since that note is only applicable when using Sub-Directories.

raamdev · 2016-06-20T23:06:37Z

@kristineds FYI, I just merged the latest from 000000-dev into this feature branch, so you'll want to git pull to make sure you have those changes locally if you're still doing testing for this feature branch.

See wpsharks/comet-cache#754

raamdev · 2016-06-24T17:53:20Z

@kristineds FYI, you had the note about Multisite Networks w/ Sub-Directories in the wrong panel; it should've gone in the URI Exclusion Patterns panel, not the Host Exclusion Patterns (in the scenario in which that note appears, the Host Exclusion Patterns panel is hidden, so it would never be seen). I fixed this in c181f9c.

kristineds · 2016-06-24T18:44:32Z

@raamdev Yikes. 😥 Thanks for catching that.

Add new UI Config panel for Host Exclusion Patterns; See: wpsharks/co…

266cdf5

…met-cache#754

kristineds added the in progress label May 15, 2016

jaswrks reviewed May 17, 2016
View reviewed changes

kristineds added 2 commits May 21, 2016 17:00

Merge branch '000000-dev' into feature/754

4d121cc

Fix conditional and update description; See: wpsharks/comet-cache#754

0d45adf

kristineds added 3 commits May 23, 2016 16:11

Merge branch '000000-dev' into feature/754

5295d96

Tweak codebase; See: wpsharks/comet-cache#754

8af5937

Tweak codebase for debugging; See: wpsharks/comet-cache#754

bac2103

jaswrks reviewed May 23, 2016
View reviewed changes

Update VSUpgrades file; See: wpsharks/comet-cache#754

1f3a9aa

kristineds added ready for review and removed in progress labels May 25, 2016

raamdev added in progress and removed ready for review labels May 26, 2016

kristineds added 2 commits June 1, 2016 01:13

Merge branch '000000-dev' into feature/754

c2ba371

Update filter to consider domain mapping; See: wpsharks/comet-cache#754

7516eb9

Merge branch '000000-dev' into feature/754

ce3aa34

jaswrks reviewed Jun 7, 2016
View reviewed changes

kristineds added 2 commits June 7, 2016 18:42

Merge branch '000000-dev' into feature/754

a66ab02

Remove logging routine; See: wpsharks/comet-cache#754

7bce007

raamdev added in progress and removed ready for review labels Jun 7, 2016

Add special Multisite Network note; See: wpsharks/comet-cache#754

93e48bf

Fix PHP notice; See: wpsharks/comet-cache#754

2a3bf73

Merge branch '000000-dev' into feature/754

848b598

Update conditional; See: wpsharks/comet-cache#754

b836797

Merge branch '000000-dev' into feature/754

273f245

raamdev added 2 commits June 24, 2016 13:46

Merge branch '000000-dev' into feature/754

5b82dc7

Move MS Networks w/ Sub-Directories note to URI Exclusion Patterns

c181f9c

See wpsharks/comet-cache#754

raamdev added in review and removed in progress labels Jun 24, 2016

raamdev closed this Jun 24, 2016

raamdev deleted the feature/754 branch June 24, 2016 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new UI Config panel for Host Exclusion Patterns; See: websharks/comet-cache#754 #250

Add new UI Config panel for Host Exclusion Patterns; See: websharks/comet-cache#754 #250

kristineds commented May 15, 2016

kristineds commented May 15, 2016

jaswrks commented May 17, 2016

jaswrks May 17, 2016

kristineds commented May 21, 2016

jaswrks commented May 21, 2016

kristineds commented May 23, 2016

jaswrks May 23, 2016

kristineds commented May 25, 2016

raamdev commented May 26, 2016

jaswrks commented May 31, 2016

jaswrks commented May 31, 2016

jaswrks commented May 31, 2016 •

edited

Loading

kristineds commented May 31, 2016

jaswrks commented Jun 1, 2016

raamdev commented Jun 1, 2016

kristineds commented Jun 1, 2016

jaswrks Jun 7, 2016

kristineds commented Jun 7, 2016

raamdev commented Jun 7, 2016

kristineds commented Jun 8, 2016

raamdev commented Jun 9, 2016

kristineds commented Jun 10, 2016

raamdev commented Jun 10, 2016

raamdev commented Jun 10, 2016

raamdev commented Jun 10, 2016

kristineds commented Jun 14, 2016

raamdev commented Jun 14, 2016

raamdev commented Jun 20, 2016

raamdev commented Jun 24, 2016

kristineds commented Jun 24, 2016

		@@ -697,6 +697,28 @@ public function __construct()

		echo '<div class="plugin-menu-page-panel">'."\n";

		if (is_multisite() && $this->plugin->applyWpFilters(GLOBAL_NS.'_exclude_hosts_option_enable', is_multisite())) {

Add new UI Config panel for Host Exclusion Patterns; See: websharks/comet-cache#754 #250

Add new UI Config panel for Host Exclusion Patterns; See: websharks/comet-cache#754 #250

Conversation

kristineds commented May 15, 2016

kristineds commented May 15, 2016

jaswrks commented May 17, 2016

jaswrks May 17, 2016

Choose a reason for hiding this comment

kristineds commented May 21, 2016

jaswrks commented May 21, 2016

kristineds commented May 23, 2016

jaswrks May 23, 2016

Choose a reason for hiding this comment

kristineds commented May 25, 2016

raamdev commented May 26, 2016

jaswrks commented May 31, 2016

jaswrks commented May 31, 2016

jaswrks commented May 31, 2016 • edited Loading

kristineds commented May 31, 2016

jaswrks commented Jun 1, 2016

raamdev commented Jun 1, 2016

kristineds commented Jun 1, 2016

jaswrks Jun 7, 2016

Choose a reason for hiding this comment

kristineds commented Jun 7, 2016

raamdev commented Jun 7, 2016

kristineds commented Jun 8, 2016

raamdev commented Jun 9, 2016

kristineds commented Jun 10, 2016

raamdev commented Jun 10, 2016

raamdev commented Jun 10, 2016

raamdev commented Jun 10, 2016

kristineds commented Jun 14, 2016

raamdev commented Jun 14, 2016

raamdev commented Jun 20, 2016

raamdev commented Jun 24, 2016

kristineds commented Jun 24, 2016

jaswrks commented May 31, 2016 •

edited

Loading