Pages hidden from the Page List still show up on the Page Tree

[michaellenaghan]

Short description of the issue

The ProcessPageList module allows you to hide pages "in page list(s)." It does hide pages in the page list — but it doesn't hide them in the Pages | Tree menu.

Expected behavior

When you hide pages in the ProcessPageList module they should be hidden in the page list and in the Pages | Tree menu.

Actual behavior

When you hide pages in the ProcessPageList module they're hidden in the page list but not in the Pages | Tree menu.

Optional: Screenshots/Links that demonstrate the issue

When you hide pages in the ProcessPageList module:

they're hidden in the page list:

but not in the Pages | Tree menu:

(Note that the last two screenshots are from a non-superuser.)

Steps to reproduce the issue

1. Create some pages under Home
2. Go to Modules | Configure | ProcessPageList
3. Add those pages to "Hide these pages in page list(s)"
4. Log out
5. Disable advanced mode
6. Disable debug mode
7. Log in as a non-superuser
8. The pages should be hidden from the main page list
9. The pages should be visible in the Pages | Tree menu

Setup/Environment

• ProcessWire 3.0.229

[ryancramerdesign]

@michaellenaghan Thanks, I've pushed a fix for this.

[matjazpotocnik]

@michaellenaghan, can we close this issue?

[michaellenaghan]

Sorry; I'll verify the fix on Mon.

[michaellenaghan]

It looks like this is not fixed.

Notice that File Atom.atom, File Robots.txt, and File Sitemap.xml appear in the menu on the right, but not in the list on the left.

I'll see if I can spot the problem in the code.

[michaellenaghan]

It looks like the problem is here:

https://github.com/processwire/processwire/blob/5b0e37e3aecf2f2480a404bd5b4b5bbd4b820fb5/wire/modules/Process/ProcessPageList/ProcessPageList.module#L562-L570

Specifically, here:

    if($c != $nots) $a = array_merge($a, $this->hidePages);

$nots is the set of optional conditions that turn off page hiding. $c is the set of those conditions that currently obtain. The intention is to say, essentially, "hide pages if all of the optional conditions currently obtain." What's missing is the possibility that there might not be any optional conditions. If there aren't, the pages should always be hidden.

Here's a possible fix:

    if(empty($nots) || $c != $nots) $a = array_merge($a, $this->hidePages);

I've added a test for empty($nots).

That's a quick fix, but a better fix might be to test $nots at the top, and skip building $c if it's empty. (If $nots is empty the hidden pages would always be merged into $a.) It's potentially better not because it's "optimized", but rather because it makes the intention of the code clearer: if there are optional conditions we need to check them before we hide pages; if there aren't, we don't.

[matjazpotocnik]

@michaellenaghan, thanks for digging into this; @ryancramerdesign, I can confirm that test for empty($nots) works for me.

[ryancramerdesign]

@michaellenaghan Thanks, I've pushed your suggested fix for this.