From e6a55abf15b51da8edb38a22ddc73e7729107afc Mon Sep 17 00:00:00 2001
From: Jim Graham <info@sparkmediagroup.com>
Date: Tue, 29 Aug 2023 16:03:00 -0400
Subject: [PATCH] Avoid errors and incorrect listings in Trash Manager (#16433)

* Fixes to Trash GetList

Change where condition to correctly group $query param and perform early return of criteria if no deletable resources exist

* Formatting fixes
---
 .../Processors/Resource/Trash/GetList.php     | 47 ++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/core/src/Revolution/Processors/Resource/Trash/GetList.php b/core/src/Revolution/Processors/Resource/Trash/GetList.php
index 6b63c8f0424..3755895ddb9 100644
--- a/core/src/Revolution/Processors/Resource/Trash/GetList.php
+++ b/core/src/Revolution/Processors/Resource/Trash/GetList.php
@@ -1,4 +1,5 @@
 <?php
+
 /*
  * This file is part of MODX Revolution.
  *
@@ -44,6 +45,13 @@ class GetList extends GetListProcessor
      */
     public function prepareQueryBeforeCount(xPDOQuery $c)
     {
+        if ($deleted = $this->getDeleted()) {
+            $c->where(['modResource.id:IN' => $deleted]);
+        } else {
+            $c->where(['modResource.id' => 0]);
+            return $c;
+        }
+
         $query = $this->getProperty('query');
         $context = $this->getProperty('context');
 
@@ -56,25 +64,15 @@ public function prepareQueryBeforeCount(xPDOQuery $c)
         $c->leftJoin(modUser::class, 'User', 'modResource.deletedby = User.id');
         $c->leftJoin(modContext::class, 'Context', 'modResource.context_key = Context.key');
 
-        // TODO add only resources if we have the save permission here (on the context!!)
-        // we need the following permissions:
-        // undelete_document - to restore the document
-        // delete_document - thats perhaps not necessary, because all documents are already deleted
-        // but we need the purge_deleted permission - for every single file
-
         if (!empty($query)) {
-            $c->where(['modResource.pagetitle:LIKE' => '%' . $query . '%']);
-            $c->orCondition(['modResource.longtitle:LIKE' => '%' . $query . '%']);
+            $c->where([
+                'modResource.pagetitle:LIKE' => '%' . $query . '%',
+                'OR:modResource.longtitle:LIKE' => '%' . $query . '%'
+            ]);
         }
         if (!empty($context)) {
             $c->where(['modResource.context_key' => $context]);
         }
-        if ($deleted = $this->getDeleted()) {
-            $c->where(['modResource.id:IN' => $deleted]);
-        } else {
-            $c->where(['modResource.id:IN' => 0]);
-        }
-
         return $c;
     }
 
@@ -88,7 +86,13 @@ public function getDeleted()
         if ($c->prepare() && $c->stmt->execute()) {
             $resources = $c->stmt->fetchAll(PDO::FETCH_ASSOC);
         }
-
+        /*
+            TODO:
+            Filter out resources where user does not have at least one of the permissions
+            applicable to the actions available in the trash manager:
+            1. undelete_document - restore resource
+            2. purge_deleted - permanently destroy resource
+        */
         $deleted = [];
         foreach ($resources as $resource) {
             $deleted[] = (int)$resource['id'];
@@ -108,7 +112,9 @@ public function prepareRow(xPDOObject $object)
         // this is a strange workaround: obviously we can access the resources even if we don't have access to the context! Check that
         // TODO check if that is the same for resource groups
         $context = $this->modx->getContext($object->get('context_key'));
-        if (!$context) return [];
+        if (!$context) {
+            return [];
+        }
 
         $charset = $this->modx->getOption('modx_charset', null, 'UTF-8');
         $objectArray = $object->toArray();
@@ -126,17 +132,16 @@ public function prepareRow(xPDOObject $object)
             if ($parentObject) {
                 $parents[] = $parentObject;
                 $parent = $parentObject->get('parent');
-            }
-            else {
+            } else {
                 break;
             }
         }
 
-        $parentPath = "";
+        $parentPath = '';
         foreach ($parents as $parent) {
-            $parentPath = $parent->get('pagetitle') . " (" . $parent->get('id') . ") > " . $parentPath;
+            $parentPath = $parent->get('pagetitle') . ' (' . $parent->get('id') . ') > ' . $parentPath;
         }
-        $objectArray['parentPath'] = "[" . $objectArray['context_key'] . "] " . $parentPath;
+        $objectArray['parentPath'] = '[' . $objectArray['context_key'] . '] ' . $parentPath;
 
         //  TODO implement permission checks for every resource and return only resources user is allowed to see