Skip to content

10.6 Fix to set/reset additional parameter on the tree node locator #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: 10.6
Choose a base branch
from
Open

10.6 Fix to set/reset additional parameter on the tree node locator #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c8bd95
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 12, 2024
d056625
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 13, 2024
3ac75bc
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 13, 2024
7bd0174
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 13, 2024
3f567dc
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 25, 2024
f26e07d
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 25, 2024
9e9818b
10.6 Fix to set/reset additional parameter on the tree node locator t…
mugge6 Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 133 additions & 45 deletions bundles/AdminBundle/Controller/Admin/DataObject/DataObjectController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,50 +118,32 @@ public function treeGetChildsByIdAction(Request $request, EventDispatcherInterfa
$limit = 100;
}

$childrenList = new DataObject\Listing();
$childrenList->setCondition($this->buildChildrenCondition($object, $filter, $view));
$childrenList->setLimit($limit);
$childrenList->setOffset($offset);

if ($object->getChildrenSortBy() === 'index') {
$childrenList->setOrderKey('objects.o_index ASC', false);
} else {
$childrenList->setOrderKey(
sprintf(
'CAST(objects.o_%s AS CHAR CHARACTER SET utf8) COLLATE utf8_general_ci %s',
$object->getChildrenSortBy(), $object->getChildrenSortOrder()
),
false
);
}
$childrenList->setObjectTypes($objectTypes);

Element\Service::addTreeFilterJoins($cv, $childrenList);

$beforeListLoadEvent = new GenericEvent($this, [
'list' => $childrenList,
'context' => $allParams,
]);
$eventDispatcher->dispatch($beforeListLoadEvent, AdminEvents::OBJECT_LIST_BEFORE_LIST_LOAD);

/** @var DataObject\Listing $childrenList */
$childrenList = $beforeListLoadEvent->getArgument('list');

$children = $childrenList->load();
$filteredTotalCount = $childrenList->getTotalCount();

foreach ($children as $child) {
$objectTreeNode = $this->getTreeNodeConfig($child);
// this if is obsolete since as long as the change with #11714 about list on line 175-179 are working fine, we already filter the list=1 there
if ($objectTreeNode['permissions']['list'] == 1) {
$objects[] = $objectTreeNode;
}
}

//pagination for custom view
$total = $cv
? $filteredTotalCount
: $object->getChildAmount(null, $this->getAdminUser());
// CANDO OPTIMIZATION START
// Because of the tree node locator is searching the tree with several pages per node with a binary search,
// which is triggered by the Admin UI JS. If there is a node with thousands of items, it makes a new request
// for every halving until the searched item was found between the offset and limit.
// To reduce the amount of requests this fix will get ID of the child node. If there are more items in the
// node as the limit it will recursive iterate through every page till the correct page is found.
// The original part of code is refactored into a separate function, which can be called recursively.
$childNodeId = $request->get('childNodeId', null);
$result = $this->getChildrenList(
$eventDispatcher,
$object,
$objectTypes,
$allParams,
$limit,
$offset,
$filter,
$view,
$cv,
$childNodeId
);
// get the children list data and map them to the correct variables
$objects = $result['objects'];
$filteredTotalCount = $result['filteredTotalCount'];
$offset = $result['offset'];
$total = $result['total'];
// CANDO OPTIMIZATION END
}

//Hook for modifying return value - e.g. for changing permissions based on object data
Expand Down Expand Up @@ -189,6 +171,107 @@ public function treeGetChildsByIdAction(Request $request, EventDispatcherInterfa
return $this->adminJson($objects);
}

// CANDO OPTIMIZATION START
private function getChildrenList(
EventDispatcherInterface $eventDispatcher,
DataObject $object,
array $objectTypes,
array $allParams,
int $limit,
int $offset,
?string $filter,
?string $view,
array|bool|null $cv,
?string $childNodeId
): array {
$objects = [];
$childrenList = new DataObject\Listing();
$childrenList->setCondition($this->buildChildrenCondition($object, $filter, $view));
$childrenList->setLimit($limit);
$childrenList->setOffset($offset);

if ($object->getChildrenSortBy() === 'index') {
$childrenList->setOrderKey('objects.o_index ASC', false);
} else {
$childrenList->setOrderKey(
sprintf(
'CAST(objects.o_%s AS CHAR CHARACTER SET utf8) COLLATE utf8_general_ci %s',
$object->getChildrenSortBy(), $object->getChildrenSortOrder()
),
false
);
}
$childrenList->setObjectTypes($objectTypes);

Element\Service::addTreeFilterJoins($cv, $childrenList);

$beforeListLoadEvent = new GenericEvent($this, [
'list' => $childrenList,
'context' => $allParams,
]);
$eventDispatcher->dispatch($beforeListLoadEvent, AdminEvents::OBJECT_LIST_BEFORE_LIST_LOAD);

/** @var DataObject\Listing $childrenList */
$childrenList = $beforeListLoadEvent->getArgument('list');

$children = $childrenList->load();
$filteredTotalCount = $childrenList->getTotalCount();

// only execute the recursive call if the childNodeId is set
if (!empty($childNodeId)) {
// check if childNodeId is in current list, if not do it again by recalling this function
$found = false;
foreach ($children as $child) {
if ($child->getId() === (int) $childNodeId) {
$found = true;
break;
}
}

// if the child node is not found, then call this function again
if (!$found) {
$nextOffset = $offset + $limit;
// if next offset is higher than the filtered total count, then the item was not found at all
if ($filteredTotalCount > $nextOffset) {
return $this->getChildrenList(
$eventDispatcher,
$object,
$objectTypes,
$allParams,
$limit,
$nextOffset,
$filter,
$view,
$cv,
$childNodeId
);
}
}
}

foreach ($children as $child) {
$objectTreeNode = $this->getTreeNodeConfig($child);
// this if is obsolete since as long as the change with #11714 about list on line 175-179 are working fine, we already filter the list=1 there
if ($objectTreeNode['permissions']['list'] == 1) {
$objects[] = $objectTreeNode;
}
}

//pagination for custom view
$total = $cv
? $filteredTotalCount
: $object->getChildAmount(null, $this->getAdminUser());

// return all calculated values to the calling function as an array
return [
'objects' => $objects,
'filteredTotalCount' => $filteredTotalCount,
'offset' => $offset,
'total' => $total,
];
}
// CANDO OPTIMIZATION END

/**
* @param DataObject\AbstractObject $object
* @param string|null $filter
Expand Down Expand Up @@ -1160,9 +1243,14 @@ private function executeUpdateAction(DataObject $object, mixed $values): array

$object->save();

if ($isIndexUpdate) {
// CANDO OPTIMIZATION START
// do update indexes only if parents childrenSortBy is set to index
// sort by index cannot be set on nodes with paged children anyway
// it doesn't make sense to sort by index on large amount of data in one node
if ($isIndexUpdate && $parent->getChildrenSortBy() === 'index') {
$this->updateIndexesOfObjectSiblings($object, $indexUpdate);
}
// CANDO OPTIMIZATION END

$success = true;
} catch (\Exception $e) {
Expand Down
8 changes: 8 additions & 0 deletions bundles/AdminBundle/Resources/public/js/pimcore/overrides.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,14 @@ Ext.define('pimcore.data.PagingTreeStore', {
me.superclass.onProxyLoad.call(this, operation);
var proxy = this.getProxy();
proxy.setExtraParam("start", 0);

// CANDO OPTIMIZATION START
// this line will reset the childNodeId to make sure it'll not interfere with other proxy requests
// childNodeId will be set in the treenodelocator.js and can be used within a custom listener
// to improve the performance on the client side
proxy.setExtraParam('childNodeId', null);
// CANDO OPTIMIZATION END

} catch (e) {
console.log(e);
}
Expand Down
7 changes: 7 additions & 0 deletions bundles/AdminBundle/Resources/public/js/pimcore/treenodelocator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,13 @@ pimcore.treenodelocator = function()
proxy.setExtraParam("start", pagingState.offset);
node.pagingData.offset = pagingState.offset;

// CANDO OPTIMIZATION START
// this line will set the childNodeId to add it to the backend request and can be used within
// a custom listener with the event pimcore.admin.object.list.beforeListLoad to improve the performance
// childNodeId will be reset in the treenodelocator.js
proxy.setExtraParam('childNodeId', pagingState.childNodeId);
// CANDO OPTIMIZATION END

store.load({
node: node,
callback: self.processPaging
Expand Down
Loading