-
Notifications
You must be signed in to change notification settings - Fork 14
Enable drag-and-drop ordering in admin controllers #596
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.
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
Enable drag-and-drop ordering in admin controllers #596
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public static function updatePositionsByHook(array $positionsByHook, int $idShop): bool | ||
| { | ||
| if (empty($positionsByHook)) { | ||
| return true; | ||
| } | ||
|
|
||
| $db = Db::getInstance(); | ||
|
|
||
| foreach ($positionsByHook as $idHook => $blockIds) { | ||
| if (!is_array($blockIds) || empty($blockIds)) { | ||
| continue; | ||
| } | ||
|
|
||
| $position = 0; | ||
| $seen = []; | ||
|
|
||
| foreach ($blockIds as $blockId) { | ||
| $blockId = (int) $blockId; | ||
| if ($blockId <= 0 || isset($seen[$blockId])) { | ||
| continue; | ||
| } | ||
|
|
||
| $seen[$blockId] = true; | ||
|
|
||
| $updated = $db->update( | ||
| 'everblock', | ||
| ['position' => (int) $position], | ||
| 'id_everblock = ' . (int) $blockId | ||
| . ' AND id_hook = ' . (int) $idHook | ||
| . ' AND id_shop = ' . (int) $idShop | ||
| ); | ||
|
|
||
| if (!$updated) { | ||
| return false; | ||
| } | ||
|
|
||
| ++$position; | ||
| } | ||
|
|
||
| static::clearPositionsCache((int) $idHook, (int) $idShop); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve existing positions when reordering paginated blocks
The new EverBlockClass::updatePositionsByHook() assigns positions starting at zero for every hook based only on the identifiers posted by the drag‑and‑drop request. When the block list is paginated or filtered, that request only contains the rows visible on the current page. Reordering items on page 2 will therefore reset those items to positions 0…N while blocks from page 1 keep their existing 0…N range, producing duplicated position values inside the same hook and making subsequent ordering unpredictable. Consider fetching the full list of IDs per hook (or applying an offset based on existing min/max positions) before renumbering so that updates remain consistent across pages.
Useful? React with 👍 / 👎.
| public static function updatePositions(array $orderedIds, int $shopId): bool | ||
| { | ||
| if (empty($orderedIds)) { | ||
| return true; | ||
| } | ||
|
|
||
| $db = Db::getInstance(); | ||
| $position = 0; | ||
| $seen = []; | ||
|
|
||
| foreach ($orderedIds as $id) { | ||
| $id = (int) $id; | ||
| if ($id <= 0 || isset($seen[$id])) { | ||
| continue; | ||
| } | ||
|
|
||
| $seen[$id] = true; | ||
|
|
||
| $updated = $db->update( | ||
| 'everblock_faq', | ||
| ['position' => (int) $position], | ||
| 'id_everblock_faq = ' . (int) $id . ' AND id_shop = ' . (int) $shopId | ||
| ); | ||
|
|
||
| if (!$updated) { | ||
| return false; | ||
| } | ||
|
|
||
| ++$position; | ||
| } | ||
|
|
||
| static::clearPositionsCache((int) $shopId); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FAQ reordering ignores items outside current page
EverblockFaq::updatePositions() renumbers only the IDs received from the AJAX call, starting again from 0. If the FAQ list spans multiple pages, a drag action on any page other than the first will renumber just that subset to 0…N while FAQs that were not part of the request retain their previous 0…N positions. This yields duplicate position values within the same shop and makes ordered displays inconsistent. To avoid this, compute positions using the complete ordered set for the shop (or at least start numbering after the highest existing position among excluded items).
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f9034c8fdc832298bb9e8b452c75a2