Skip to content

Commit

Permalink
Avoid errors and incorrect listings in Trash Manager (#16433)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Jim Graham authored Aug 29, 2023
1 parent 0540f82 commit e6a55ab
Showing 1 changed file with 26 additions and 21 deletions.
47 changes: 26 additions & 21 deletions core/src/Revolution/Processors/Resource/Trash/GetList.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

/*
* This file is part of MODX Revolution.
*
Expand Down Expand Up @@ -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');

Expand All @@ -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;
}

Expand All @@ -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'];
Expand All @@ -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();
Expand All @@ -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

Expand Down

0 comments on commit e6a55ab

Please sign in to comment.