feat(structure): slice priority on add + new /move endpoint - #50
Closed
marcohanke wants to merge 3 commits into
Closed
feat(structure): slice priority on add + new /move endpoint#50marcohanke wants to merge 3 commits into
marcohanke wants to merge 3 commits into
Conversation
POST /api/structure/articles/{id}/slices now accepts an optional `priority`
integer in the body. When omitted, the slice is appended at the end (default
behaviour, unchanged). When set to 1 (or any positive integer), the slice is
inserted at that priority within its (article, clang, ctype) group; the core
service's rex_sql_util::organizePriorities() pass renormalises the sequence.
The handler now forwards `priority` into the $data array that
rex_content_service::addSlice() already understands (content_service.php:17-24)
— no new SQL, no new EPs, no behavioural change for callers that don't send
the param.
New POST /api/structure/articles/{id}/slices/{slice_id}/move endpoint with
body { "direction": "moveup" | "movedown" }. Mirrors REDAXO core's
rex_api_content_move_slice exactly:
- Scope: structure/articles/slices/move (Bearer) and the automatically
generated backend/structure/articles/slices/move (session-cookie auth).
- clang_id and module_id are derived from the slice row (no extra body params).
- Permission cascade for session-cookie callers: moveSlice[] perm,
structure category perm, modules complex-perm. Admins bypass moveSlice[].
- Delegates to rex_content_service::moveSlice() which fires SLICE_MOVE,
swaps priority, runs rex_sql_util::organizePriorities(),
calls rex_article_cache::deleteContent() and fires art_content_updated.
No extra EPs or cache calls from the API layer — same as the core API
function.
- rex_api_exception (e.g. first slice tried to moveup) is surfaced as 422
with the original i18n message; any other Exception falls back to 500.
Bearer-scope tests in StructureApiTest.php: - testCreateArticleSliceAtTop: POST with priority=1, then verify the new slice is first in its ctype with priority=1. - testCreateArticleSliceWithoutPriorityAppends: regression — without the param, the slice keeps the previous append-at-end behaviour. - testMoveArticleSliceUp / testMoveArticleSliceDown: two-slice setups, swap order via the new /move endpoint and verify priorities flip. - testMoveArticleSliceInvalidDirection: direction=sideways → 400. - testMoveArticleSliceNotFound: 404 on non-existent slice id. Backend-session tests in BackendApiTest.php: - testAdminCreateArticleSliceAtTop: admin session can create with priority=1. - testAdminMoveArticleSlice: admin session moves a slice via /move. - testRestrictedUserCannotMoveArticleSlice: restricted user (no moveSlice[] perm, no category perm) must get 403/404, not 200. Tests gracefully skip when the test article's template lacks the configured module — same pattern as the existing slice tests.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds slice ordering controls to the Structure API by allowing explicit insertion position on slice creation and introducing a dedicated slice move endpoint (up/down) that mirrors REDAXO core behavior, along with new API/Backend tests.
Changes:
- Extend
POST /structure/articles/{id}/slicesto accept optionalpriorityto insert a slice at a specific position. - Add
POST /structure/articles/{id}/slices/{slice_id}/movewith{ direction: "moveup" | "movedown" }and permission checks. - Add/extend PHPUnit coverage for priority insertion and slice moving (bearer + backend).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/StructureApiTest.php | Adds bearer-auth tests for priority insertion and slice move behavior (contains assertion ordering bugs that need fixing). |
| tests/BackendApiTest.php | Adds backend-admin + restricted-user tests for priority insertion and slice moving (contains an assertion ordering bug that needs fixing). |
| lib/RoutePackage/Structure.php | Adds priority handling in add-slice body and registers/implements the new /move endpoint with 422 mapping for boundary moves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+818
to
+824
| $getSecond = $this->adminGet('structure/articles/' . $articleId . '/slices/' . $secondId); | ||
| $getFirst = $this->adminGet('structure/articles/' . $articleId . '/slices/' . $firstId); | ||
| $this->assertLessThan( | ||
| (int) $getFirst['data']['priority'], | ||
| (int) $getSecond['data']['priority'], | ||
| 'After moveup the second slice should now have a lower priority than the first', | ||
| ); |
|
|
||
| $getSecond = $this->get('structure/articles/' . $articleId . '/slices/' . $secondId); | ||
| $getFirst = $this->get('structure/articles/' . $articleId . '/slices/' . $firstId); | ||
| $this->assertLessThan((int) $getFirst['data']['priority'], (int) $getSecond['data']['priority']); |
|
|
||
| $getFirst = $this->get('structure/articles/' . $articleId . '/slices/' . $firstId); | ||
| $getSecond = $this->get('structure/articles/' . $articleId . '/slices/' . $secondId); | ||
| $this->assertGreaterThan((int) $getSecond['data']['priority'], (int) $getFirst['data']['priority']); |
Comment on lines
+1529
to
+1537
| } catch (rex_api_exception $e) { | ||
| // moveSlice throws when the slice is already at the boundary (top can't moveup, etc.) | ||
| // — surface as 422 rather than 500 since the request was structurally valid but the | ||
| // requested state transition is not possible. | ||
| return new JsonResponse([ | ||
| 'error' => $e->getMessage(), | ||
| 'slice_id' => $sliceId, | ||
| 'direction' => $direction, | ||
| ], 422); |
Member
|
Danke @marcohanke — inhaltlich überzeugt der Zuschnitt, insbesondere dass beides ohne neues SQL und ohne erfundene EPs auskommt und Der Branch ist inzwischen gegen Dieser Text wurde durch eine KI erstellt. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
prioritybody param toPOST /api/structure/articles/{id}/slices. Optional; when omitted, behaviour is unchanged (slice appended at end). When set (e.g.priority: 1), the slice is inserted at that position within its(article, clang, ctype)group. The value is forwarded into the$dataarray ofrex_content_service::addSlice(), which already understandspriority(content_service.php:17-24) and re-normalises the sequence viarex_sql_util::organizePriorities(). No new SQL, no new EPs.POST /api/structure/articles/{id}/slices/{slice_id}/moveendpoint with body{ "direction": "moveup" | "movedown" }. Mirrors REDAXO corerex_api_content_move_slice::execute()1:1 (addons/structure/plugins/content/lib/api_functions/api_content.php).clang_idandmodule_idare derived from the slice row. Permission cascade for session-cookie callers:moveSlice[]perm → structure category perm → modules complex-perm. Admins bypassmoveSlice[]. Bearer-token callers pass through scope-based auth (structure/articles/slices/move). On boundary (moveupon first slice etc.)rex_api_exceptionsurfaces as422with the original i18n message.RoutePackage/Backend/Structure.php—POST /api/backend/structure/articles/{id}/slices/{slice_id}/moveis available with session-cookie auth without a separate Backend implementation.priorityviaPATCH /slices/{id}(backend edit page also doesn't), absolute-position moves, and conveniencemove-to-top/move-to-bottomendpoints (usepriority: 1on add instead).Test plan
./vendor/bin/phpunit tests/StructureApiTest.phpagainst a configuredtests/.envinstance — coverstestCreateArticleSliceAtTop,testCreateArticleSliceWithoutPriorityAppends,testMoveArticleSliceUp,testMoveArticleSliceDown,testMoveArticleSliceInvalidDirection,testMoveArticleSliceNotFound../vendor/bin/phpunit tests/BackendApiTest.php—testAdminCreateArticleSliceAtTop,testAdminMoveArticleSlice,testRestrictedUserCannotMoveArticleSlice.POST /api/structure/articles/{id}/sliceswithpriority: 1→ 201, slice appears at top of its ctype in the list.POST /api/structure/articles/{id}/slices/{slice_id}/movewithdirection: "movedown"→ 200, list reflects new order.POST .../movewithdirection: "moveup"on the top-most slice → 422.POST .../movewithdirection: "sideways"→ 400./redaxo/index.php?page=api/openapi): newpriorityfield appears on the add-slice request schema, new/moveendpoint is listed under structure.structure/articles/slices/moveandbackend/structure/articles/slices/move; a token without the scope returns 401 on the new endpoint.