Count subselector on page reference field includes trashed pages

[adrianbj]

Short description of the issue

Page A has a page reference field. If that field on page A has selected page B and you subsequently trash page B and you do a selector search like:

$pages->find('prfield.count>0')

it will include page A in the returned results.

Expected behavior

Pages that reference a trashed page should not be returned.

[Toutouwai]

Related issue: #620

[adrianbj]

Thanks @Toutouwai - I need to have a full read through of that issue. Obviously they are related, but perhaps this one can be fixed even if the other isn't.

There are valid reasons to not empty the trash, so I think this needs to be dealt with.

This stuff is starting to drive me a little crazy :)

[Toutouwai]

I think that when a page is trashed it should be removed from Page Reference fields the same as if it was deleted.

When a page is in the trash it needs to store a bunch of metadata about its relationships that existed before it was trashed (parent, sort, page reference fields, maybe more). It's probably getting beyond what can be stored in the name of the trashed page so perhaps a dedicated table is needed, or the relatively recent $page->meta() could perhaps be used.

[adrianbj]

When a page is in the trash it needs to store a bunch of metadata about its relationships that existed before it was trashed

Yeah, I think this is key - trashed pages should act like they are deleted, but if you decide to restore, everything should be restored, including page reference fields that may have referenced them.

[adrianbj]

@ryancramerdesign - just a follow up note to show this:

Here you can clearly see that the count subselector includes trashed pages, but when you go to get the actual page(s) in the subscriber field, there aren't any. This inconsistency makes it impossible to work with because you can't actually find only pages with a field that references a valid (not trashed) page.

[ryancramerdesign]

The count subfield is part of FieldtypeMulti and it refers to a count of DB rows, not pages. Whether that's what you need I suppose depends on what you are trying to do and whether front-end or back-end. I understand you are looking to find pages that have a page reference field with at least one item in it that is also published/visible. So you aren't actually looking for a count of DB rows and instead looking to filter those DB rows by the status of what they point to. The way that I usually do it is to specify that in the selector. In my case, the field is named "categories", but I think it's probably similar to what you are doing:

$pages->find("categories.status<unpublished, categories.count>0"); 

You could replace "unpublished" with "hidden" or "trash", etc., depending on what your need was. This provides a nice level of control. But if it's too verbose, maybe FieldtypePage could support its own dedicated subfield like "categories.pages>0" or something like that, that excludes hidden, unpublished, trash.

[adrianbj]

@ryancramerdesign - I am not sure whether I should add this here or not (it is kinda related), but I have just noticed that with beach_attributes which is a Table field:

$pages->has('beach_attributes.beach_id=NY626819')

returns the ID of a page that's in the Trash.

I understand technically why it's being returned, but from what I can gather, the use of the has() method needs to include status!=trash in the selector to be useful in almost all use cases.

In this case, count() doesn't include pages in the trash, but maybe that's because this table field isn't paginated, or because it doesn't extend FieldtypeMulti. There are just too many options to keep track of :)

[ryancramerdesign]

@adrianbj That is a purpose of this method, note the has() function title:

“Is there any page that matches the given $selector in the system? (with no exclusions)”

The "no exclusions" part means nothing is excluded automatically. So this is exactly like the get() method except that it returns an ID if there's a match or a 0 if not. If you want exclusions (like trash, unpublished, etc.) then you'll want to use one of the find methods instead (find, findOne, findMany, findIDs, etc.).

has() method needs to include status!=trash in the selector to be useful in almost all use cases

If you find the method only useful by including a "status!=trash" in it, then you've chosen the wrong method for your particular need. It sounds like what you need is findOne(), or perhaps findIDs() with "limit=1" in the selector instead.

In this case, count() doesn't include pages in the trash, but maybe that's because this table field isn't paginated, or because it doesn't extend FieldtypeMulti. There are just too many options to keep track of :)

Unless we're talking about something different than what I think, neither of those are it. You mentioned we are talking about a Table field here, and that is a FieldtypeMulti. I think that only FieldtypeMulti and FieldtypeRepeater support 'count'. When you use "count" in the context of a Table field, it's referring to the number of rows in the table field (same as any other multi-value field). But I think you might be talking about a count on a Page column within a Table field? Like table_field.page_field.count? That would be cool to have, but it's not possible for a Table field to know the quantity of items without loading the field value, so not something that can be queried from a selector. You should be able to identify empty vs. not empty for that case though, i.e. table_field.page_field!=""

[adrianbj]

Thanks @ryancramerdesign - I do appreciate the detailed answer.

Perhaps has() is the wrong method, but I guess I just don't see it being very useful. I thought it was supposed to be the fastest way to find it a page exists or not, but how often do you want to see if a page exists, even if it's in the trash.

I was just looking at the SQL generated for findIDs with limit=1 and count() and was surprised to see the use of SQL_CALC_FOUND_ROWS. My understanding is that this isn't that fast: https://stackoverflow.com/questions/186588/which-is-fastest-select-sql-calc-found-rows-from-table-or-select-count

Also, it is deprecated as of MySQL 8.0.17 so it's probably time to replace it with a separate count select query.

[ryancramerdesign]

@adrianbj PW supports both. You can tell it how you want to get the total by specifying get_total=count or get_total=calc in your selector to $pages->find(). My experience was that CALC was faster when you needed both the count and rows together (as is the cast in most PW page finds), and COUNT was faster when you only needed the total. So that's what PW uses. Though it also depends somewhat on the case, but overall CALC was faster in testing, which is why it's the default when retrieving both rows and total. But this was awhile ago and maybe it depends on the MySQL version. Perhaps they deprecated it in that new version because COUNT is faster in newer versions, I don't know. But my main interest is stability, and short of any major security concern, I stay away from the latest major versions of PHP or MySQL until it's no longer the latest version. But I will explore official support for MySQL 8 soon, and it may be that we have to start doing version detection to adjust behavior for things for things like this, forcing it to use COUNT In MySQL 8, etc.

[adrianbj]

Thanks for the explanation @ryancramerdesign. Regarding versions though, MySQL 8 has been the latest stable for 2 years now and it comes pre-packaged with Debian (the equivalent MariaDB version) and Ubuntu (actual MySQL) so it's widely used on production servers.

In my humble opinion, if PW doesn't properly support the current stable version of MySQL and PHP, it's going to become a big problem. I always run my local dev environment with the very latest of both so I can be ahead of the curve for when I update my production servers, rather than having nasty surprises come along.