This repository was archived by the owner on Aug 13, 2026. It is now read-only.
Conversation
…sers Required By pagination filtered permissions after the SQL LIMIT/OFFSET, so a restricted user's page count matched an admin's while individual pages could come back sparse or empty, and the ExtJS grid treated a zero-row page as the end of the dataset, blocking navigation to later pages that had visible items. - getRequiredByDependenciesAction now scans past hidden rows to fill each page, re-deriving the raw-to-visible mapping from scratch on every request so paging stays correct across independent calls, and reports an exact permission-filtered total via the newly cached pimcore/pimcore Service::getRequiredByVisibleTotalCount(). - getRequiresDependenciesAction filters the already fully-loaded in-memory requires set before slicing, giving an exact total at no extra cost. - dependencies.js refreshes the hidden-dependencies note on every page load instead of only the first. - overrides.js adds a narrow Ext.toolbar.Paging fix so navigation isn't disabled when a page is empty but the store's total indicates more pages exist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes dependency-grid pagination for restricted users by filtering permissions before paging.
Changes:
- Adds permission-aware pagination and totals.
- Refreshes hidden-dependency notices per page.
- Overrides ExtJS empty-page navigation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/Controller/Admin/ElementController.php |
Adds permission-aware dependency pagination. |
public/js/pimcore/element/dependencies.js |
Refreshes hidden-dependency notices. |
public/js/pimcore/overrides.js |
Adjusts empty-page paging controls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Both path-filter branches (getFilterRequiresByPath/getFilterRequiredByPath) were still raw SQL LIMIT/OFFSET with no permission filtering, so typing into the grid's path filter reproduced the original bug even after the unfiltered listing was fixed. Both now go through the same permission-aware chunk scan as the unfiltered path, via a new shared scanDependencyPage() helper. - The scan cap was a flat ceiling independent of how far a page's offset was into visible-space, so sufficiently deep pages into a >5000-row raw set were permanently unreachable rather than just more expensive. The scan budget now scales with the requested offset, and each chunk request is bounded by the remaining budget so an oversized limit (e.g. the grid's "show all" option) can't itself blow past the cap in one query. - hasHidden for the unfiltered Required By listing now comes from the new core Service::getRequiredByHasHiddenDependencies(), which shares the same ~5000-row scan as the cached total, instead of only reflecting whichever hidden rows a single page's much shorter fill-scan happened to pass on the way to collecting that page's items. - The Ext.toolbar.Paging override now fully mirrors the stock onLoad() or (refresh button re-enable, page input, "change" event) instead of an early return, changing only how "isEmpty" is determined. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
The This branch calls two new Once pimcore/pimcore#19257 is merged into |
…c claim - scanDependencyPage()'s scan budget scaled with $offset alone, growing only by $limit per page - a hidden-row run bigger than the base cap could take dozens or hundreds of forward page clicks to cross even though it isn't a permanent block. Each page number now gets its own full scan cap instead, so the budget grows much faster relative to how far the user has actually paged. - The loop only treated a fully empty fetch as reaching the end of the (possibly filtered) raw set, so a result that exactly filled the requested page never set that flag, and reported an inflated total instead of the exact one. Also, once genuinely exhausted, the total was computed as $offset + count($page), which overstates the true total whenever $offset lands beyond the last visible row. Both fixed: end-of-set is now detected whenever a fetch returns fewer rows than requested (regardless of whether the page also happened to fill), and the exact total is the number of visible rows actually seen. - Softened an inline comment that overclaimed the core-sourced total/ hasHidden are unconditionally exact - they share the same underlying scan cap as scanDependencyPage() and fall back conservatively beyond it, same as everywhere else in this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nsafe The previous page-number-scaled budget (introduced to make deep pages reachable faster) broke a required invariant and could permanently orphan visible rows: since every page's scan restarts at raw offset 0 and skips the first $offset visible rows it finds (trusting that an earlier, smaller-budget page already returned them), a later page's budget growing faster than its own offset lets it reach visible rows an earlier page's smaller budget never got to - and then discard them as "already shown", when nothing ever actually returned them. Confirmed by walking through Copilot's counterexample (a hidden run just over the cap, followed by visible rows): page 1 legitimately finds nothing (budget too small), but page 2's larger budget reaches the same visible rows and skips the first $limit of them anyway, since its own offset says to - permanently losing them from every page. Reverted to `offset + limit + cap`, which keeps budget growth in the same units as offset so this can't happen, at the cost of needing more forward paging to cross an unusually long hidden run - the safer of the two trade-offs, and the only one that doesn't produce phantom-lost data by design. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both prior designs (page-number-scaled, then offset-synced) bounded total raw rows scanned with a single counter that scaled with $offset, which meant whether a scan could get past a given hidden-row run depended on which page was asking. A later page's bigger allowance could reach visible rows an earlier page's smaller one never got to, then discard them via its own offset-based skip as "already returned by that earlier page" - permanently losing them, since every subsequent page skips exactly $offset of whatever it finds, not what previous pages actually returned. The offset-synced version narrowed this from whole orphaned pages to a small constant number of rows right at the hidden/visible boundary, but didn't eliminate it. Now hidden rows and visible rows are tracked as two independent counters: hiddenSeen is capped at a fixed constant that does not depend on $offset/$limit at all, so whether a given hidden run is crossable is the same answer for every page - either all of them get past it and behave correctly, or none do (consistent empty result, no partial/orphaned state in between). Verified empirically with a standalone simulation across several hidden-row layouts (leading run under/at/over the cap, interspersed hidden rows, large "show all" limit): every true visible row is found exactly once with no duplicates when the hidden run stays within the cap, and cap-exceeding runs fail the same way on every page rather than losing a handful of rows at the boundary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Controller/Admin/ElementController.php:800
- Model\Element\Service::getElementById() can return null; calling $element->getDependencies() unconditionally will trigger a fatal error for invalid/deleted IDs. getVersionsAction in this controller guards against this case, but getRequiresDependenciesAction currently does not.
$element = Model\Element\Service::getElementById($type, $id);
$dependencies = $element->getDependencies();
src/Controller/Admin/ElementController.php:875
- Model\Element\Service::getElementById() can return null; calling $element->getDependencies() unconditionally will trigger a fatal error for invalid/deleted IDs. This endpoint should bail out the same way as getVersionsAction does when the element is missing.
$element = Model\Element\Service::getElementById($type, $id);
$dependencies = $element->getDependencies();
Co-Authored-By: Claude <noreply@anthropic.com>
|
jcPimcore
deleted the
851-bug-ec-pees-1176-dependencies-tab-pagination-broken-for-restricted-users-admin-ui-classic
branch
July 27, 2026 14:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves https://github.com/pimcore/service-operations/issues/851
Depends on: pimcore/pimcore#19257
Required By pagination filtered permissions after the SQL LIMIT/OFFSET, so a restricted user's page count matched an admin's while individual pages could come back sparse or empty, and the ExtJS grid treated a zero-row page as the end of the dataset, blocking navigation to later pages that had visible items.